fix(optimizer): avoid double-commit of optimized deps when discovery is disabled #13865
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
Fixes #13864
This prevents the optimizer from attempting to rename the
processingCacheDir
->depsCacheDir
twice for the initial build.When configured with
noDiscovery: true
, the optimizer is initialized with a procedure (runOptimizer()
) to optimize dependencies and then immediately commit the result. This would be safe, but there a 2 things going on that cause this to break downrunOptimizer()
to behave correctly when an existing scan or optimizer run is in progress.onCrawlEnd
hook that runs after request transformation is missing the necessary condition to skip re-running the optimizer when discovery is disabled. The existing code leads to a double-commitTo summarize the issue - calling
runOptimizer()
will always lead to committing dependencies, regardless of any in-flight requests. When noDiscovery is set to true it's called twice - once at server start, and again when the first request is transformedAdditional context
There's a fix that works equally well by adding a condition to the
onCrawlEnd
hook here:https://github.com/kherock/vite/blob/132a5a40a295fc68fdf5e2eb260631ccc13df8a1/packages/vite/src/node/optimizer/optimizer.ts#L629
However, I feel like this would have consequences on dependencies that receive changes while the server is active, so I think the proposed change in this PR is the better fix.
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).