Fix hang up when loading duplicated CSS module chunks #4879
Conversation
This file contains 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
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
9 Ignored Deployments
|
|
This change may fail to build |
🟢 CI successful 🟢Thanks |
alexkirsz
force-pushed
the
alexkirsz/web-1023-race-condition-between-css-chunks-can
branch
from
21c1701
to
a931caf
Compare
sokra
approved these changes
May 9, 2023
alexkirsz
deleted the
alexkirsz/web-1023-race-condition-between-css-chunks-can
branch
NicholasLYang
pushed a commit
to NicholasLYang/turbo
that referenced
this pull request
May 9, 2023
### Description I identified this issue when authoring a test for PostCSS. Here's how it goes: 1. CSS chunk 1 loads with module chunk A and source = runtime. This marks chunk A as an available module chunk. The promise for A is the same promise as for CSS chunk 1, which is resolved in registerChunk because chunk 1 is part of the chunk group of the initial JS chunk. 2. CSS chunk 2 loads with module chunks A and B. Since A is already available, we run into a different branch, where we only try to load B. However, since we're still using source = runtime, we don't actually attempt to load B and instead return a promise to the resolver of B. But B isn't part of the chunk group of the initial JS chunk, so it is never resolved. This hangs up the runtime entirely and breaks page hydration. Instead, we move the "automatically resolving CSS chunks when source = Runtime" from `registerChunk` (which wasn't quite correct either as far as I can tell) into `loadChunk`, which solves this issue. ### Testing Instructions I don't know if it's worth having a test case just for this, but this will technically be covered under the PostCSS test case in vercel/next.js#49463. link WEB-1023
sokra
pushed a commit
to vercel/next.js
that referenced
this pull request
May 10, 2023
### What? - closes WEB-1024. Minor refactoring to avoid explicit AST cloning. Still visitors are using fold though. ### Turbopack changes * vercel/turbo#4869 * vercel/turbo#4879 * vercel/turbo#4881
alexkirsz
added a commit
that referenced
this pull request
May 12, 2023
This reverts commit 422feeb.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
I identified this issue when authoring a test for PostCSS. Here's how it goes:
CSS chunk 1 loads with module chunk A and source = runtime. This marks chunk A as an available module chunk. The promise for A is the same promise as for CSS chunk 1, which is resolved in registerChunk because chunk 1 is part of the chunk group of the initial JS chunk.
CSS chunk 2 loads with module chunks A and B. Since A is already available, we run into a different branch, where we only try to load B. However, since we're still using source = runtime, we don't actually attempt to load B and instead return a promise to the resolver of B. But B isn't part of the chunk group of the initial JS chunk, so it is never resolved.
This hangs up the runtime entirely and breaks page hydration.
Instead, we move the "automatically resolving CSS chunks when source = Runtime" from
registerChunk(which wasn't quite correct either as far as I can tell) intoloadChunk, which solves this issue.Testing Instructions
I don't know if it's worth having a test case just for this, but this will technically be covered under the PostCSS test case in vercel/next.js#49463.
link WEB-1023