fix: force remote module chunks to isolate themselves #14571
Merged
Conversation
This file contains hidden or 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
🦋 Changeset detectedLatest commit: 7b831dd The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Merged
5 tasks
dummdidumm
added a commit
that referenced
this pull request
Oct 7, 2025
…sues, but they snuck back in. The problem, it turns out (read https://rollupjs.org/configuration-options/#output-manualchunks very carefully), is that dependencies of a manual chunk might get pulled into that manual chunk, too. If the dependency happens to be our env module, this can cause issues since the module might eagerly access the env. Luckily Rollup has recently solved this via a dedicated option (that will be the default in version 5 and is hence deprecated right away) named `onlyExplicitManualChunks` (rollup/rollup#6087). This solves our problems around chunk ordering. For users not on the latest version yet we do a best-effort fallback by extracting env into its own chunk. This should solve most issues people encounter but the general problem can still occur, which is only fixed with the new option (hence we warn if the rollup version is not up to date). Fixes #14590
dummdidumm
added a commit
that referenced
this pull request
Oct 7, 2025
…ules With #14571 we hoped to have solved the chunk/code execution order issues, but they snuck back in. The problem, it turns out (read https://rollupjs.org/configuration-options/#output-manualchunks very carefully), is that dependencies of a manual chunk might get pulled into that manual chunk, too. If the dependency happens to be our env module, this can cause issues since the module might eagerly access the env. Luckily Rollup has recently solved this via a dedicated option (that will be the default in version 5 and is hence deprecated right away) named `onlyExplicitManualChunks` (rollup/rollup#6087). This solves our problems around chunk ordering. For users not on the latest version yet we do a best-effort fallback by extracting env into its own chunk. This should solve most issues people encounter but the general problem can still occur, which is only fixed with the new option (hence we warn if the rollup version is not up to date). Fixes #14590
5 tasks
Rich-Harris
pushed a commit
that referenced
this pull request
Oct 7, 2025
…ules (#14632) With #14571 we hoped to have solved the chunk/code execution order issues, but they snuck back in. The problem, it turns out (read https://rollupjs.org/configuration-options/#output-manualchunks very carefully), is that dependencies of a manual chunk might get pulled into that manual chunk, too. If the dependency happens to be our env module, this can cause issues since the module might eagerly access the env. Luckily Rollup has recently solved this via a dedicated option (that will be the default in version 5 and is hence deprecated right away) named `onlyExplicitManualChunks` (rollup/rollup#6087). This solves our problems around chunk ordering. For users not on the latest version yet we do a best-effort fallback by extracting env into its own chunk. This should solve most issues people encounter but the general problem can still occur, which is only fixed with the new option (hence we warn if the rollup version is not up to date). Fixes #14590
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 is my attempt to provide an alternative to #14456, which is extremely clever but makes me very nervous. It works, but as soon as another plugin tries the same trick (repeatedly getting a list of every module in the graph and loading it, until there are no new modules to load) you end up in a deadlock situation. The 30 second timeout really isn't a solution. It's almost certain that very few users would ever encounter this, but for the ones that did it would be a very confusing and annoying bug.
This PR sticks with the
manualChunkstrick, but adds another layer: any modules imported by.remote.tsfiles are also put in their own chunks. This has the effect of forcing each chunk corresponding to a.remote.tsmodule to only contain that module, and none of the dependencies that Rollup incorrectly deems safe to include in the chunk.My hunch — or at least, my hope — is that Rollup gets confused about how to structure the graph because it's not expecting these chunks to be treated as entries (resulting in the bad kind of circular dependency) and that this change renders that moot. It's possible that I haven't fixed it at all, but have rather pushed the bug 'one level down' so to speak, but I think it's worth a shot.
Fixes #14444 and possibly #14430
Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
Tests
pnpm testand lint the project withpnpm lintandpnpm checkChangesets
pnpm changesetand following the prompts. Changesets that add features should beminorand those that fix bugs should bepatch. Please prefix changeset messages withfeat:,fix:, orchore:.Edits