fix: yarn3+ pnp support

[saulotoledo]

Closes #285.

[hi-ogawa]

It fails because yarn requires dependencies #285 (comment)

I'm just testing this PR locally and what exactly is the error you're seeing currently?

It looks like there's an error when trying to import vitest/node via full path like this, but is Yarn pnp meant to work in that way? For example, running it directly with yarn node doesn't seem to work either.

// repro.mjs
await import("file:///home/hiroshi/.yarn/berry/cache/vitest-npm-1.4.0-465b7cb84c-10c0.zip/node_modules/vitest/dist/node.js")

yarn node repro.mjs

$ yarn node repro.mjs 

node:internal/process/esm_loader:34
      internalBinding('errors').triggerUncaughtException(
                                ^
Error: vitest tried to access pathe, but it isn't declared in its dependencies; this makes the require call ambiguous and unsound.

Required package: pathe (via "pathe/package.json")
Required by: vitest@npm:1.4.0 (via /home/hiroshi/.yarn/berry/cache/vitest-npm-1.4.0-465b7cb84c-10c0.zip/node_modules/vitest/dist/node.js)

    at makeError (/home/hiroshi/code/others/vitest-vscode/samples/yarn-pnp/.pnp.cjs:7353:34)
    at resolveToUnqualified (/home/hiroshi/code/others/vitest-vscode/samples/yarn-pnp/.pnp.cjs:9001:21)
    at Object.resolveToUnqualified (/home/hiroshi/code/others/vitest-vscode/samples/yarn-pnp/.pnp.cjs:9181:26)
    at resolve$1 (file:///home/hiroshi/code/others/vitest-vscode/samples/yarn-pnp/.pnp.loader.mjs:1993:31)
    at nextResolve (node:internal/modules/esm/hooks:865:28)
    at Hooks.resolve (node:internal/modules/esm/hooks:303:30)
    at handleMessage (node:internal/modules/esm/worker:196:24)
    at checkForMessages (node:internal/modules/esm/worker:138:28)
    at process.<anonymous> (node:internal/modules/esm/worker:157:5)
    at process.emit (node:events:518:28)

So, I'm wondering if we can just use import("vitest/node") and then yarn pnp loader will figure out everything without resolving path on our own?

From my testing, it looks like it's working after replacing this

vscode/src/worker/worker.ts

Line 77 in dad5884

const vitestMode = await import(meta.vitestNodePath) as typeof import('vitest/node')

with

  if (process.versions.pnp) {
    meta.vitestNodePath = "vitest/node";
  }
  await import(meta.vitestNodePath)

Also, loader setup here is not necessary since --require and --experimental-loader are already setup via execArgv?

vscode/src/worker/worker.ts

Lines 113 to 114 in dad5884

	if (data.loader)
	register(data.loader)

[sheremet-va]

Also, loader setup here is not necessary since --require and --experimental-loader are already setup via execArgv?

This option is deprecated in the latest Node.js version:

--experimental-loader` may be removed in the future; instead use `register()`

If we use yarn exec, then we can remove this since it will be handled by Yarn anyway.

[saulotoledo]

@sheremet-va Should we push the first version as of now and create another PR for removing the deprecated option? The proposal by @hi-ogawa just worked. I am investigating one issue with workspaces where vitest is not detected for yarn. I can push it here as soon as I have fixed it.

[sheremet-va]

@sheremet-va Should we push the first version as of now and create another PR for removing the deprecated option?

I don't see a reason why we shouldn't do it here

[saulotoledo]

@sheremet-va ok. I am checking the extension against another project I have, and it is not working. I will debug that first, and then I will get back to this case. One thing I just found is that gte() from semver is always returning true after minified, so it is not detecting versions below the minimum. For now I just updated the versions of the project to investigate my current issue, but it would be nice to check that issue as well. I checked it on GNU/Linux.

[saulotoledo]

Hi @hi-ogawa and @sheremet-va. I did not find time to work on this in the past few weeks, but I started looking into it today. Before I continue, I would like to check whether my understanding is correct below. Please also let me know if you have better suggestions.

From now on, the main idea is to use yarn directly instead of the Vitest API. I do not know the entire code of the extension, but my first idea after looking into the code is to spawn the yarn process at the worker.ts file. I would create two versions of the initVitest method, one using the Vitest API and one running yarn. My idea was to create a wrapper and keep the rest of the code untouched.

However, I noticed that the extension is somewhat "coupled" to the Vitest instance. For example, VSCodeReporter, createWorkerRPC and their dependencies depend directly on instances of Vitest (from the Vitest API). I would like to avoid creating an entirely new structure to run Yarn. It would be better to have a standard interface between those, or we risk missing functionality and having to support two different implementations.

That said, do you have any suggestions on how I should proceed? I welcome any ideas so I can provide a solution that is aligned with your project plans. Thanks in advance!

[sheremet-va]

The idea is not to use yarn to run tests, the idea is to use yarn exec to spawn the worker.js file, then the only thing you need to figure out is communication with that file.

[sheremet-va]

Support added in #456