-
Notifications
You must be signed in to change notification settings - Fork 26.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix streaming SSR with multi-byte characters #35724
Fix streaming SSR with multi-byte characters #35724
Conversation
export function decodeText(input?: Uint8Array) { | ||
return new TextDecoder().decode(input) | ||
export function decodeText(input?: Uint8Array, textDecoder?: TextDecoder) { | ||
return textDecoder ? textDecoder.decode(input, { stream: true } ) : new TextDecoder().decode(input) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since decodeText is being exported I didn't want to mess with the code signature, so I just added the textDecoder argument.
@@ -207,9 +207,11 @@ export function createBufferedTransformStream(): TransformStream< | |||
return pendingFlush | |||
} | |||
|
|||
const textDecoder = new TextDecoder(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to keep the same TextEncoder instance until the stream ends, or else the stream option won't work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pause for incoming patch, will re-approve later. would be nice to have a test during this period
This comment has been minimized.
This comment has been minimized.
986bdbb
to
6c7de5e
Compare
export default function Page() { | ||
return ( | ||
<div> | ||
<p>{"マルチバイト".repeat(28)}</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
マルチバイト is multi-byte in Japanese. Repeating this word exactly 28 times results in corrupted text in the response.
I've added a test. Thank you so much for the work you do on Next.js! |
Thanks for adding the test! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! 🙏
This comment has been minimized.
This comment has been minimized.
It seems that there's a lint error in packages/next/server/node-web-streams-helper.ts. |
When using streaming SSR decodeText is called repeatedly with incoming chunks of data. In that case a multi-byte character may occasionally split between chunks, causing corruption. By setting the TextDecoder option 'stream' to true, and reusing the same TextDecoder instance, TextDecoder will memorise “unfinished” characters and decode them when the next chunk comes.
14a3369
to
04cf4c9
Compare
I'm sorry, my bad! I ran lint-fix and pushed the changes. |
Failing test suitesCommit: 04cf4c9
Expand output● CLI Usage › info › should print output
Read more about building and testing Next.js in contributing.md. |
Stats from current PRDefault Build (Increase detected
|
vercel/next.js canary | martinnabhan/next.js fix/multi-byte-streaming-ssr | Change | |
---|---|---|---|
buildDuration | 16.5s | 16.1s | -394ms |
buildDurationCached | 6.3s | 6.4s | |
nodeModulesSize | 467 MB | 467 MB |
Page Load Tests Overall increase ✓
vercel/next.js canary | martinnabhan/next.js fix/multi-byte-streaming-ssr | Change | |
---|---|---|---|
/ failed reqs | 0 | 0 | ✓ |
/ total time (seconds) | 3.349 | 3.289 | -0.06 |
/ avg req/sec | 746.49 | 760.04 | +13.55 |
/error-in-render failed reqs | 0 | 0 | ✓ |
/error-in-render total time (seconds) | 1.501 | 1.484 | -0.02 |
/error-in-render avg req/sec | 1665.26 | 1684.63 | +19.37 |
Client Bundles (main, webpack)
vercel/next.js canary | martinnabhan/next.js fix/multi-byte-streaming-ssr | Change | |
---|---|---|---|
925.HASH.js gzip | 179 B | 179 B | ✓ |
framework-HASH.js gzip | 42 kB | 42 kB | ✓ |
main-HASH.js gzip | 28.4 kB | 28.4 kB | ✓ |
webpack-HASH.js gzip | 1.44 kB | 1.44 kB | ✓ |
Overall change | 72.1 kB | 72.1 kB | ✓ |
Legacy Client Bundles (polyfills)
vercel/next.js canary | martinnabhan/next.js fix/multi-byte-streaming-ssr | Change | |
---|---|---|---|
polyfills-HASH.js gzip | 31 kB | 31 kB | ✓ |
Overall change | 31 kB | 31 kB | ✓ |
Client Pages
vercel/next.js canary | martinnabhan/next.js fix/multi-byte-streaming-ssr | Change | |
---|---|---|---|
_app-HASH.js gzip | 1.36 kB | 1.36 kB | ✓ |
_error-HASH.js gzip | 192 B | 192 B | ✓ |
amp-HASH.js gzip | 309 B | 309 B | ✓ |
css-HASH.js gzip | 327 B | 327 B | ✓ |
dynamic-HASH.js gzip | 2.57 kB | 2.57 kB | ✓ |
head-HASH.js gzip | 351 B | 351 B | ✓ |
hooks-HASH.js gzip | 920 B | 920 B | ✓ |
image-HASH.js gzip | 5.48 kB | 5.48 kB | ✓ |
index-HASH.js gzip | 263 B | 263 B | ✓ |
link-HASH.js gzip | 2.26 kB | 2.26 kB | ✓ |
routerDirect..HASH.js gzip | 320 B | 320 B | ✓ |
script-HASH.js gzip | 387 B | 387 B | ✓ |
withRouter-HASH.js gzip | 319 B | 319 B | ✓ |
85e02e95b279..7e3.css gzip | 107 B | 107 B | ✓ |
Overall change | 15.2 kB | 15.2 kB | ✓ |
Client Build Manifests
vercel/next.js canary | martinnabhan/next.js fix/multi-byte-streaming-ssr | Change | |
---|---|---|---|
_buildManifest.js gzip | 460 B | 460 B | ✓ |
Overall change | 460 B | 460 B | ✓ |
Rendered Page Sizes
vercel/next.js canary | martinnabhan/next.js fix/multi-byte-streaming-ssr | Change | |
---|---|---|---|
index.html gzip | 532 B | 532 B | ✓ |
link.html gzip | 546 B | 546 B | ✓ |
withRouter.html gzip | 526 B | 526 B | ✓ |
Overall change | 1.6 kB | 1.6 kB | ✓ |
Default Build with SWC (Increase detected ⚠️ )
General Overall increase ⚠️
vercel/next.js canary | martinnabhan/next.js fix/multi-byte-streaming-ssr | Change | |
---|---|---|---|
buildDuration | 19.8s | 19.8s | |
buildDurationCached | 6.3s | 6.4s | |
nodeModulesSize | 467 MB | 467 MB |
Page Load Tests Overall increase ✓
vercel/next.js canary | martinnabhan/next.js fix/multi-byte-streaming-ssr | Change | |
---|---|---|---|
/ failed reqs | 0 | 0 | ✓ |
/ total time (seconds) | 3.32 | 3.294 | -0.03 |
/ avg req/sec | 752.97 | 758.94 | +5.97 |
/error-in-render failed reqs | 0 | 0 | ✓ |
/error-in-render total time (seconds) | 1.525 | 1.459 | -0.07 |
/error-in-render avg req/sec | 1639.27 | 1713.5 | +74.23 |
Client Bundles (main, webpack)
vercel/next.js canary | martinnabhan/next.js fix/multi-byte-streaming-ssr | Change | |
---|---|---|---|
925.HASH.js gzip | 178 B | 178 B | ✓ |
framework-HASH.js gzip | 42.3 kB | 42.3 kB | ✓ |
main-HASH.js gzip | 28.8 kB | 28.8 kB | ✓ |
webpack-HASH.js gzip | 1.45 kB | 1.45 kB | ✓ |
Overall change | 72.7 kB | 72.7 kB | ✓ |
Legacy Client Bundles (polyfills)
vercel/next.js canary | martinnabhan/next.js fix/multi-byte-streaming-ssr | Change | |
---|---|---|---|
polyfills-HASH.js gzip | 31 kB | 31 kB | ✓ |
Overall change | 31 kB | 31 kB | ✓ |
Client Pages
vercel/next.js canary | martinnabhan/next.js fix/multi-byte-streaming-ssr | Change | |
---|---|---|---|
_app-HASH.js gzip | 1.35 kB | 1.35 kB | ✓ |
_error-HASH.js gzip | 179 B | 179 B | ✓ |
amp-HASH.js gzip | 313 B | 313 B | ✓ |
css-HASH.js gzip | 324 B | 324 B | ✓ |
dynamic-HASH.js gzip | 2.56 kB | 2.56 kB | ✓ |
head-HASH.js gzip | 351 B | 351 B | ✓ |
hooks-HASH.js gzip | 921 B | 921 B | ✓ |
image-HASH.js gzip | 5.59 kB | 5.59 kB | ✓ |
index-HASH.js gzip | 261 B | 261 B | ✓ |
link-HASH.js gzip | 2.33 kB | 2.33 kB | ✓ |
routerDirect..HASH.js gzip | 322 B | 322 B | ✓ |
script-HASH.js gzip | 388 B | 388 B | ✓ |
withRouter-HASH.js gzip | 317 B | 317 B | ✓ |
85e02e95b279..7e3.css gzip | 107 B | 107 B | ✓ |
Overall change | 15.3 kB | 15.3 kB | ✓ |
Client Build Manifests
vercel/next.js canary | martinnabhan/next.js fix/multi-byte-streaming-ssr | Change | |
---|---|---|---|
_buildManifest.js gzip | 458 B | 458 B | ✓ |
Overall change | 458 B | 458 B | ✓ |
Rendered Page Sizes
vercel/next.js canary | martinnabhan/next.js fix/multi-byte-streaming-ssr | Change | |
---|---|---|---|
index.html gzip | 532 B | 532 B | ✓ |
link.html gzip | 546 B | 546 B | ✓ |
withRouter.html gzip | 527 B | 527 B | ✓ |
Overall change | 1.6 kB | 1.6 kB | ✓ |
Fixes #35720
When using streaming SSR decodeText is called repeatedly with incoming chunks of data.
In that case a multi-byte character may occasionally split between chunks, causing corruption.
By setting the TextDecoder option 'stream' to true, and reusing the same TextDecoder instance,
TextDecoder will memorise “unfinished” characters and decode them when the next chunk comes.
I found out about the stream option here: https://javascript.info/text-decoder