Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cacheDir not honored if deps.optimizer.{mode}.enabled is true #6733

Closed
6 tasks done
Pistonight opened this issue Oct 17, 2024 · 4 comments · Fixed by #6910 · May be fixed by #6902
Closed
6 tasks done

cacheDir not honored if deps.optimizer.{mode}.enabled is true #6733

Pistonight opened this issue Oct 17, 2024 · 4 comments · Fixed by #6910 · May be fixed by #6902
Labels
p3-minor-bug An edge case that only affects very specific usage (priority)

Comments

@Pistonight
Copy link

Pistonight commented Oct 17, 2024 •
edited
Loading

Describe the bug

cacheDir not honored if deps.optimizer.{mode}.enabled is true. vitest will save cache to node_modules/.vite as if cacheDir is not specified.

Reproduction

Run

npm create vite@latest
npm i

Edit the config

/// <reference types="vitest/config" />
import { defineConfig } from 'vite'
import react from '@vitejs/plugin-react'

export default defineConfig({
  cacheDir: ".vite",
  plugins: [react()],
  test: {
    deps: {
      optimizer: {
        ssr: {
          enabled: true // toggling this to repro
        }
      }
    }
  }
})

Create a test src/App.test.tsx

import { describe, expect, test } from "vitest";
describe("App", () =>{
    test("works", () => {
        expect(1).not.toBe(2);
    });
});

Run

npx vitest
  1. Observe cache is unexpectedly created in node_modules/.vite.
  2. Toggle test.deps.optimizer.ssr.enabled to false, and run npx vitest again
  3. Observe cache is created in .vite as expected

System Info

System:
    OS: Windows 11 10.0.26120
    CPU: (32) x64 AMD Ryzen 9 5950X 16-Core Processor
    Memory: 41.33 GB / 63.91 GB
  Binaries:
    Node: 18.20.4 - ~\dotbin\extra\portable\nvm\symlink\node.EXE
    npm: 10.7.0 - ~\dotbin\extra\portable\nvm\symlink\npm.CMD
    bun: 1.1.6 - ~\.bun\bin\bun.EXE
  Browsers:
    Edge: Chromium (127.0.2651.86)
    Internet Explorer: 11.0.26100.1
  npmPackages:
    @vitejs/plugin-react: ^4.3.2 => 4.3.2
    vite: ^5.4.8 => 5.4.9
    vitest: ^2.1.3 => 2.1.3

Used Package Manager

npm

Validations
@Pistonight
Copy link
Author

Pistonight commented Oct 17, 2024 •
edited
Loading

I did notice that the deprecated cache.dir option behave correctly.

Seems like it's not checking vite cacheDir here but it should?

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

Lines 44 to 64 in a939779

const cacheDir
= testConfig.cache !== false ? testConfig.cache?.dir : undefined
const currentInclude = testOptions.include || viteOptions?.include || []
const exclude = [
'vitest',
// Ideally, we shouldn't optimize react in test mode, otherwise we need to optimize _every_ dependency that uses react.
'react',
'vue',
...(testOptions.exclude || viteOptions?.exclude || []),
]
const runtime = currentInclude.filter(
n => n.endsWith('jsx-dev-runtime') || n.endsWith('jsx-runtime'),
)
exclude.push(...runtime)
const include = (testOptions.include || viteOptions?.include || []).filter(
(n: string) => !exclude.includes(n),
)
newConfig.cacheDir
= cacheDir ?? VitestCache.resolveCacheDir(root, cacheDir, testConfig.name)

@sheremet-va
Copy link
Member

Seems like it's not checking vite cacheDir here but it should?

Looks like a bug indeed. PR is welcome!

@sheremet-va sheremet-va added p3-minor-bug An edge case that only affects very specific usage (priority) and removed pending triage labels Oct 17, 2024
@Pistonight
Copy link
Author

Thanks for confirming, just to clarify my understanding: how do these 3 properties relate and resolve exactly?

  1. Vite cacheDir
  2. Vitest server.deps.cacheDir
  3. Vitest cache.dir

@hi-ogawa
Copy link
Contributor

hi-ogawa commented Oct 18, 2024 •
edited
Loading

As par the change #5229, the intent is to put things under cacheDir + '/vitest'. Vitest users don't need to configure server.deps.cacheDir and the others should mostly look like:

const finalCacheDir = cache.dir ?? join(cacheDir, "vitest")
                      //                ^^^^^^^^
                      // this is probably hard-coded as `node_modules/.vite`,
                      // which is a bug

By default Vite has cacheDir = 'node_modules/.vite', so Vitest uses node_modules/.vite/vitest. If you explicitly set cacheDir = '.vite', Vitest should probably use .vite/vitest.

@jacoberdman2147 jacoberdman2147 mentioned this issue Nov 13, 2024
Open
6 tasks
@hi-ogawa hi-ogawa mentioned this issue Nov 15, 2024
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
p3-minor-bug An edge case that only affects very specific usage (priority)
Projects
None yet
Milestone
No milestone
3 participants
@hi-ogawa @sheremet-va @Pistonight