-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
feat: add "strictESM" option to Node environment #2854
Conversation
@antfu do you approve of this idea as a whole? (refactor is probably needed) It bypasses Vite resolution, because it's unrestrictive, and I don't want to add this complexity to Vite itself. I think we should have some kind of a strict mode, because of this line in our documentation:
Currently, Vitest is too permissive and will not fail, when it would've in Node.js, which is a problem for a tool that aims to eliminate such issues. Also, with our current approach, we can give useful error messages when someone uses a syntax when it's not supported (implemented in this PR partially). |
While I think it make sense to have a strict mode, looking into the code, am I not sure if the complexity introduced to support it is worth it. I am a bit worried about if having this would create more problems as we will be in a middle ground that not 100% align with Node nor 100% align with Vite. I don't have a strong opinion to push it back tho. We can still have it if you think it's worth it. |
I also do not like introduced complexity. But I think it can be fixed with a refactor, this PR is a proof of concept currently.
Not sure about 100% with Vite statement here 🤔 Strict ESM should not be 100% aligned with Vite, because it has its own module resolution and transform pipeline. About 100% with Node: there is a need to have our own module mechanism to allow mocking with
I think it's worth it, but it's a hard task. The main problem is that we don't use Vite plugin pipeline here at all, but we still have these options in config file 🤷🏻 Which will be confusing for people. |
To me, the important part of Vitest is it uses the Vite pipeline. If we are going not to use the Vite pipeline, I would honestly consider it out-of-scope of Vitest. I think we could have a mode to be strict on global variables like |
Then we should not advertise Vitest as a "solid alternative even for projects not using Vite.", and have a badge somewhere to recommend other test runners in certain situations. |
We can also try to add configurations to Vite/improve existing ones/add our own plugin so they align with how Node ESM works (need for extensions for example). |
To me, I use Vitest to test a lot of Node projects, and I am completely fine with how it works right now. I think the statement itself has no problem - it's true you can test non-Vite projects. I think every tool has its own behavior and own caveat, not strictly aligning with Node's behavior doesn't change it. Probably only the node's builtin test can represent the 100% node behavior. Being able to mock a module might be considered a misalignment of ESM spec if you go very strict. And for sure, we could document other running as we are certainly not a tool for every use case. |
Being able to "simulate" node resolution as a standalone extension sounds like a good solution to me 👍 |
It's might be fine for you, but your code doesn't actually represent how it works for your users. The easiest example is misconfigured package.json. If you test Node.js code, it should at least fail where it suppose to, otherwise, there is no point in the tool.
We don't need to 100% follow it, but we should behave as closely to it as possible.
I do not like module mocking, but people seem to enjoy it, even though it's very "magical" and sometimes hard to predict how it behaves. But it is an intentional deviation from the spec that people expect. When their package works in Vitest, but doesn't work for users - that's a problem. I for example would like to test Vitest's node entry point, but I cannot do that because Vitest will always work, but it won't work for actual users. |
@sheremet-va @antfu still think this will get in? any major blockers? |
only time |
Superseded by #3995 |
Closes #2841
Closes #2198
TODO: