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

remove ChunkingType::Separate #5419

Merged
merged 3 commits into from
Jul 4, 2023
Merged

Conversation

sokra
Copy link
Member

@sokra sokra commented Jun 29, 2023

Description

We don't need that.
That simplifies code

Also renames SeparateAsync to Async to simplify the name a bit

Testing Instructions

CI

@sokra sokra requested a review from a team as a code owner June 29, 2023 07:42
@vercel
Copy link

vercel bot commented Jun 29, 2023

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

Name Status Preview Comments Updated (UTC)
examples-basic-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 3, 2023 8:37am
examples-cra-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 3, 2023 8:37am
examples-designsystem-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 3, 2023 8:37am
examples-gatsby-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 3, 2023 8:37am
examples-kitchensink-blog ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 3, 2023 8:37am
examples-native-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 3, 2023 8:37am
examples-nonmonorepo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 3, 2023 8:37am
examples-svelte-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 3, 2023 8:37am
examples-tailwind-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 3, 2023 8:38am
examples-vite-web 🔄 Building (Inspect) Visit Preview 💬 Add feedback Jul 3, 2023 8:38am
turbo-site ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 3, 2023 8:38am

@github-actions
Copy link
Contributor

github-actions bot commented Jun 29, 2023

⚠️ This change may fail to build next-swc.

Logs

packages/next-swc/crates/next-core/src/app_source.rs:20:27: �[38;5;9merror[E0432]: unresolved import `turbopack_binding::turbopack::core::environment::EnvironmentIntention`
packages/next-swc/crates/next-core/src/next_client/context.rs:19:37: �[38;5;9merror[E0432]: unresolved import `turbopack_binding::turbopack::core::environment::EnvironmentIntention`
packages/next-swc/crates/next-core/src/next_edge/context.rs:13:40: �[38;5;9merror[E0432]: unresolved import `turbopack_binding::turbopack::core::environment::EnvironmentIntention`
packages/next-swc/crates/next-core/src/next_server/context.rs:14:17: �[38;5;9merror[E0432]: unresolved import `turbopack_binding::turbopack::core::environment::EnvironmentIntention`
packages/next-swc/crates/next-core/src/page_source.rs:19:27: �[38;5;9merror[E0432]: unresolved import `turbopack_binding::turbopack::core::environment::EnvironmentIntention`
packages/next-swc/crates/next-core/src/router.rs:23:27: �[38;5;9merror[E0432]: unresolved import `turbopack_binding::turbopack::core::environment::EnvironmentIntention`
packages/next-swc/crates/next-core/src/web_entry_source.rs:16:37: �[38;5;9merror[E0432]: unresolved import `turbopack_binding::turbopack::core::environment::EnvironmentIntention`
packages/next-swc/crates/next-core/src/next_client/context.rs:110:30: �[38;5;9merror[E0061]: this function takes 1 argument but 2 arguments were supplied
packages/next-swc/crates/next-core/src/next_edge/context.rs:69:30: �[38;5;9merror[E0061]: this function takes 1 argument but 2 arguments were supplied
packages/next-swc/crates/next-core/src/next_server/context.rs:253:30: �[38;5;9merror[E0061]: this function takes 1 argument but 2 arguments were supplied
packages/next-swc/crates/next-core/src/web_entry_source.rs:65:30: �[38;5;9merror[E0061]: this function takes 1 argument but 2 arguments were supplied
error: could not compile `next-core` (lib) due to 11 previous errors

See job summary for details

@github-actions
Copy link
Contributor

github-actions bot commented Jun 29, 2023

🟢 CI successful 🟢

Thanks

@github-actions
Copy link
Contributor

Linux Benchmark for 65aa710

Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 8031.64µs ± 23.31µs 8158.32µs ± 35.94µs +1.58% +0.10%
Click to view full benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 8031.64µs ± 23.31µs 8158.32µs ± 35.94µs +1.58% +0.10%
bench_hmr_to_eval/Turbopack CSR/1000 modules 7486.45µs ± 38.77µs 7580.19µs ± 167.33µs +1.25%
bench_startup/Turbopack CSR/1000 modules 934.60ms ± 3.14ms 948.92ms ± 5.49ms +1.53%

@sokra sokra requested a review from alexkirsz June 29, 2023 08:05
fn chunking_type(&self) -> Result<ChunkingTypeOptionVc> {
Ok(ChunkingTypeOptionVc::cell(Some(ChunkingType::Separate)))
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you verified that this still produces a single CSS chunk for each source CSS asset?

Copy link
Member Author

@sokra sokra Jun 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't, I guess the tests do that.

But the SingleItemCssChunkReference is used in references from CssChunk, which is in the output graph, where ChunkableAssetReference is not used at all. ChunkableAssetReference is only used in the module graph while determining the chunking.

@github-actions
Copy link
Contributor

MacOS Benchmark for 65aa710

Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 27.81ms ± 0.19ms 27.06ms ± 0.08ms -2.68% -0.69%
bench_hmr_to_eval/Turbopack CSR/1000 modules 26.22ms ± 0.07ms 26.91ms ± 0.06ms +2.62% +1.62%
Click to view full benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 27.81ms ± 0.19ms 27.06ms ± 0.08ms -2.68% -0.69%
bench_hmr_to_eval/Turbopack CSR/1000 modules 26.22ms ± 0.07ms 26.91ms ± 0.06ms +2.62% +1.62%
bench_startup/Turbopack CSR/1000 modules 3300.83ms ± 19.37ms 3451.19ms ± 93.44ms +4.56%

@sokra sokra requested a review from alexkirsz June 29, 2023 12:07
@sokra sokra enabled auto-merge (squash) June 29, 2023 13:20
@github-actions
Copy link
Contributor

github-actions bot commented Jul 3, 2023

Linux Benchmark for e412035

Click to view benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 7081.59µs ± 18.07µs 7115.00µs ± 34.80µs +0.47%
bench_hmr_to_eval/Turbopack CSR/1000 modules 6614.32µs ± 60.65µs 6583.43µs ± 29.69µs -0.47%
bench_startup/Turbopack CSR/1000 modules 934.35ms ± 2.45ms 936.30ms ± 4.28ms +0.21%

@sokra sokra merged commit e528ac2 into main Jul 4, 2023
@sokra sokra deleted the sokra/remove-separate-chunking-type branch July 4, 2023 11:33
kodiakhq bot pushed a commit to vercel/next.js that referenced this pull request Jul 4, 2023
shuding pushed a commit to vercel/next.js that referenced this pull request Jul 8, 2023
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Jul 25, 2024
### Description

We don't need that.
That simplifies code

Also renames SeparateAsync to Async to simplify the name a bit

### Testing Instructions

CI
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Jul 29, 2024
### Description

We don't need that.
That simplifies code

Also renames SeparateAsync to Async to simplify the name a bit

### Testing Instructions

CI
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Aug 1, 2024
### Description

We don't need that.
That simplifies code

Also renames SeparateAsync to Async to simplify the name a bit

### Testing Instructions

CI
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