-
Notifications
You must be signed in to change notification settings - Fork 27.2k
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
appDir + async server component + css-modules causes syntax error #42466
Comments
I do not seem to be able to reproduce this. Can you try
Is this a hard refresh (eg. F5) or hot reload (eg. editing and saving a file, in that case which one?) |
It still reproduces with Something I just discovered is that it reproduces in Node.js 18 but not Node.js 17 |
This comment was marked as outdated.
This comment was marked as outdated.
Getting this same error Node v18.7.0, next@13.0.1. |
Ok I investigated a bit, so I can confirm that this happens with Node 18 and not Node 16. It does not reproduce on the first hit to the page but on all of them afterwards (refresh or not). What causes the error is that on subsequent page loads, we seem to miss a part of the script payload. what we are suppose to inject
what we end up receiving, hence the unexpected string
we seem to be missing some part of the payload🤔 Will investigate further. next.js/packages/next/compiled/react-dom/cjs/react-dom-server.browser.development.js Line 4630 in 70e7e58
this particular buffer seems to get cleared out at some point, which causes the error |
Found the bug, let's hope I can get facebook/react#25645 merged fast! |
## Edit Went for another approach after talking with @gnoff. The approach is now: - add a dev-only error when a precomputed chunk is too big to be written - suggest to copy it before passing it to `writeChunk` This PR also includes porting the React Float tests to use the browser build of Fizz so that we can test it out on that environment (which is the one used by next). <!-- Thanks for submitting a pull request! We appreciate you spending the time to work on these changes. Please provide enough information so that others can review your pull request. The three fields below are mandatory. Before submitting a pull request, please make sure the following is done: 1. Fork [the repository](https://github.com/facebook/react) and create your branch from `main`. 2. Run `yarn` in the repository root. 3. If you've fixed a bug or added code that should be tested, add tests! 4. Ensure the test suite passes (`yarn test`). Tip: `yarn test --watch TestName` is helpful in development. 5. Run `yarn test --prod` to test in the production environment. It supports the same options as `yarn test`. 6. If you need a debugger, run `yarn debug-test --watch TestName`, open `chrome://inspect`, and press "Inspect". 7. Format your code with [prettier](https://github.com/prettier/prettier) (`yarn prettier`). 8. Make sure your code lints (`yarn lint`). Tip: `yarn linc` to only check changed files. 9. Run the [Flow](https://flowtype.org/) type checks (`yarn flow`). 10. If you haven't already, complete the CLA. Learn more about contributing: https://reactjs.org/docs/how-to-contribute.html --> ## Summary Someone reported [a bug](vercel/next.js#42466) in Next.js that pointed to an issue with Node 18 in the streaming renderer when using importing a CSS module where it only returned a malformed bootstraping script only after loading the page once. After investigating a bit, here's what I found: - when using a CSS module in Next, we go into this code path, which writes the aforementioned bootstrapping script https://github.com/facebook/react/blob/5f7ef8c4cbe824ef126a947b7ae0e1c07b143357/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js#L2443-L2447 - the reason for the malformed script is that `completeBoundaryWithStylesScript1FullBoth` is emptied after the call to `writeChunk` - it gets emptied in `writeChunk` because we stream the chunk directly without copying it in this codepath https://github.com/facebook/react/blob/a438590144d2ad40865b58e0c0e69595fc1aa377/packages/react-server/src/ReactServerStreamConfigBrowser.js#L63 - the reason why it only happens from Node 18 is because the Webstreams APIs are available natively from that version and in their implementation, [`enqueue` transfers the array buffer ownership](https://github.com/nodejs/node/blob/9454ba6138d11e8a4d18b073de25781cad4bd2c8/lib/internal/webstreams/readablestream.js#L2641), thus making it unavailable/empty for subsequent calls. In older Node versions, we don't encounter the bug because we are using a polyfill in Next.js, [which does not implement properly the array buffer transfer behaviour](https://cs.github.com/MattiasBuelens/web-streams-polyfill/blob/d354a7457ca8a24030dbd0a135ee40baed7c774d/src/lib/abstract-ops/ecmascript.ts#L16). I think the proper fix for this is to clone the array buffer before enqueuing it. (we do this in the other code paths in the function later on, see ```((currentView: any): Uint8Array).set(bytesToWrite, writtenBytes);``` ## How did you test this change? Manually tested by applying the change in the compiled Next.js version. <!-- Demonstrate the code is solid. Example: The exact commands you ran and their output, screenshots / videos if the pull request changes the user interface. How exactly did you verify that your PR solves the issue you wanted to solve? If you leave this empty, your PR will very likely be closed. --> Co-authored-by: Sebastian Markbage <sebastian@calyptus.eu>
Update pre-compiled react to fix the streaming issue with node 18. x-ref: facebook/react#25645 Fixes: #42466 Manually tested locally works for the reproduction from the issue ## Bug - [x] Related issues linked using `fixes #number` - [ ] Integration tests added - [ ] Errors have a helpful link attached, see [`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md)
…k#25645) ## Edit Went for another approach after talking with @gnoff. The approach is now: - add a dev-only error when a precomputed chunk is too big to be written - suggest to copy it before passing it to `writeChunk` This PR also includes porting the React Float tests to use the browser build of Fizz so that we can test it out on that environment (which is the one used by next). <!-- Thanks for submitting a pull request! We appreciate you spending the time to work on these changes. Please provide enough information so that others can review your pull request. The three fields below are mandatory. Before submitting a pull request, please make sure the following is done: 1. Fork [the repository](https://github.com/facebook/react) and create your branch from `main`. 2. Run `yarn` in the repository root. 3. If you've fixed a bug or added code that should be tested, add tests! 4. Ensure the test suite passes (`yarn test`). Tip: `yarn test --watch TestName` is helpful in development. 5. Run `yarn test --prod` to test in the production environment. It supports the same options as `yarn test`. 6. If you need a debugger, run `yarn debug-test --watch TestName`, open `chrome://inspect`, and press "Inspect". 7. Format your code with [prettier](https://github.com/prettier/prettier) (`yarn prettier`). 8. Make sure your code lints (`yarn lint`). Tip: `yarn linc` to only check changed files. 9. Run the [Flow](https://flowtype.org/) type checks (`yarn flow`). 10. If you haven't already, complete the CLA. Learn more about contributing: https://reactjs.org/docs/how-to-contribute.html --> ## Summary Someone reported [a bug](vercel/next.js#42466) in Next.js that pointed to an issue with Node 18 in the streaming renderer when using importing a CSS module where it only returned a malformed bootstraping script only after loading the page once. After investigating a bit, here's what I found: - when using a CSS module in Next, we go into this code path, which writes the aforementioned bootstrapping script https://github.com/facebook/react/blob/5f7ef8c4cbe824ef126a947b7ae0e1c07b143357/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js#L2443-L2447 - the reason for the malformed script is that `completeBoundaryWithStylesScript1FullBoth` is emptied after the call to `writeChunk` - it gets emptied in `writeChunk` because we stream the chunk directly without copying it in this codepath https://github.com/facebook/react/blob/a438590144d2ad40865b58e0c0e69595fc1aa377/packages/react-server/src/ReactServerStreamConfigBrowser.js#L63 - the reason why it only happens from Node 18 is because the Webstreams APIs are available natively from that version and in their implementation, [`enqueue` transfers the array buffer ownership](https://github.com/nodejs/node/blob/9454ba6138d11e8a4d18b073de25781cad4bd2c8/lib/internal/webstreams/readablestream.js#L2641), thus making it unavailable/empty for subsequent calls. In older Node versions, we don't encounter the bug because we are using a polyfill in Next.js, [which does not implement properly the array buffer transfer behaviour](https://cs.github.com/MattiasBuelens/web-streams-polyfill/blob/d354a7457ca8a24030dbd0a135ee40baed7c774d/src/lib/abstract-ops/ecmascript.ts#L16). I think the proper fix for this is to clone the array buffer before enqueuing it. (we do this in the other code paths in the function later on, see ```((currentView: any): Uint8Array).set(bytesToWrite, writtenBytes);``` ## How did you test this change? Manually tested by applying the change in the compiled Next.js version. <!-- Demonstrate the code is solid. Example: The exact commands you ran and their output, screenshots / videos if the pull request changes the user interface. How exactly did you verify that your PR solves the issue you wanted to solve? If you leave this empty, your PR will very likely be closed. --> Co-authored-by: Sebastian Markbage <sebastian@calyptus.eu>
…k#25645) ## Edit Went for another approach after talking with @gnoff. The approach is now: - add a dev-only error when a precomputed chunk is too big to be written - suggest to copy it before passing it to `writeChunk` This PR also includes porting the React Float tests to use the browser build of Fizz so that we can test it out on that environment (which is the one used by next). <!-- Thanks for submitting a pull request! We appreciate you spending the time to work on these changes. Please provide enough information so that others can review your pull request. The three fields below are mandatory. Before submitting a pull request, please make sure the following is done: 1. Fork [the repository](https://github.com/facebook/react) and create your branch from `main`. 2. Run `yarn` in the repository root. 3. If you've fixed a bug or added code that should be tested, add tests! 4. Ensure the test suite passes (`yarn test`). Tip: `yarn test --watch TestName` is helpful in development. 5. Run `yarn test --prod` to test in the production environment. It supports the same options as `yarn test`. 6. If you need a debugger, run `yarn debug-test --watch TestName`, open `chrome://inspect`, and press "Inspect". 7. Format your code with [prettier](https://github.com/prettier/prettier) (`yarn prettier`). 8. Make sure your code lints (`yarn lint`). Tip: `yarn linc` to only check changed files. 9. Run the [Flow](https://flowtype.org/) type checks (`yarn flow`). 10. If you haven't already, complete the CLA. Learn more about contributing: https://reactjs.org/docs/how-to-contribute.html --> ## Summary Someone reported [a bug](vercel/next.js#42466) in Next.js that pointed to an issue with Node 18 in the streaming renderer when using importing a CSS module where it only returned a malformed bootstraping script only after loading the page once. After investigating a bit, here's what I found: - when using a CSS module in Next, we go into this code path, which writes the aforementioned bootstrapping script https://github.com/facebook/react/blob/5f7ef8c4cbe824ef126a947b7ae0e1c07b143357/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js#L2443-L2447 - the reason for the malformed script is that `completeBoundaryWithStylesScript1FullBoth` is emptied after the call to `writeChunk` - it gets emptied in `writeChunk` because we stream the chunk directly without copying it in this codepath https://github.com/facebook/react/blob/a438590144d2ad40865b58e0c0e69595fc1aa377/packages/react-server/src/ReactServerStreamConfigBrowser.js#L63 - the reason why it only happens from Node 18 is because the Webstreams APIs are available natively from that version and in their implementation, [`enqueue` transfers the array buffer ownership](https://github.com/nodejs/node/blob/9454ba6138d11e8a4d18b073de25781cad4bd2c8/lib/internal/webstreams/readablestream.js#L2641), thus making it unavailable/empty for subsequent calls. In older Node versions, we don't encounter the bug because we are using a polyfill in Next.js, [which does not implement properly the array buffer transfer behaviour](https://cs.github.com/MattiasBuelens/web-streams-polyfill/blob/d354a7457ca8a24030dbd0a135ee40baed7c774d/src/lib/abstract-ops/ecmascript.ts#L16). I think the proper fix for this is to clone the array buffer before enqueuing it. (we do this in the other code paths in the function later on, see ```((currentView: any): Uint8Array).set(bytesToWrite, writtenBytes);``` ## How did you test this change? Manually tested by applying the change in the compiled Next.js version. <!-- Demonstrate the code is solid. Example: The exact commands you ran and their output, screenshots / videos if the pull request changes the user interface. How exactly did you verify that your PR solves the issue you wanted to solve? If you leave this empty, your PR will very likely be closed. --> Co-authored-by: Sebastian Markbage <sebastian@calyptus.eu>
…k#25645) ## Edit Went for another approach after talking with @gnoff. The approach is now: - add a dev-only error when a precomputed chunk is too big to be written - suggest to copy it before passing it to `writeChunk` This PR also includes porting the React Float tests to use the browser build of Fizz so that we can test it out on that environment (which is the one used by next). <!-- Thanks for submitting a pull request! We appreciate you spending the time to work on these changes. Please provide enough information so that others can review your pull request. The three fields below are mandatory. Before submitting a pull request, please make sure the following is done: 1. Fork [the repository](https://github.com/facebook/react) and create your branch from `main`. 2. Run `yarn` in the repository root. 3. If you've fixed a bug or added code that should be tested, add tests! 4. Ensure the test suite passes (`yarn test`). Tip: `yarn test --watch TestName` is helpful in development. 5. Run `yarn test --prod` to test in the production environment. It supports the same options as `yarn test`. 6. If you need a debugger, run `yarn debug-test --watch TestName`, open `chrome://inspect`, and press "Inspect". 7. Format your code with [prettier](https://github.com/prettier/prettier) (`yarn prettier`). 8. Make sure your code lints (`yarn lint`). Tip: `yarn linc` to only check changed files. 9. Run the [Flow](https://flowtype.org/) type checks (`yarn flow`). 10. If you haven't already, complete the CLA. Learn more about contributing: https://reactjs.org/docs/how-to-contribute.html --> ## Summary Someone reported [a bug](vercel/next.js#42466) in Next.js that pointed to an issue with Node 18 in the streaming renderer when using importing a CSS module where it only returned a malformed bootstraping script only after loading the page once. After investigating a bit, here's what I found: - when using a CSS module in Next, we go into this code path, which writes the aforementioned bootstrapping script https://github.com/facebook/react/blob/5f7ef8c4cbe824ef126a947b7ae0e1c07b143357/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js#L2443-L2447 - the reason for the malformed script is that `completeBoundaryWithStylesScript1FullBoth` is emptied after the call to `writeChunk` - it gets emptied in `writeChunk` because we stream the chunk directly without copying it in this codepath https://github.com/facebook/react/blob/a438590144d2ad40865b58e0c0e69595fc1aa377/packages/react-server/src/ReactServerStreamConfigBrowser.js#L63 - the reason why it only happens from Node 18 is because the Webstreams APIs are available natively from that version and in their implementation, [`enqueue` transfers the array buffer ownership](https://github.com/nodejs/node/blob/9454ba6138d11e8a4d18b073de25781cad4bd2c8/lib/internal/webstreams/readablestream.js#L2641), thus making it unavailable/empty for subsequent calls. In older Node versions, we don't encounter the bug because we are using a polyfill in Next.js, [which does not implement properly the array buffer transfer behaviour](https://cs.github.com/MattiasBuelens/web-streams-polyfill/blob/d354a7457ca8a24030dbd0a135ee40baed7c774d/src/lib/abstract-ops/ecmascript.ts#L16). I think the proper fix for this is to clone the array buffer before enqueuing it. (we do this in the other code paths in the function later on, see ```((currentView: any): Uint8Array).set(bytesToWrite, writtenBytes);``` ## How did you test this change? Manually tested by applying the change in the compiled Next.js version. <!-- Demonstrate the code is solid. Example: The exact commands you ran and their output, screenshots / videos if the pull request changes the user interface. How exactly did you verify that your PR solves the issue you wanted to solve? If you leave this empty, your PR will very likely be closed. --> Co-authored-by: Sebastian Markbage <sebastian@calyptus.eu>
This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you. |
Verify canary release
Provide environment information
What browser are you using? (if relevant)
Chrome, Safari
How are you deploying your application? (if relevant)
next dev
Describe the Bug
When using the experimental /app mode with the following setup, RSC seems to stream in a malformed script tag, causing a crash:
layout.tsx
and aloading.tsx
page.tsx
exports an async server componentpage.tsx
uses a class name from a CSS module,styles.module.css
Unexpected string literal ",". Parse error.
See minimal repro in code here: mxmul/next13-async-component-css-modules-repro@c562d4d
Expected Behavior
Page should render the loading state immediately, then the loaded content.
e.g.
Link to reproduction
https://github.com/mxmul/next13-async-component-css-modules-repro
To Reproduce
Clone the repo, and run
yarn dev
.The first time you load the page, it seems to render properly. You see a loading state, then the rendered page:
After refreshing the page, you should see next crash with a syntax error:
The text was updated successfully, but these errors were encountered: