change source map handling to not invalidate on file change #2474

sokra · 2022-10-28T22:34:12Z

No description provided.

vercel · 2022-10-28T22:34:15Z

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

Name	Status	Preview	Comments	Updated
examples-designsystem-docs	🔄 Building (Inspect)			Oct 29, 2022 at 7:44AM (UTC)
turbo-site	✅ Ready (Inspect)	Visit Preview	💬 Add your feedback	Oct 29, 2022 at 7:44AM (UTC)

4 Ignored Deployments

Name	Status	Updated
examples-basic-web	⬜️ Ignored (Inspect)	Oct 29, 2022 at 7:44AM (UTC)
examples-kitchensink-blog	⬜️ Ignored (Inspect)	Oct 29, 2022 at 7:44AM (UTC)
examples-native-web	⬜️ Ignored (Inspect)	Oct 29, 2022 at 7:44AM (UTC)
examples-svelte-web	⬜️ Ignored (Inspect)	Oct 29, 2022 at 7:44AM (UTC)

jridgewell · 2022-10-28T22:56:01Z

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

@@ -845,6 +845,16 @@ impl ValueToString for EcmascriptChunk {

 #[turbo_tasks::value_impl]
 impl EcmascriptChunkVc {
+    #[turbo_tasks::function]
+    async fn chunk_content_result(self) -> Result<EcmascriptChunkContentResultVc> {


Nit: there are a few ecmascript_chunk_content calls in this file that could now call this method.

jridgewell · 2022-10-28T22:57:06Z

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

-                        code: item.code_vc,
-                        id: item.chunk_item.id(),
-                    }
-                    .keyed_cell(EcmascriptChunkEntrySourceMapAssetCellKey(item.chunk_item))


I never understood what keyed_cell did, but is it ok that it's no longer used?

jridgewell · 2022-10-29T00:13:10Z

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

+                .iter()
+            {
+                for item in chunk_items_chunk.await?.iter() {
+                    source_maps.push(EcmascriptChunkEntrySourceMapAssetVc::new(path, *item).into());


I'm a little lost with how this helps to not invalidate. As far as I understand, the id is just ValueToString of the item, so always constant until the items path changes (which shouldn't happen). The code_vc just depends on the item.content() cell, so it shouldn't change either.

Is it the factory().await? read that's causing the invalidations? So by deferring that until the EcmascriptChunkEntrySourceMapAsset::content(), we avoid recreating the EcmascriptChunkEntrySourceMapAssetVc itself. And in this case it should be ok, because if the factory's read were to change, the source map would necessarily change.

self.chunk.chunk_content().await? is the problematic thing. It's a snapshot of the chunk content, so it changes everytime a chunk item changes (as it includes the hash).

In order to serve sourcemaps with HMR updates, we (used to) reference a source map asset for every chunk item using the items version hash (see #2464 and #2474 for the headaches this can cause). This update changes that, so that a chunk becomes responsible for giving access to one of its item's source map. We already have the `GenerateSourceMap` trait which assembles the chunk's `Sectioned` sourcemap from all chunk items, so why not just allow access to an individual section? Coupled with a `ContentSource` that can retrieve the chunk, we can then provide access to that chunk's current item sourcemaps per request.

In order to serve source maps with HMR updates, we (used to) reference a source map asset for every chunk item using the items version hash (see #2464 and #2474 for the headaches this can cause). This update changes that, so that a chunk becomes responsible for giving access to one of its item's source map. We already have the `GenerateSourceMap` trait which assembles the chunk's `Sectioned` source map from all chunk items, so why not just allow access to an individual section? Coupled with a `ContentSource` that can retrieve the chunk, we can then provide access to that chunk's current item source maps per request.

…3059) In order to serve source maps with HMR updates, we (used to) reference a source map asset for every chunk item using the items version hash (see vercel/turborepo#2464 and vercel/turborepo#2474 for the headaches this can cause). This update changes that, so that a chunk becomes responsible for giving access to one of its item's source map. We already have the `GenerateSourceMap` trait which assembles the chunk's `Sectioned` source map from all chunk items, so why not just allow access to an individual section? Coupled with a `ContentSource` that can retrieve the chunk, we can then provide access to that chunk's current item source maps per request.

…urborepo#2474)

…3059) In order to serve source maps with HMR updates, we (used to) reference a source map asset for every chunk item using the items version hash (see vercel/turborepo#2464 and vercel/turborepo#2474 for the headaches this can cause). This update changes that, so that a chunk becomes responsible for giving access to one of its item's source map. We already have the `GenerateSourceMap` trait which assembles the chunk's `Sectioned` source map from all chunk items, so why not just allow access to an individual section? Coupled with a `ContentSource` that can retrieve the chunk, we can then provide access to that chunk's current item source maps per request.

…urborepo#2474)

…3059) In order to serve source maps with HMR updates, we (used to) reference a source map asset for every chunk item using the items version hash (see vercel/turborepo#2464 and vercel/turborepo#2474 for the headaches this can cause). This update changes that, so that a chunk becomes responsible for giving access to one of its item's source map. We already have the `GenerateSourceMap` trait which assembles the chunk's `Sectioned` source map from all chunk items, so why not just allow access to an individual section? Coupled with a `ContentSource` that can retrieve the chunk, we can then provide access to that chunk's current item source maps per request.

sokra marked this pull request as ready for review October 28, 2022 22:34

sokra requested a review from a team as a code owner October 28, 2022 22:34

sokra requested a review from alexkirsz October 28, 2022 22:34

vercel bot deployed to Preview – turbo-site October 28, 2022 22:35 View deployment

sokra added the area: turbopack label Oct 28, 2022

sokra force-pushed the sokra/perf/source-map-invalidate branch from 404299d to 8ba360e Compare October 28, 2022 23:42

vercel bot deployed to Preview – turbo-site October 28, 2022 23:44 View deployment

alexkirsz approved these changes Oct 28, 2022

View reviewed changes

jridgewell approved these changes Oct 29, 2022

View reviewed changes

sokra force-pushed the sokra/perf/source-map-invalidate branch from 8ba360e to 27448cc Compare October 29, 2022 03:21

vercel bot deployed to Preview – turbo-site October 29, 2022 03:23 View deployment

change source map handling to not invalidate on file change

1d3ffa8

sokra force-pushed the sokra/perf/source-map-invalidate branch from 27448cc to 1d3ffa8 Compare October 29, 2022 07:43

sokra merged commit 8266705 into main Oct 29, 2022

sokra deleted the sokra/perf/source-map-invalidate branch October 29, 2022 07:43

vercel bot deployed to Preview – turbo-site October 29, 2022 07:44 View deployment

jridgewell mentioned this pull request Dec 17, 2022

Remove chunk item source maps from the asset graph #3059

Merged

ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Jul 25, 2024

change source map handling to not invalidate on file change (vercel/t…

940252d

…urborepo#2474)

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

change source map handling to not invalidate on file change (vercel/t…

7029b4f

…urborepo#2474)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

change source map handling to not invalidate on file change #2474

change source map handling to not invalidate on file change #2474

sokra commented Oct 28, 2022

vercel bot commented Oct 28, 2022 •

edited

Loading

jridgewell Oct 28, 2022

jridgewell Oct 28, 2022

sokra Oct 29, 2022

jridgewell Oct 29, 2022

sokra Oct 29, 2022

change source map handling to not invalidate on file change #2474

change source map handling to not invalidate on file change #2474

Conversation

sokra commented Oct 28, 2022

vercel bot commented Oct 28, 2022 • edited Loading

jridgewell Oct 28, 2022

Choose a reason for hiding this comment

jridgewell Oct 28, 2022

Choose a reason for hiding this comment

sokra Oct 29, 2022

Choose a reason for hiding this comment

jridgewell Oct 29, 2022

Choose a reason for hiding this comment

sokra Oct 29, 2022

Choose a reason for hiding this comment

vercel bot commented Oct 28, 2022 •

edited

Loading