Import conditions not respected on main thread

[mrginglymus]

Describe the bug

Following from https://discord.com/channels/917386801235247114/1344949468579233805

If you have custom import conditions in a workspace package where the default points to a file that does not exist (say, a ./dist/file.js), and import conditions in the parent package, then running vitest with coverage and all will fail.

I'm not sure of the exact path towards the error, but the significant area appears to be that vite's resolveSubpathImports is called without the conditions specified in vitest config.

Reproduction

https://github.com/mrginglymus/vitest-coverage-import

System Info

-

Used Package Manager

yarn

Validations

• Follow our Code of Conduct
• Read the Contributing Guidelines.
• Read the docs.
• Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
• Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.
• The provided reproduction is a minimal reproducible example of the bug.

[mrginglymus]

A workaround is to add:

  ssr: {
    resolve: {conditions:['xxx']}
  },

to the vite config. However, that was extremely unintuitive!

[hi-ogawa]

Is coverage relevant to your repro? It looks like it's failing either way.

A workaround is to add:

  ssr: {
    resolve: {conditions:['xxx']}
  },

to the vite config. However, that was extremely unintuitive!

Do you mean { resolve: { conditions: ['xxx'] } } is intuitive, but { ssr: { resolve: { conditions: ["xxx"] } } is not? This is technically working intended as Vite feature https://v6.vite.dev/guide/migration.html#default-value-for-resolve-conditions since Vitest is essentially SSR mode.

I don't think Vitest should mess with this part further to avoid confusing Vite users, but if this turns out to be a common pitfall for pure Vitest users, then maybe we can elaborate something on documentation.

[mrginglymus]

It may not be, no - but it’s how the error finally exposed itself.

As I don’t use SSR anywhere else in my project I’ve never had to configure it.

Theres possibly some room for documentation here, but given how indirect the effects of this are, I’m not sure I would have found it.

I can also appreciate not wanting to unexpectedly modify users Vite config more than necessary. However, it seems very unlikely that when setting custom conditions users wouldn’t want them applied universally.

Three possible thoughts on how to improve the DX:

1. Add runtime warning if resolve conditions mismatch
2. add option in vitest to mirror client options to SSR options
3. add options to test section that get applied to both client and ssr config

I’m not sure all/any of these are good ideas…but something in the config to emphasise to client-only users that they also need to configure ssr would help a lot

[mrginglymus]

Also relevant is that it’s surprising that nothing exploded earlier - without the custom condition, none of my imports would resolve, so I don’t understand how anything was working without that config.

[AriPerkkio]

This seems to fail even without code coverage too. Based on previous discussion, I thought this was about coverage.all failing due to not using the conditions.

To me this looks like it's working as intended, as @hi-ogawa mentioned in #7588 (comment). The root resolve.conditions is not used, while ssr.resolve.conditions is.

[mrginglymus]

This seems to fail even without code coverage too. Based on previous discussion, I thought this was about coverage.all failing due to not using the conditions.

In my real world usage, it only manifested under that very strange and specific set of conditions - that it only manifested on linux, not windows, was extra weird.

My concern here is that perhaps something is silently swallowing a lot of errors - without my custom condition, none of the files exist, so I'd expect something to explode a bit earlier on in the process.

[mrginglymus]

Wow. So on the repro it does fail fast. Now the question is why is it not failing in my real-world code!

[mrginglymus]

Ah - it seems like adding browser mode is the key difference here. It does not reproduce with brwoser mode enabled

[mrginglymus]

I've updated the repro. I've simplified the config as best I can (removing coverage, conditional imports). So now it's a simple conditional export in use.

The vitest config is currently in the state I have it - without having configured ssr.resolve.

If you run yarn test (ie, without browser) then it fails fast, which is good and correct and makes it much more likely for the user to identify the error.

If you run yarn test --browser then it doesn't fail, and the fact that you've misconfigured ssr only becomes apparent in very obscure circumstances.

[sheremet-va]

When running tests in Node.js, if the environment is set to node (the default), it follows ssr options. If it's happy-dom or jsdom, then it uses top level resolve field

[mrginglymus]

Gotcha. Then there's a bug in istanbul (or vitests usage of it). I've updated the repro again, this time with browser and coverage.

If you run yarn test as things currently stand, it will fail when collecting coverage.

Either one of these three things will make it pass when collecting coverage:

1. set all to false
2. set provider to v8
3. add ssr.resolve.conditions

But the first two I don't want to do, and the last one feels like I shouldn't have to do - unless I should have to do, in which case it probably needs documenting somewhere.