fix(optimizer): avoid double-commit of optimized deps when discovery is disabled

[kherock]

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 down

• with discovery enabled, the initial scan and optimize step doesn't normally lead to a commit. This permits runOptimizer() to behave correctly when an existing scan or optimizer run is in progress.
• the 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-commit

To 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 transformed

Additional 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

    if (!isBuild && optimizationResult && !config.optimizeDeps.noDiscovery) {

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?

• Bug fix
• New Feature
• Documentation update
• Other

Before submitting the PR, please make sure you do the following

• Read the Contributing Guidelines.
• Read the Pull Request Guidelines and follow the PR Title Convention.
• Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
• Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
• Ideally, include relevant tests that fail without this PR but pass with it.

[stackblitz]

Run & review this pull request in StackBlitz Codeflow.

[patak-dev]

With the current PR, we aren't going to be able to cancel using optimizationResult.

And if a dependency has changed, then a server reset is needed and the optimized deps will be re-optimized because the hash will change. So I think your second idea is better, let's bail out on onCrawlEnd.

We could also avoid finding the onCrawlEnd event. Using a noop crawl finder. But it is a bit more complex and we should check if we gain any perf by doing so (we may also need the onCrawlEnd event for something else later).

[kherock]

Thanks for looking at this! I switched to the other approach.

[patak-dev]

/ecosystem-ci run vitest

[vite-ecosystem-ci]

📝 Ran ecosystem CI: Open

suite	result
vitest	✅ success