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.

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

Snapshots do not work with Yarn PnP #3426

Closed
6 tasks done
koistya opened this issue May 22, 2023 · 13 comments · Fixed by #3489 or #4238
Closed
6 tasks done

Snapshots do not work with Yarn PnP #3426

koistya opened this issue May 22, 2023 · 13 comments · Fixed by #3489 or #4238
Labels
p3-minor-bug An edge case that only affects very specific usage (priority)

Comments

@koistya
Copy link

koistya commented May 22, 2023

Describe the bug

expect(...).toMatchInlineSnapshot('...');
> TypeError: Cannot read properties of undefined (reading 'match')

Reproduction

  • Create a Yarn-based project with a coupe of workspaces (can be the root workspace + a child workspace)
  • Ensure that Yarn PnP is enabled (yarn config set pnp)
  • Add a couple of Vitest project configurations + the root Vitest (workspace) config, a few tests with inline snapshots
  • Run yarn vitest

It only works if all the peer dependencies in all Vitest workspaces match exactly, otherwise Yarn creates a separate virtual folder for each vitest instance which breaks the way Vitest works with inline snapshots. It looks like snapshot initialization happens in one "vitest" instance, while the test result matching uses a different "vitest" instance.

You can check virtual packages using yarn info --virtuals vitest command, or by looking up for vitest-virtual occurrences in .pnp.cjs.

Reproduction: https://github.com/koistya/vitest-workspaces
Referene: https://yarnpkg.com/advanced/pnp-spec#virtual-folders

System Info

System:
    OS: macOS 11.6
    CPU: (8) arm64 Apple M1
    Memory: 141.69 MB / 16.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 18.16.0 - /opt/homebrew/bin/node
    Yarn: 4.0.0-rc.44 - /opt/homebrew/bin/yarn
    npm: 9.5.1 - /opt/homebrew/bin/npm
    Watchman: 2023.04.24.00 - /opt/homebrew/bin/watchman

Used Package Manager

yarn

Validations

@koistya
Copy link
Author

koistya commented May 25, 2023

https://github.com/koistya/vitest-workspaces

git clone https://github.com/koistya/vitest-workspaces.git
cd ./vitest-workspaces
yarn install
yarn test

const snapshotState = this.getSnapshotState(filepath)
const { actual, expected, key, pass } = snapshotState.match({
testName,
received,
isInline,
error,
inlineSnapshot,
rawSnapshot,
})

^^^ snapshotState is undefined

@koistya
Copy link
Author

koistya commented May 26, 2023

vitest is being loaded from two different places:

/.yarn/__virtual__/vitest-virtual-66a00e0566/4/.yarn/berry/cache/vitest-npm-0.31.1-d67f37acf0-9.zip/node_modules/vitest/dist/vendor-vi.458e47b1.js
/.yarn/__virtual__/vitest-virtual-cb2edbe32a/4/.yarn/berry/cache/vitest-npm-0.31.1-d67f37acf0-9.zip/node_modules/vitest/dist/vendor-vi.458e47b1.js

And the previously initialized snapshotClient (in onBeforeTestRun) cannot be re-used when the assertion is being executed.

@koistya
Copy link
Author

koistya commented May 26, 2023

$ yarn info --virtuals vitest

─ vitest@npm:0.31.1
│  ├─ Instances: 1
│  ├─ Version: 0.31.1
│  │
│  ├─ Exported Binaries
│  │  └─ vitest
│  │
│  └─ Dependencies
│     ├─ @types/chai-subset@npm:^1.3.3 → npm:1.3.3
│     ├─ @types/chai@npm:^4.3.5 → npm:4.3.5
│     ├─ @types/node@npm:* → npm:20.2.4
│     ├─ @vitest/expect@npm:0.31.1 → npm:0.31.1
│     ├─ @vitest/runner@npm:0.31.1 → npm:0.31.1
│     ├─ @vitest/snapshot@npm:0.31.1 → npm:0.31.1
│     ├─ @vitest/spy@npm:0.31.1 → npm:0.31.1
│     ├─ @vitest/utils@npm:0.31.1 → npm:0.31.1
│     ├─ acorn-walk@npm:^8.2.0 → npm:8.2.0
│     ├─ acorn@npm:^8.8.2 → npm:8.8.2
│     ├─ cac@npm:^6.7.14 → npm:6.7.14
│     ├─ chai@npm:^4.3.7 → npm:4.3.7
│     ├─ concordance@npm:^5.0.4 → npm:5.0.4
│     ├─ debug@npm:^4.3.4 → npm:4.3.4
│     ├─ local-pkg@npm:^0.4.3 → npm:0.4.3
│     ├─ magic-string@npm:^0.30.0 → npm:0.30.0
│     ├─ pathe@npm:^1.1.0 → npm:1.1.0
│     ├─ picocolors@npm:^1.0.0 → npm:1.0.0
│     ├─ std-env@npm:^3.3.2 → npm:3.3.3
│     ├─ strip-literal@npm:^1.0.1 → npm:1.0.1
│     ├─ tinybench@npm:^2.5.0 → npm:2.5.0
│     ├─ tinypool@npm:^0.5.0 → npm:0.5.0
│     ├─ vite-node@npm:0.31.1 → npm:0.31.1
│     ├─ vite@npm:^3.0.0 || ^4.0.0 → npm:4.3.9
│     └─ why-is-node-running@npm:^2.2.2 → npm:2.2.2
│
└─ vitest@npm:0.31.1 [2b68c]
   ├─ Version: 0.31.1
   │
   └─ Peer dependencies
      ├─ @edge-runtime/vm@* → ✘
      ├─ @types/edge-runtime__vm@* → ✘
      ├─ @types/happy-dom@* → ✘
      ├─ @types/jsdom@* → ✘
      ├─ @types/playwright@* → ✘
      ├─ @types/safaridriver@* → ✘
      ├─ @types/vitest__browser@* → ✘
      ├─ @types/vitest__ui@* → ✘
      ├─ @types/webdriverio@* → ✘
      ├─ @vitest/browser@* → ✘
      ├─ @vitest/ui@* → npm:0.31.1 [2b68c]
      ├─ happy-dom@* → ✘
      ├─ jsdom@* → npm:22.0.0 [2b68c]
      ├─ playwright@* → ✘
      ├─ safaridriver@* → ✘
      └─ webdriverio@* → ✘

@koistya
Copy link
Author

koistya commented May 26, 2023

As long as all the "vitest" peer dependencies in all Yarn workspaces match exactly, there is a single instance of "vitest" (virtual) package and Vitest works as expected.

@koistya koistya closed this as completed May 26, 2023
@koistya koistya reopened this May 31, 2023
@sheremet-va
Copy link
Member

Not sure what can be done here? Why does yarn create separate Vite instances?

@koistya
Copy link
Author

koistya commented Jun 1, 2023

@sheremet-va PnP is more strict in regard to dependency resolution, if the same module has different peer dependencies it cannot be safely used from under different workspaces with their own set of peer dependencies for such module, because of the potential side effects. E.g. one workspace has "happy-dom" dependency, while the other doesn't.

Root Workspace (vitest + @vitest/ui)
├── Workspace A (vitest)
└── Workspace B (vitest + happy-dom)

In a multi-workspace scenario (via vitest.workspace.ts), Vitest correctly loads "vitest" module from the target workspace, e.g. when running tests located in Workspace B, it uses "vitest" from Workspace B. This is good. But it doesn't work for pre-test hooks. So that when snapshots are loaded in one of the pre-test hooks, Vitest uses a vitest instance loaded from a different workspace or realm (e.g. from the Root Workspace instead of Workspace B). I assume this can be easily fixed.

https://yarnpkg.com/advanced/pnp-spec#virtual-folders

@sheremet-va
Copy link
Member

sheremet-va commented Jun 1, 2023

I assume this can be easily fixed.

"easily" is a stretch, but I think I can find a way to fix that. Vitest always loads the "test runner" from the root Vitest:

const { run } = await import(pathToFileURL(resolve(distDir, 'entry.js')).href)

If we can resolve it for each workspace, then it should fix the error.

@sheremet-va sheremet-va self-assigned this Jun 1, 2023
@koistya
Copy link
Author

koistya commented Jun 1, 2023

@sheremet-va Interestingly, when tests are being executed Vitest uses the correct instances of "vitest", but this particular hook is being run from under the (root?) instance of vitest:

https://github.com/vitest-dev/vitest/blob/94668725e8d7bdfd5ed0f09b3c167d738bb114a0/packages/vitest/src/runtime/runners/test.ts#L54C9-L69

@sheremet-va
Copy link
Member

sheremet-va commented Jun 1, 2023

@sheremet-va Interestingly, when tests are being executed Vitest uses the correct instances of "vitest", but this particular hook is being run from under the (root?) instance of vitest:

9466872/packages/vitest/src/runtime/runners/test.ts#L54C9-L69

Vitest always uses the root worker to run tests (we can't change that), so distDir is always the root Vitest dir.

const workerPath = pathToFileURL(resolve(distDir, './worker.js')).href

But when it runs tests, Vitest is resolved relative to the file it is imported from.

If we resolve entry to the correct Vitest package, this should fix the error. You can already try this with something like this:

const rootHtml = join(config.root, 'index.html')
const { id: vitestEntry } = await rpc().resolveId('vitest/dist/entry.js', rootHtml, 'node')
const { run } = await import(vitestEntry)

@koistya
Copy link
Author

koistya commented Jun 13, 2023

@sheremet-va I'm testing with Vitest 0.32.0 and this issue still remains:

https://github.com/koistya/vitest-workspaces

$ yarn install
$ yarn test

 FAIL  |api| index.test.ts > foo
TypeError: Cannot read properties of undefined (reading 'match')
 ❯ api/index.test.ts:5:17
      3| 
      4| test("foo", () => {
      5|   expect(foo()).toMatchInlineSnapshot('"OK"');
       |                 ^
      6| });
      7| 

@sheremet-va sheremet-va reopened this Jun 13, 2023
@hakubo
Copy link

hakubo commented Sep 14, 2023

@sheremet-va We're on Vitest 0.34.4 and this issue still remains.

We're using NX to orchestrate our monorepo.

As a workaround I found that I can remove vitest from root package.json - and it looks like it's working. But then of course I get:

(node:15252) [MODULE_NOT_FOUND] Error: @nx/vite tried to access vitest (a peer dependency) but it isn't provided by your application; this makes the require call ambiguous and unsound.
(Use `node --trace-warnings ...` to show where the warning was created)

But is very brittle for example
as soon as I add: 'vitest-fail-on-console' - it breaks again.

@dallonf
Copy link

dallonf commented Sep 26, 2023

I just updated to v0.34.5 with pnpm and seeing something very similar. I can try to set up a repro if that would help...

@sheremet-va sheremet-va removed their assignment Sep 26, 2023
@dallonf
Copy link

dallonf commented Sep 26, 2023

Update: ah, I just got a mismatch between vitest core and @vitest/coverage-v8, because I forgot the latter was in my repo. Upgrading them together helped. However, it was very difficult to track down the problem and some better error reporting would be very nice!

@sheremet-va sheremet-va added the p3-minor-bug An edge case that only affects very specific usage (priority) label Oct 2, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Oct 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
p3-minor-bug An edge case that only affects very specific usage (priority)
Projects
None yet
4 participants