Skip to content
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 hang up when loading duplicated CSS module chunks #4879

Merged

Conversation

alexkirsz
Copy link
Contributor

@alexkirsz alexkirsz commented 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

@alexkirsz alexkirsz requested a review from a team as a code owner May 9, 2023 13:57
@vercel
Copy link

vercel bot commented May 9, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
examples-kitchensink-blog 🔄 Building (Inspect) May 9, 2023 2:36pm
examples-svelte-web 🔄 Building (Inspect) May 9, 2023 2:36pm
9 Ignored Deployments
Name Status Preview Comments Updated (UTC)
examples-basic-web ⬜️ Ignored (Inspect) May 9, 2023 2:36pm
examples-cra-web ⬜️ Ignored (Inspect) May 9, 2023 2:36pm
examples-designsystem-docs ⬜️ Ignored (Inspect) May 9, 2023 2:36pm
examples-gatsby-web ⬜️ Ignored (Inspect) May 9, 2023 2:36pm
examples-native-web ⬜️ Ignored (Inspect) May 9, 2023 2:36pm
examples-nonmonorepo ⬜️ Ignored (Inspect) May 9, 2023 2:36pm
examples-tailwind-web ⬜️ Ignored (Inspect) May 9, 2023 2:36pm
examples-vite-web ⬜️ Ignored (Inspect) May 9, 2023 2:36pm
turbo-site ⬜️ Ignored (Inspect) Visit Preview May 9, 2023 2:36pm

@github-actions
Copy link
Contributor

github-actions bot commented May 9, 2023

This change may fail to build next-swc.

info: syncing channel updates for 'nightly-2023-03-09-x86_64-unknown-linux-gnu'
info: latest update on 2023-03-09, rust version 1.70.0-nightly (900c35403 2023-03-08)
info: downloading component 'cargo'
info: downloading component 'rust-std'
info: downloading component 'rustc'
info: installing component 'cargo'
info: installing component 'rust-std'
info: installing component 'rustc'
packages/next-swc/crates/next-core/src/next_client/context.rs:197:9: warning: use of deprecated field `turbo_binding::turbopack::turbopack::module_options::ModuleOptionsContext::custom_ecmascript_transforms`: use custom_ecma_transform_plugins instead
packages/next-swc/crates/next-core/src/next_server/context.rs:323:17: warning: use of deprecated field `turbo_binding::turbopack::turbopack::module_options::ModuleOptionsContext::custom_ecmascript_transforms`: use custom_ecma_transform_plugins instead
packages/next-swc/crates/next-core/src/next_server/context.rs:362:17: warning: use of deprecated field `turbo_binding::turbopack::turbopack::module_options::ModuleOptionsContext::custom_ecmascript_transforms`: use custom_ecma_transform_plugins instead
packages/next-swc/crates/next-core/src/next_shared/transforms.rs:73:17: error[E0195]: lifetime parameters or bounds on method `transform` do not match the trait declaration
packages/next-swc/crates/next-core/src/next_shared/transforms.rs:140:17: error[E0195]: lifetime parameters or bounds on method `transform` do not match the trait declaration
packages/next-swc/crates/next-core/src/next_shared/transforms.rs:179:17: error[E0195]: lifetime parameters or bounds on method `transform` do not match the trait declaration
packages/next-swc/crates/next-core/src/next_shared/transforms.rs:270:17: error[E0195]: lifetime parameters or bounds on method `transform` do not match the trait declaration
error: could not compile `next-core` (lib) due to 4 previous errors; 3 warnings emitted

@github-actions
Copy link
Contributor

github-actions bot commented May 9, 2023

🟢 CI successful 🟢

Thanks

@alexkirsz alexkirsz force-pushed the alexkirsz/web-1023-race-condition-between-css-chunks-can branch from 21c1701 to a931caf Compare May 9, 2023 14:34
@alexkirsz alexkirsz enabled auto-merge (squash) May 9, 2023 14:34
@alexkirsz alexkirsz merged commit 422feeb into main May 9, 2023
@alexkirsz alexkirsz deleted the alexkirsz/web-1023-race-condition-between-css-chunks-can branch May 9, 2023 14:52
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/turborepo#4869
* vercel/turborepo#4879
* vercel/turborepo#4881
alexkirsz added a commit that referenced this pull request May 12, 2023
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Jul 25, 2024
…po#4879)

### 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
#49463.
link WEB-1023
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Jul 29, 2024
…po#4879)

### 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
#49463.
link WEB-1023
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Aug 1, 2024
…po#4879)

### 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
#49463.
link WEB-1023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants