Closed saulotoledo closed 3 weeks ago
It fails because yarn requires dependencies https://github.com/vitest-dev/vscode/issues/285#issuecomment-2013624894
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")
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
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
?
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.
@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 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
@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.
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!
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.
Support added in https://github.com/vitest-dev/vscode/pull/456
Closes #285.