fix(vitest): Fix cacheDir not being honored if deps.optimizer.{mode}.enabled is true

[jacoberdman2147]

Description

Fixes #6733

So initially it just seemed like the viteConfig.cacheDir set within optimizer should have been prioritizing the one defined in the config but there were other issues with the way cacheDir was being handled within the resolveOptimizerConfig function so I decided to remove the cacheDir logic entirely since it seemed to be unnecessary.

With this, I also fixed an issue where the runVitest function used within some of the tests wasn't properly passing through the options that were given to it. This uncovered the failures within cache.test.ts that should have been happening all along.

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

• It's really useful if your PR references an issue where it is discussed ahead of time. If the feature is substantial or introduces breaking changes without a discussion, PR might be closed.
• Ideally, include a test that fails without this PR but passes with it.
• Please, don't make changes to pnpm-lock.yaml unless you introduce a new test example.

Tests

• Run the tests with pnpm test:ci.

Documentation

• If you introduce new functionality, document it. You can run documentation with pnpm run docs command.

Changesets

• Changes in changelog are generated from PR name. Please, make sure that it explains your changes in an understandable manner. Please, prefix changeset messages with feat:, fix:, perf:, docs:, or chore:.

[netlify]

✅ Deploy Preview for vitest-dev ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	9f7c89d
🔍 Latest deploy log	https://app.netlify.com/sites/vitest-dev/deploys/67353b5cf6dadc0008d19280
😎 Deploy Preview	https://deploy-preview-6902--vitest-dev.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[jacoberdman2147]

So I spent a while looking into the test failures and I'm not really sure what to do. The specific web.test.ts and ssr.test.ts work fine before my last commit to remove the resolver plugin's handling of cacheDir, but without that commit we run into issues with cache.test.ts in the default case with optimizer enabled. The issue (with all commits) seems to be that the config for the ViteNodeServer isn't using the resolved cacheDir that we get from the resolveConfig method in the Vitest class, but I'm not sure if it should be or not. I don't completely understand how it all works but it seems like the cache for resolved modules and stuff being based around .vite by default could maybe make sense, but it may also make sense to pass down our resolved cacheDir, I'm not sure.

[hi-ogawa]

Thanks for investigating this. I took a look and indeed the current state is quite complicated. But my intuition is that we shouldn't need to change createVitest. We need to figure out how to deal with cacheDir overwritten here for optimize deps:

vitest/packages/vitest/src/node/plugins/optimizer.ts

Lines 22 to 23 in 5e6de27

	viteConfig.cacheDir
	= webOptimizer.cacheDir || ssrOptimizer.cacheDir || viteConfig.cacheDir

and we again deriving cache.dir from it

vitest/packages/vitest/src/node/config/resolveConfig.ts

Lines 677 to 681 in 5e6de27

	let cacheDir = VitestCache.resolveCacheDir(
	'',
	resolve(viteConfig.cacheDir, 'vitest'),
	resolved.name,
	)

Note that Vite's cacheDir for optimize deps and Vitest's cache.dir for ResultCache are two completely different concepts. Probably there's some limits what we can do, but let me try something in #6910.

If we have to choose one, probably test/optimize-deps should pass and cache.test.ts is the one broken.

[jacoberdman2147]

Yeah it looks like your fix better addresses the cacheDir behavior, thanks for looking into that. As far as the tests go though, that web optimizer test for cache.test.ts wasn't actually getting that optimizer option through to the plugin like it should be, and that's why I had the change in createVitest. Without it, those tests aren't actually testing the behavior that they say they are so it seems like we might still want to keep that part? That is, unless the change you made here https://github.com/vitest-dev/vitest/pull/6910/files#r1843197601 addresses that piece, in which case, I'll close this PR.

[hi-ogawa]

As far as the tests go though, that web optimizer test for cache.test.ts wasn't actually getting that optimizer option through to the plugin like it should be, and that's why I had the change in createVitest. Without it, those tests aren't actually testing the behavior that they say they are so it seems like we might still want to keep that part? That is, unless the change you made here https://github.com/vitest-dev/vitest/pull/6910/files#r1843197601 addresses that piece, in which case, I'll close this PR.

Your observation is right. Passing test: options in createVitest does make sense to me, but I wasn't entirely sure if it has any downside, so I did a smaller change for now. Maybe we'll need to revisit that in the future, but I think we can close this.
Thanks again for the contribution 🙏