[Flight] Allow String Chunks to Passthrough in Node streams and renderToMarkup#30131
Merged
sebmarkbage merged 3 commits intofacebook:mainfrom Jul 3, 2024
Merged
[Flight] Allow String Chunks to Passthrough in Node streams and renderToMarkup#30131sebmarkbage merged 3 commits intofacebook:mainfrom
sebmarkbage merged 3 commits intofacebook:mainfrom
Conversation
…chunks It can be efficient to accept raw string chunks to pass through a stream instead of encoding them into a binary copy first. However, to avoid having to deal with encoding them back to binary from the parser itself, this helper is limited to dealing with unsplit and unconcattened strings so that we can make some assumptions. This is mainly intended for use for pass-through within the same memory.
Also, let Node accept string chunks as long as they're following our expected constraints. This lets us test the mixed protocol using pass-throughs.
This lets us avoid the dependency on TextDecoder/TextEncoder.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
facebook-github-bot
added
CLA Signed
React Core Team
sebmarkbage
commented
Jun 28, 2024
| return new Promise((resolve, reject) => { | ||
| const textEncoder = new TextEncoder(); | ||
| const flightDestination = { | ||
| push(chunk: string | null): boolean { |
Contributor
Author
There was a problem hiding this comment.
We currently only encode strings in these streams because there's no reason to send any binary data to the "client". However, if we enabled Blobs to be used inside e.g. <img src> as planned, then there would be reason to do that and we can enable the stream config to allow pushing raw binary chunks and receive them here using processBinaryChunk.
eps1lon
approved these changes
Jul 3, 2024
This was referenced Jul 25, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
It can be efficient to accept raw string chunks to pass through a stream instead of encoding them into a binary copy first.
Previously our Flight parsers didn't accept receiving string chunks. That's partly because we sometimes need to encode binary chunks anyway so string only transport isn't enough but some chunks can be strings. This adds a partial ability for chunks to be received as strings.
However, accepting strings comes with some downsides. E.g. if the strings are split up we need to buffer it which compromises the perf for the common case. If the chunk represents binary data, then we'd need to encode it back into a typed array which would require a TextEncoder dependency in the parser. If the string chunk represents a byte length encoded string we don't know how many unicode characters to read without measuring them in terms of binary - also requiring a TextEncoder.
This PR is mainly intended for use for pass-through within the same memory. We can simplify the implementation by assuming that any string chunk is passed as the original chunk. This requires that the server stream config doesn't arbitrarily concatenate strings (e.g. large strings should not be concatenated which is probably a good heuristic anyway). It also means that this is not suitable to be used with for example receiving string chunks on the client by passing them through SSR hydration data - except if the encoding that way was only used with chunks that were already encoded as strings by Flight.
Web streams mostly just work on binary data anyway so they can't use this.
In Node.js streams we concatenate precomputed and small strings into larger buffers. It might make sense to do that using string ropes instead. However, in the meantime we can at least pass large strings that are outside our buffer view size as raw strings. There's no benefit to us eagerly encoding those.
Also, let Node accept string chunks as long as they're following our expected constraints. This lets us test the mixed protocol using pass-throughs. This can also be useful when the RSC server is in the same environment as the SSR server as they don't have to go from strings to typed arrays back to strings.
Now we can also use this in the pass-through used in renderToMarkup. This lets us avoid the dependency on TextDecoder/TextEncoder in that package.