Chunking Refactor pt. 2 #4450

alexkirsz · 2023-04-04T15:28:10Z

Description

Next.js side: vercel/next.js#47961

Previous episode: #4397

In this episode, we:

Get rid of ChunkGroupVc. ChunkGroupVc previously took care of:
a. optimizing chunks;
b. adding additional chunks, such as the "evaluate" chunk, which evaluates runtime entries.
Both of these use cases are a concern of the chunking context (i.e. it would differ between dev and build), hence they're now handled by the chunking context.
Move the chunk list registration logic into a different chunk. The chunk list was previously a bit leaky as it required the Next.js code to instantiate it manually. There were also a bunch of other instantiations in the code which differed ever so slightly. The chunk list registration code is now part of a separate chunk, which is only added by the dev chunking context.

Testing Instructions

Snapshots + Next dev tests + Manual tests
fix WEB-822

vercel · 2023-04-04T15:29:21Z

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

Name	Status	Preview	Comments	Updated (UTC)
examples-basic-web	🔄 Building (Inspect)			Apr 11, 2023 1:14pm
examples-designsystem-docs	🔄 Building (Inspect)			Apr 11, 2023 1:14pm
examples-gatsby-web	🔄 Building (Inspect)			Apr 11, 2023 1:14pm
examples-svelte-web	🔄 Building (Inspect)			Apr 11, 2023 1:14pm
turbo-site	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Apr 11, 2023 1:14pm

6 Ignored Deployments

Name	Status	Updated (UTC)
examples-cra-web	⬜️ Ignored (Inspect)	Apr 11, 2023 1:14pm
examples-kitchensink-blog	⬜️ Ignored (Inspect)	Apr 11, 2023 1:14pm
examples-native-web	⬜️ Ignored (Inspect)	Apr 11, 2023 1:14pm
examples-nonmonorepo	⬜️ Ignored (Inspect)	Apr 11, 2023 1:14pm
examples-tailwind-web	⬜️ Ignored (Inspect)	Apr 11, 2023 1:14pm
examples-vite-web	⬜️ Ignored (Inspect)	Apr 11, 2023 1:14pm

github-actions · 2023-04-04T15:44:49Z

🟢 CI successful 🟢

Thanks

github-actions · 2023-04-05T13:18:23Z

✅ This changes can build next-swc

sokra

Let's wait until after the next.js release before merging that

crates/turbopack-core/src/chunk/mod.rs

sokra · 2023-04-05T15:42:28Z

crates/turbopack-core/src/chunk/mod.rs

+    pub fn new(chunks: AssetsVc) -> Self {
+        Self::cell(ChunkGroupReference { chunks })
    }
 }

 #[turbo_tasks::value_impl]
 impl AssetReference for ChunkGroupReference {
    #[turbo_tasks::function]
    async fn resolve_reference(&self) -> Result<ResolveResultVc> {
-        let set = self.chunk_group.chunks().await?.clone_value();
+        let set = self.chunks.await?.clone_value();


This is eager instead of lazy now...

If I'm not mistaken, this specific line was always eager. However, creating a chunk group reference was not.

crates/turbopack-core/src/chunk/mod.rs

sokra

Great to see all these __turbopack_register_chunk_list__() calls to be gone

sokra · 2023-04-11T13:53:31Z

crates/turbopack-dev/src/chunking_context.rs

+            if let Some(ecmascript_chunk) = EcmascriptChunkVc::resolve_from(chunk).await? {
+                EcmascriptDevChunkVc::new(self_vc, ecmascript_chunk).into()
+            } else {
+                chunk.into()


This only works when trait Chunk : Asset. Don't you want to remove this?

Yes, this will eventually have to go. But for now, in order to keep supporting CSS chunks without modifications, we have to do this.

sokra · 2023-04-11T14:02:51Z

crates/turbopack-dev/src/chunking_context.rs

+        assets.push(self_vc.generate_chunk_list_register_chunk(
+            entry_chunk,
+            AssetsVc::cell(assets.clone()),
+            Value::new(EcmascriptDevChunkListSource::Dynamic),
+        ));


This looks like a race condition. chunk list is registered maybe before or maybe after the chunk are loaded. It should probably register it before the chunks are loaded and also wait for ack from the dev server. But this is already broken, so we can ignore that for now... We can solve that together with solving the initial registration

sokra · 2023-04-11T14:06:42Z

crates/turbopack-dev/src/chunking_context.rs

+    Ok(parent
+        .references()
+        .await?
+        .iter()
+        .copied()
+        .map(reference_to_chunks)
+        .try_join()
+        .await?
+        .into_iter()
+        .flatten())


The Chunk trait should have a parallel_chunks() method to expose parallel chunks. references() should no longer contain parallel chunks

sokra · 2023-04-11T14:17:25Z

crates/turbopack-ecmascript/src/chunk/optimize.rs

-    children: Vec<ChunksVc>,
-    _chunk_group: ChunkGroupVc,
-) -> Result<ChunksVc> {
+async fn optimize_ecmascript(local: Option<ChunksVc>, children: Vec<ChunksVc>) -> Result<ChunksVc> {


This function might move into turbopack-dev

At this point, the whole optimize module should move into turbopack-dev.

alexkirsz · 2023-04-11T15:15:21Z

The remaining comments will be addressed in pt. 3.

See vercel/turborepo#4450 This PR updates Turbopack to turbopack-230411.2: ## Miscellaneous * vercel/turborepo#4450

### Description This PR does two things, addressing related comments from #4450: 1. Replace `ChunkReferenceVc` and `ParallelChunkReferenceVc` with `Chunk::parallel_chunks`. 2. Moves all non-generic optimization logic out of `turbopack-core` and into `turbopack-dev`. These optimization methods are very specific to a development environment, and `turbopack-build` will have different ones. ### Testing Instructions Snapshots link WEB-891

…el#4546) ### Description This PR does two things, addressing related comments from vercel#4450: 1. Replace `ChunkReferenceVc` and `ParallelChunkReferenceVc` with `Chunk::parallel_chunks`. 2. Moves all non-generic optimization logic out of `turbopack-core` and into `turbopack-dev`. These optimization methods are very specific to a development environment, and `turbopack-build` will have different ones. ### Testing Instructions Snapshots link WEB-891

### Description Next.js side: #47961 Previous episode: vercel/turborepo#4397 In this episode, we: 1. Get rid of `ChunkGroupVc`. `ChunkGroupVc` previously took care of: a. optimizing chunks; b. adding additional chunks, such as the "evaluate" chunk, which evaluates runtime entries. Both of these use cases are a concern of the chunking context (i.e. it would differ between dev and build), hence they're now handled by the chunking context. 2. Move the chunk list registration logic into a different chunk. The chunk list was previously a bit leaky as it required the Next.js code to instantiate it manually. There were also a bunch of other instantiations in the code which differed ever so slightly. The chunk list registration code is now part of a separate chunk, which is only added by the dev chunking context. ### Testing Instructions Snapshots + Next dev tests + Manual tests fix WEB-822

…el/turborepo#4546) ### Description This PR does two things, addressing related comments from vercel/turborepo#4450: 1. Replace `ChunkReferenceVc` and `ParallelChunkReferenceVc` with `Chunk::parallel_chunks`. 2. Moves all non-generic optimization logic out of `turbopack-core` and into `turbopack-dev`. These optimization methods are very specific to a development environment, and `turbopack-build` will have different ones. ### Testing Instructions Snapshots link WEB-891

### Description Next.js side: #47961 Previous episode: vercel/turborepo#4397 In this episode, we: 1. Get rid of `ChunkGroupVc`. `ChunkGroupVc` previously took care of: a. optimizing chunks; b. adding additional chunks, such as the "evaluate" chunk, which evaluates runtime entries. Both of these use cases are a concern of the chunking context (i.e. it would differ between dev and build), hence they're now handled by the chunking context. 2. Move the chunk list registration logic into a different chunk. The chunk list was previously a bit leaky as it required the Next.js code to instantiate it manually. There were also a bunch of other instantiations in the code which differed ever so slightly. The chunk list registration code is now part of a separate chunk, which is only added by the dev chunking context. ### Testing Instructions Snapshots + Next dev tests + Manual tests fix WEB-822

…el/turborepo#4546) ### Description This PR does two things, addressing related comments from vercel/turborepo#4450: 1. Replace `ChunkReferenceVc` and `ParallelChunkReferenceVc` with `Chunk::parallel_chunks`. 2. Moves all non-generic optimization logic out of `turbopack-core` and into `turbopack-dev`. These optimization methods are very specific to a development environment, and `turbopack-build` will have different ones. ### Testing Instructions Snapshots link WEB-891

### Description Next.js side: #47961 Previous episode: vercel/turborepo#4397 In this episode, we: 1. Get rid of `ChunkGroupVc`. `ChunkGroupVc` previously took care of: a. optimizing chunks; b. adding additional chunks, such as the "evaluate" chunk, which evaluates runtime entries. Both of these use cases are a concern of the chunking context (i.e. it would differ between dev and build), hence they're now handled by the chunking context. 2. Move the chunk list registration logic into a different chunk. The chunk list was previously a bit leaky as it required the Next.js code to instantiate it manually. There were also a bunch of other instantiations in the code which differed ever so slightly. The chunk list registration code is now part of a separate chunk, which is only added by the dev chunking context. ### Testing Instructions Snapshots + Next dev tests + Manual tests fix WEB-822

…el/turborepo#4546) ### Description This PR does two things, addressing related comments from vercel/turborepo#4450: 1. Replace `ChunkReferenceVc` and `ParallelChunkReferenceVc` with `Chunk::parallel_chunks`. 2. Moves all non-generic optimization logic out of `turbopack-core` and into `turbopack-dev`. These optimization methods are very specific to a development environment, and `turbopack-build` will have different ones. ### Testing Instructions Snapshots link WEB-891

turbo-orchestrator bot added the team: turbopack label Apr 4, 2023

alexkirsz force-pushed the alexkirsz/web-822-chunk-refactor-2 branch from 75b76ba to 197daa9 Compare April 4, 2023 15:33

alexkirsz force-pushed the alexkirsz/web-822-chunk-refactor-2 branch from 197daa9 to c680de9 Compare April 5, 2023 13:12

alexkirsz mentioned this pull request Apr 5, 2023

Chunking Refactor pt. 2 vercel/next.js#47961

Merged

alexkirsz marked this pull request as ready for review April 5, 2023 13:19

alexkirsz requested a review from a team as a code owner April 5, 2023 13:19

alexkirsz force-pushed the alexkirsz/web-822-chunk-refactor-2 branch from 624bba3 to 1718e24 Compare April 5, 2023 13:28

vercel bot deployed to Preview – turbo-site April 5, 2023 13:29 View deployment

sokra requested changes Apr 5, 2023

View reviewed changes

alexkirsz force-pushed the alexkirsz/web-822-chunk-refactor-2 branch from 4fb5f34 to fadfc43 Compare April 6, 2023 09:20

vercel bot deployed to Preview – turbo-site April 6, 2023 09:22 View deployment

alexkirsz added 3 commits April 11, 2023 15:09

Chunking Refactor pt. 2

476052f

Make chunk group lazy again

ba0d972

AsyncChunkGroup { entry }

84d3130

alexkirsz force-pushed the alexkirsz/web-822-chunk-refactor-2 branch from fadfc43 to 84d3130 Compare April 11, 2023 13:13

vercel bot deployed to Preview – turbo-site April 11, 2023 13:14 View deployment

sokra approved these changes Apr 11, 2023

View reviewed changes

alexkirsz merged commit 8a6c4b8 into main Apr 11, 2023

alexkirsz deleted the alexkirsz/web-822-chunk-refactor-2 branch April 11, 2023 15:15

alexkirsz mentioned this pull request Apr 12, 2023

Chunking Context Refactor pt. 3: Address PR comments from pt. 2 #4546

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Chunking Refactor pt. 2 #4450

Chunking Refactor pt. 2 #4450

alexkirsz commented Apr 4, 2023 •

edited

Loading

vercel bot commented Apr 4, 2023 •

edited

Loading

github-actions bot commented Apr 4, 2023 •

edited

Loading

github-actions bot commented Apr 5, 2023

sokra left a comment

sokra Apr 5, 2023

alexkirsz Apr 6, 2023

sokra left a comment

sokra Apr 11, 2023

alexkirsz Apr 12, 2023

sokra Apr 11, 2023

sokra Apr 11, 2023

sokra Apr 11, 2023

alexkirsz Apr 12, 2023

alexkirsz commented Apr 11, 2023

Chunking Refactor pt. 2 #4450

Chunking Refactor pt. 2 #4450

Conversation

alexkirsz commented Apr 4, 2023 • edited Loading

Description

Testing Instructions

vercel bot commented Apr 4, 2023 • edited Loading

github-actions bot commented Apr 4, 2023 • edited Loading

🟢 CI successful 🟢

github-actions bot commented Apr 5, 2023

sokra left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sokra left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexkirsz commented Apr 11, 2023

alexkirsz commented Apr 4, 2023 •

edited

Loading

vercel bot commented Apr 4, 2023 •

edited

Loading

github-actions bot commented Apr 4, 2023 •

edited

Loading