-
-
Notifications
You must be signed in to change notification settings - Fork 83
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
No vitest config detected on yarn + pnp + nx projects #285
Comments
Will probably be fixed by #253, and it should also increase the stability I believe. |
#253 doesn't support pnp currently. |
@sheremet-va I took a look into it and made some progress. However, when running vitest through the .pnp.cjs files, we need to have all vitest dependencies in the package.json. This should not be necessary. I need to dig deeper to understand what can be done. For a minute, I thought it would be easier to fallback to yarn for the PnP case. I will try to find some time to work on it, but I might need some help. Would it be ok if I open a draft PR to discuss solutions if I do not make progress here? |
Sure, give it a shot. The current approach was me trying to make it work, but I wasn't able to crack it before the release. |
@sheremet-va I pushed the PR and added comments. It fails because yarn requires dependencies, but I have not yet understood the differences between running the process through yarn and running it in Also, do you have a link for the findNode API you mentioned in the comments? I first thought it was from the Yarn APIs, but I did not find it. |
Maybe we can use |
@sheremet-va Ok, I will try that. I was trying to skip running yarn directly, but I had no success. I will find some time today and/or tomorrow to push a solution with |
Also how about Something like |
@sheremet-va, @hi-ogawa, here's a bit of the status on the task: I tried both, and they sort of work, but I am having trouble running them through the |
@saulotoledo Thanks for investigating this further. Yeah, I see now One idea I'm thinking is maybe we can sniff necessary env vars by spawning extra process like For example, I'm seeing this when running
|
Hi @hi-ogawa. Thanks for your suggestions. I will check them carefully. Line 63 in daf97d4
However, when using the PnP loader, the direct import complains about the non-declared dependencies from vitest in the project's The issue with the dependencies in
|
Can't we just import |
@sheremet-va Do you mean not setting the |
I just saw @hi-ogawa's review. I will have time to work on it in a few minutes. |
Sorry about my comment #285 (comment). I guess that one is obsolete as I commented in your PR afterwords #316 (comment). Let's continue discussion on your PR. |
Should be fixed in #456 |
Describe the bug
No tests are detected on Yarn + PnP projects. The logs show the following lines
Reproduction
I cannot share the project code I used to verify it, but I believe the investigation below might be enough to highlight the problem.
The function https://github.com/vitest-dev/vscode/blob/main/src/pure/utils.ts#L38 is triggered at
vscode/src/config.ts
Line 81 in 4283697
vscode/src/config.ts
Line 84 in 4283697
The proposed solutions:
Additionally, there is an issue with the execution of
chunksToLinesAsync(child.stdout)
atvscode/src/pure/utils.ts
Line 113 in 4283697
System Info
Used Package Manager
yarn
Validations
The text was updated successfully, but these errors were encountered: