Merge EcmascriptChunkUpdates before sending them to the client
#3975
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 ↗︎
8 Ignored Deployments
|
This was referenced Feb 27, 2023
|
Current dependencies on/for this PR:
This comment was auto-generated by Graphite. |
alexkirsz
force-pushed
the
alexkirsz/web-616-merged-updates
branch
from
98226e6 to
670aa30
Compare
alexkirsz
force-pushed
the
alexkirsz/web-616-split-ecmascript-modules
branch
from
041f42e to
9ec7680
Compare
alexkirsz
force-pushed
the
alexkirsz/web-616-merged-updates
branch
from
670aa30 to
5a5db52
Compare
|
alexkirsz
force-pushed
the
alexkirsz/web-616-merged-updates
branch
2 times, most recently
from
1719e4a to
a08e55b
Compare
alexkirsz
force-pushed
the
alexkirsz/web-616-split-ecmascript-modules
branch
from
9ec7680 to
a2a5e3e
Compare
alexkirsz
force-pushed
the
alexkirsz/web-616-merged-updates
branch
from
a08e55b to
c8c2962
Compare
alexkirsz
force-pushed
the
alexkirsz/web-616-split-ecmascript-modules
branch
from
a2a5e3e to
10de7af
Compare
alexkirsz
changed the base branch from
alexkirsz/web-616-split-ecmascript-modules
to
alexkirsz/web-616-remove-cell_local-in-favor-of-more
alexkirsz
force-pushed
the
alexkirsz/web-616-merged-updates
branch
2 times, most recently
from
9e318d9 to
8338dcd
Compare
alexkirsz
commented
Feb 27, 2023
| let path = chunk.path().await?; | ||
| data.push(Value::String(path.path.clone())); | ||
| } | ||
| let server_root = this.server_root.await?; |
There was a problem hiding this comment.
The way the page loader asset computed paths was incorrect before, as it wasn't basing them upon the server root path. But this doesn't really have any measurable effect today as we don't support base path.
| location.reload(); | ||
| aggregated = { | ||
| resource: msg.resource, | ||
| update: mergeChunkListUpdates(aggregated.update, msg.instruction), |
There was a problem hiding this comment.
Since the HMR updates are now very different (from a simple "EcmascriptChunkUpdate" to a "ChunkListUpdate" that can contain simple chunk updates and complex "EcmascriptMergedChunkUpdate"s), I cut the merging logic into smaller functions that each operate at a specific nested-type's level.
|
|
||
| const runHooks = chunksWithUpdates.size === 0; | ||
| // TODO(WEB-582) Disable update aggregation for now. |
There was a problem hiding this comment.
This was already the case, but I made it explicit here.
| throw new Error(`unknown update type \`${update}\``); | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
CSS updates are now handled at the chunk list level, using the BACKEND.loadChunk/unloadChunk/reloadChunk logic.
alexkirsz
force-pushed
the
alexkirsz/web-616-remove-cell_local-in-favor-of-more
branch
from
2da7d96 to
f882eb8
Compare
alexkirsz
force-pushed
the
alexkirsz/web-616-merged-updates
branch
from
a14da5c to
19f426a
Compare
alexkirsz
force-pushed
the
alexkirsz/web-616-remove-cell_local-in-favor-of-more
branch
from
f882eb8 to
468c9c9
Compare
alexkirsz
force-pushed
the
alexkirsz/web-616-merged-updates
branch
from
19f426a to
d8142d3
Compare
alexkirsz
force-pushed
the
alexkirsz/web-616-remove-cell_local-in-favor-of-more
branch
from
468c9c9 to
b269366
Compare
alexkirsz
force-pushed
the
alexkirsz/web-616-merged-updates
branch
from
d8142d3 to
754ae23
Compare
Benchmark for f907f7f
Click to view full benchmark
|
alexkirsz
force-pushed
the
alexkirsz/web-616-remove-cell_local-in-favor-of-more
branch
from
b269366 to
1f19e84
Compare
Benchmark for 38bc550
Click to view full benchmark
|
sokra
reviewed
Mar 8, 2023
crates/next-core/src/next_client_component/with_client_chunks.rs
Outdated
Show resolved
Hide resolved
Comment on lines
3
to
+7
| function __turbopack_load__(path: string): any; | ||
| function __turbopack_register_chunk_list__( | ||
| chunkListPath: string, | ||
| chunksPaths: string[] | ||
| ): any; |
There was a problem hiding this comment.
should we merge these and offer a single method:
function __turbopack_load_chunks__(
chunkPaths: string[]
chunkListPath: string,
): any;
which loads all the chunks at once and registers the chunk list.
There was a problem hiding this comment.
I think __turbopack_load__ is still useful on its own for loading separate assets which don't have a chunk list.
Benchmark for 91c7ebf
Click to view full benchmark
|
alexkirsz
force-pushed
the
alexkirsz/web-616-remove-cell_local-in-favor-of-more
branch
from
6939aef to
170319c
Compare
alexkirsz
force-pushed
the
alexkirsz/web-616-merged-updates
branch
from
2d20446 to
feee3bd
Compare
Base automatically changed from
alexkirsz/web-616-remove-cell_local-in-favor-of-more
to
main
March 8, 2023 17:55
alexkirsz
added
the
pr: automerge
Benchmark for 18bfdaa
Click to view full benchmark
|
alexkirsz
force-pushed
the
alexkirsz/web-616-merged-updates
branch
from
fd5d61e to
c48c28f
Compare
sokra
approved these changes
Mar 9, 2023
alexkirsz
force-pushed
the
alexkirsz/web-616-merged-updates
branch
from
c48c28f to
f183674
Compare
Benchmark for 5aa923b
Click to view full benchmark
|
kodiakhq bot
pushed a commit
that referenced
this pull request
Mar 9, 2023
### Description It's a bit ugly and I dislike the way it's implemented, but this will help for now. * When using HMR and introducing an parse error to a module, this keeps the previous references and exports type to keep the environment intact and avoid unloading a bunch of modules depends on #3975 to work correctly
ijjk
pushed a commit
to vercel/next.js
that referenced
this pull request
Mar 9, 2023
# New Features - vercel/turborepo#3975 # Bug Fixes - vercel/turborepo#4129 - vercel/turborepo#4134 - vercel/turborepo#4062 # Performance - vercel/turborepo#4093
jridgewell
pushed a commit
to vercel/next.js
that referenced
this pull request
Mar 10, 2023
…cel/turborepo#3975) This diff: * introduces the `VersionedContentMerger` trait, which allows for merging the updates of versioned contents within the same chunk group; * implements this for `EcmascriptChunkContent`/`EcmascriptChunkUpdate`, turning them into `EcmascriptMergedChunkContent`/`EcmascriptMergedChunkUpdate`; * creates a new `ChunkList` asset which is capable of merging chunk updates of chunks within the same chunk group, and create such an asset for dynamic chunks (through manifest/loader_item.rs) and chunk group files assets. This fixes a bunch of edge cases related to HMR: * HMR of dynamic imports now works; * Chunks getting added/deleted/renamed now also works with HMR, since we're listening to updates at the chunk group level; * CSS chunks get reloaded in the right order, with respect for precedence. There are still known edge cases with HMR: * CSS chunks added through HMR are not inserted at the right position to respect precedence (WEB-652). * Update aggregation is disabled because we don't want a critical issue to stop all HMR (WEB-582) . This would be fixed by applying aggregated updates when dismissing the error modal, but there are some edge cases with this too (e.g. what happens when an HMR update also causes an error on top of an existing error).
sokra
pushed a commit
to vercel/next.js
that referenced
this pull request
Mar 13, 2023
…cel/turborepo#3975) This diff: * introduces the `VersionedContentMerger` trait, which allows for merging the updates of versioned contents within the same chunk group; * implements this for `EcmascriptChunkContent`/`EcmascriptChunkUpdate`, turning them into `EcmascriptMergedChunkContent`/`EcmascriptMergedChunkUpdate`; * creates a new `ChunkList` asset which is capable of merging chunk updates of chunks within the same chunk group, and create such an asset for dynamic chunks (through manifest/loader_item.rs) and chunk group files assets. This fixes a bunch of edge cases related to HMR: * HMR of dynamic imports now works; * Chunks getting added/deleted/renamed now also works with HMR, since we're listening to updates at the chunk group level; * CSS chunks get reloaded in the right order, with respect for precedence. There are still known edge cases with HMR: * CSS chunks added through HMR are not inserted at the right position to respect precedence (WEB-652). * Update aggregation is disabled because we don't want a critical issue to stop all HMR (WEB-582) . This would be fixed by applying aggregated updates when dismissing the error modal, but there are some edge cases with this too (e.g. what happens when an HMR update also causes an error on top of an existing error).
ForsakenHarmony
pushed a commit
to vercel/next.js
that referenced
this pull request
Jul 25, 2024
…cel/turborepo#3975) This diff: * introduces the `VersionedContentMerger` trait, which allows for merging the updates of versioned contents within the same chunk group; * implements this for `EcmascriptChunkContent`/`EcmascriptChunkUpdate`, turning them into `EcmascriptMergedChunkContent`/`EcmascriptMergedChunkUpdate`; * creates a new `ChunkList` asset which is capable of merging chunk updates of chunks within the same chunk group, and create such an asset for dynamic chunks (through manifest/loader_item.rs) and chunk group files assets. This fixes a bunch of edge cases related to HMR: * HMR of dynamic imports now works; * Chunks getting added/deleted/renamed now also works with HMR, since we're listening to updates at the chunk group level; * CSS chunks get reloaded in the right order, with respect for precedence. There are still known edge cases with HMR: * CSS chunks added through HMR are not inserted at the right position to respect precedence (WEB-652). * Update aggregation is disabled because we don't want a critical issue to stop all HMR (WEB-582) . This would be fixed by applying aggregated updates when dismissing the error modal, but there are some edge cases with this too (e.g. what happens when an HMR update also causes an error on top of an existing error).
ForsakenHarmony
pushed a commit
to vercel/next.js
that referenced
this pull request
Jul 25, 2024
### Description It's a bit ugly and I dislike the way it's implemented, but this will help for now. * When using HMR and introducing an parse error to a module, this keeps the previous references and exports type to keep the environment intact and avoid unloading a bunch of modules depends on vercel/turborepo#3975 to work correctly
ForsakenHarmony
pushed a commit
to vercel/next.js
that referenced
this pull request
Jul 29, 2024
…cel/turborepo#3975) This diff: * introduces the `VersionedContentMerger` trait, which allows for merging the updates of versioned contents within the same chunk group; * implements this for `EcmascriptChunkContent`/`EcmascriptChunkUpdate`, turning them into `EcmascriptMergedChunkContent`/`EcmascriptMergedChunkUpdate`; * creates a new `ChunkList` asset which is capable of merging chunk updates of chunks within the same chunk group, and create such an asset for dynamic chunks (through manifest/loader_item.rs) and chunk group files assets. This fixes a bunch of edge cases related to HMR: * HMR of dynamic imports now works; * Chunks getting added/deleted/renamed now also works with HMR, since we're listening to updates at the chunk group level; * CSS chunks get reloaded in the right order, with respect for precedence. There are still known edge cases with HMR: * CSS chunks added through HMR are not inserted at the right position to respect precedence (WEB-652). * Update aggregation is disabled because we don't want a critical issue to stop all HMR (WEB-582) . This would be fixed by applying aggregated updates when dismissing the error modal, but there are some edge cases with this too (e.g. what happens when an HMR update also causes an error on top of an existing error).
ForsakenHarmony
pushed a commit
to vercel/next.js
that referenced
this pull request
Jul 29, 2024
### Description It's a bit ugly and I dislike the way it's implemented, but this will help for now. * When using HMR and introducing an parse error to a module, this keeps the previous references and exports type to keep the environment intact and avoid unloading a bunch of modules depends on vercel/turborepo#3975 to work correctly
ForsakenHarmony
pushed a commit
to vercel/next.js
that referenced
this pull request
Aug 1, 2024
…cel/turborepo#3975) This diff: * introduces the `VersionedContentMerger` trait, which allows for merging the updates of versioned contents within the same chunk group; * implements this for `EcmascriptChunkContent`/`EcmascriptChunkUpdate`, turning them into `EcmascriptMergedChunkContent`/`EcmascriptMergedChunkUpdate`; * creates a new `ChunkList` asset which is capable of merging chunk updates of chunks within the same chunk group, and create such an asset for dynamic chunks (through manifest/loader_item.rs) and chunk group files assets. This fixes a bunch of edge cases related to HMR: * HMR of dynamic imports now works; * Chunks getting added/deleted/renamed now also works with HMR, since we're listening to updates at the chunk group level; * CSS chunks get reloaded in the right order, with respect for precedence. There are still known edge cases with HMR: * CSS chunks added through HMR are not inserted at the right position to respect precedence (WEB-652). * Update aggregation is disabled because we don't want a critical issue to stop all HMR (WEB-582) . This would be fixed by applying aggregated updates when dismissing the error modal, but there are some edge cases with this too (e.g. what happens when an HMR update also causes an error on top of an existing error).
ForsakenHarmony
pushed a commit
to vercel/next.js
that referenced
this pull request
Aug 1, 2024
### Description It's a bit ugly and I dislike the way it's implemented, but this will help for now. * When using HMR and introducing an parse error to a module, this keeps the previous references and exports type to keep the environment intact and avoid unloading a bunch of modules depends on vercel/turborepo#3975 to work correctly
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.
This diff:
VersionedContentMergertrait, which allows for merging the updates of versioned contents within the same chunk group;EcmascriptChunkContent/EcmascriptChunkUpdate, turning them intoEcmascriptMergedChunkContent/EcmascriptMergedChunkUpdate;ChunkListasset which is capable of merging chunk updates of chunks within the same chunk group, and create such an asset for dynamic chunks (through manifest/loader_item.rs) and chunk group files assets.This fixes a bunch of edge cases related to HMR:
There are still known edge cases with HMR: