feat(config): deprecate `cache.dir` option #5229

fenghan34 · 2024-02-18T13:48:47Z

Description

Closes #3272

This PR removes the cache.dir option and reuses Vite's cacheDir to save cache.

fix: resolving dep optimizer issues with workspace #4036

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 · 2024-02-18T13:49:04Z

✅ Deploy Preview for fastidious-cascaron-4ded94 ready!

Name	Link
🔨 Latest commit	`8af373b`
🔍 Latest deploy log	https://app.netlify.com/sites/fastidious-cascaron-4ded94/deploys/65f3fecb5edb86000821e512
😎 Deploy Preview	https://deploy-preview-5229--fastidious-cascaron-4ded94.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.

sheremet-va · 2024-02-18T16:59:38Z

packages/vitest/src/node/cli/cli-config.ts

    // cache can only be "false" or an object
    transform(cache) {
      if (cache)
        return {}
      return cache
    },
-  },
+  } as VitestCLIOptions['cache'],


No, you need to manually pass null to dir subcommand instead of removing it

Thanks for letting me know, I've updated it.

sheremet-va · 2024-02-18T17:00:14Z

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

@@ -24,8 +24,6 @@ export function resolveOptimizerConfig(_testOptions: DepsOptimizationOptions | u
    }
  }
  else {
-    const root = testConfig.root ?? process.cwd()
-    const cacheDir = testConfig.cache !== false ? testConfig.cache?.dir : undefined


We still need to handle it during the deprecation period

I've updated it. I'm new to the optimizer config, not sure if this is right change, could you please help to review again? Thank you so much!

I would expect test.cache.dir to just set cacheDir, everything else should just refer to config.cacheDir

sheremet-va · 2024-02-20T08:47:11Z

Deprecating is not a breaking change as long as it's still resolved.

fenghan34 · 2024-02-20T09:07:37Z

Deprecating is not a breaking change as long as it's still resolved.

Aha...thanks for clarifying! As you said it's still resolved, do I need to handle cache.dir in resolving config? Like assigning Vite's cacheDir as cache.dir if it exists?

sheremet-va · 2024-02-20T09:11:08Z

Deprecating is not a breaking change as long as it's still resolved.

Aha...thanks for clarifying! As you said it's still resolved, do I need to handle cache.dir in resolving config? Like assigning Vite's cacheDir as cache.dir if it exists?

Yes, I would expect cache.dir to affect the directory somehow until the option is removed completely in the next major.

hi-ogawa · 2024-02-22T03:29:28Z

test/optimize-deps/test/web.test.ts

@@ -6,5 +6,5 @@ import { expect, test } from 'vitest'
 import { importMetaUrl } from '@vitest/test-dep-url'

 test('import.meta.url', () => {
-  expect(importMetaUrl).toContain('/node_modules/.vitest/deps/')
+  expect(importMetaUrl).toContain('/node_modules/.vite/deps/')


I think this change means that Vite and Vitest are going to share the same folder for pre-bundling (aka optimizeDeps).

Though Vitest side of pre-bundling is disabled by default since v1.3.0 #5156, would this mean that users will not be able to run vite dev and vitest in watch mode at the same time when they explicitly use deps.optimizer.enable and have different optimizeDeps settings between two?

For example, in current main branch, running these two commands at the same time seems to work even with deps.optimizer.web.enabled: true. DEBUG=vite:deps is convenient to see what Vite does inside node_modules/.vite

DEBUG=vite:deps pnpm -C examples/react-testing-lib dev DEBUG=vite:deps pnpm -C examples/react-testing-lib test

Thanks for the question and example, I tried running pnpm dlx concurrently "DEBUG=vite:deps pnpm -C examples/react-testing-lib dev" "DEBUG=vite:deps pnpm -C examples/react-testing-lib test" in this PR's branch, with the workspace's Vitest version, it seems to have worked as expected.

I see. I just tried and it looks like it actually works, but it might be a bit coincidental.

When running pnpm dev, Vite generates files such as examples/react-testing-lib/node_modules/.vite/deps/react.js, then when I run pnpm test, Vitest will remove those files. The reason why Vite can keep serving the files in .vite/deps might be because of Vite's in-memory cache for js requests (I tried "Disable cache" in browser devtools).

I think I need to dig into this further to see whether buggy behavior is reproducible.
Or maybe running Vite and Vitest at the same time in the same directory can be considered an unsupported feature (at least when combined with deps.optimizer.enabled).

Actually I think Vitest already uses node_modules/.vite when using Vitest browser mode https://vitest.dev/guide/browser.html#browser-mode, so this might not be an issue in practice.

@sheremet-va Do you know if it's safe to use a same directory node_modules/.vite/deps as Vite for deps optimization?

I don't think it's safe, that's why this option was introduced in the first place 🤔

The main reason back then was that we were not sure if .vite deletes everything when deps are changed, or just the deps folder. If it just deletes the deps, then we can have .vite/vitest/... cache folder

It looks like Vite deletes only node_modules/.vite/deps, for example, this code is for --force flag https://github.com/vitejs/vite/blob/b20d54257e6105333c19676a403c574667878e0f/packages/vite/src/node/optimizer/index.ts#L382-L384

I agree we should avoid overlapping .vite/deps, so using node_modules/.vite/vitest sounds good to me.

Thanks for the suggestion, I'll update with .vite/vitest later.

sheremet-va · 2024-03-14T13:50:05Z

packages/vitest/src/node/cli/cli-config.ts

-        argument: '<path>',
-        normalize: true,
-      },
+      dir: null,


Maybe it would be better to add a handler that throws an error? I think CLI will still propagate the value even if it's not defined in a schema

Yeah I noticed that, I'll update it later

fenghan34 added 2 commits February 18, 2024 21:26

feat!(config): deprecate cache.dir option

c77c545

docs: update

bd44d67

fenghan34 marked this pull request as draft February 18, 2024 13:51

fenghan34 added 4 commits February 18, 2024 21:56

types: revert cache

b42c227

test: update

91f5d50

chore: update cli config

f156b7e

test: update

e3341fb

sheremet-va requested changes Feb 18, 2024

View reviewed changes

fenghan34 added 3 commits February 19, 2024 15:54

chore: keep subcommands

11456b8

chore: handle cache.dir

30e122b

Merge branch 'main' into fix/result-location

02b08a0

sheremet-va changed the title ~~feat!(config): deprecate cache.dir option~~ feat(config): deprecate cache.dir option Feb 20, 2024

fenghan34 added 3 commits February 21, 2024 22:25

chore: update

82c5c2a

Merge branch 'main' into fix/result-location

b43a562

test: update

f831b7a

fenghan34 marked this pull request as ready for review February 21, 2024 15:02

hi-ogawa reviewed Feb 22, 2024

View reviewed changes

hi-ogawa requested a review from sheremet-va February 26, 2024 00:42

fenghan34 added 5 commits March 1, 2024 17:28

chore: update

4a39b08

test: add tests

a7cd849

Merge branch 'main' into fix/result-location

9cb6ec2

chore: update

1aff021

docs: update

147e6b9

sheremet-va requested changes Mar 14, 2024

View reviewed changes

fenghan34 added 2 commits March 15, 2024 15:05

Merge branch 'main' into fix/result-location

f0bab3b

chore: intercept the --cache.dir command option

8af373b

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(config): deprecate `cache.dir` option #5229

feat(config): deprecate `cache.dir` option #5229

fenghan34 commented Feb 18, 2024

netlify bot commented Feb 18, 2024 •

edited

sheremet-va Feb 18, 2024

fenghan34 Feb 19, 2024

sheremet-va Feb 18, 2024

fenghan34 Feb 19, 2024

sheremet-va Feb 20, 2024

sheremet-va commented Feb 20, 2024

fenghan34 commented Feb 20, 2024

sheremet-va commented Feb 20, 2024

hi-ogawa Feb 22, 2024

hi-ogawa Feb 22, 2024 •

edited

fenghan34 Feb 22, 2024

hi-ogawa Feb 22, 2024

hi-ogawa Feb 26, 2024

sheremet-va Feb 26, 2024 •

edited

hi-ogawa Feb 26, 2024 •

edited

fenghan34 Feb 26, 2024

sheremet-va Mar 14, 2024

fenghan34 Mar 15, 2024 •

edited

feat(config): deprecate cache.dir option #5229

feat(config): deprecate cache.dir option #5229

Conversation

fenghan34 commented Feb 18, 2024

Description

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

Tests

Documentation

Changesets

netlify bot commented Feb 18, 2024 • edited

✅ Deploy Preview for fastidious-cascaron-4ded94 ready!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sheremet-va commented Feb 20, 2024

fenghan34 commented Feb 20, 2024

sheremet-va commented Feb 20, 2024

Choose a reason for hiding this comment

hi-ogawa Feb 22, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sheremet-va Feb 26, 2024 • edited

Choose a reason for hiding this comment

hi-ogawa Feb 26, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fenghan34 Mar 15, 2024 • edited

Choose a reason for hiding this comment

feat(config): deprecate `cache.dir` option #5229

feat(config): deprecate `cache.dir` option #5229

netlify bot commented Feb 18, 2024 •

edited

hi-ogawa Feb 22, 2024 •

edited

sheremet-va Feb 26, 2024 •

edited

hi-ogawa Feb 26, 2024 •

edited

fenghan34 Mar 15, 2024 •

edited