fix: consistent module graph IDs during transform #8807
Closed
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.
Description
I was noticing that certain modules weren't updating when saving a change to their dependencies, and traced it back to this recent addition:
https://github.com/vitejs/vite/blob/v2/packages/vite/src/node/plugins/importAnalysis.ts#L676-L679
What's happening is some modules are given a browser-safe URL with the
/@id/
prefix before they're added to the module graph. Then the above code strips the prefix and the imports are sent back through the transform pipeline, which adds duplicate modules without the prefix. This has the effect of creating a discontinuous module graph.I would expect it to look like this:
But instead the graph looks like this:
Thus any changes made to
dependency.js
don't properly trickle down when purging the module cache and I must restart the server to see the changes.I've been running this fix in an internal project for a few weeks via a yarn patch. Wanted to contribute the fix and a failing test.
Additional context
The fix essentially moves the
/@id/
unwrapping to a more-central location alongside a tiny change to re-use module IDs if they already exist in the graph.What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).