Skip to content
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

[Vitest] When using Preact and Svelte Vite plugins together, Preact tests fail #581

Closed
molily opened this issue Jan 24, 2023 · 6 comments · Fixed by #582
Closed

[Vitest] When using Preact and Svelte Vite plugins together, Preact tests fail #581

molily opened this issue Jan 24, 2023 · 6 comments · Fixed by #582
Labels
bug Something isn't working triage Awaiting triage by a project member

Comments

@molily
Copy link

molily commented Jan 24, 2023

Describe the bug

Hi, this issue already has been discusesd in the Vitest repo:

vitest-dev/vitest#737

I have a Vite project with both Preact and Svelte component and want to test both using Vitest. Minimal example:

https://github.com/molily/vitest-test/tree/preact-and-svelte

Preact tests are green if @preact/preset-vite is loaded solely. If @sveltejs/vite-plugin-svelte is added, Svelte tests are green but Preact tests fail. There is no problem in Vite (dev mode or build) alone, only in Vitest the plugins clash.

The Preact and Vitest maintainers were involved and together solved an import issue, but the problem seems to remain. As far as I understand, the problem arises because Vitest sets mainFields: [] whereas vite-plugins-svelte sets mainFields: ['svelte', 'module', 'jsnext:main', 'jsnext'].

Is there any way that both plugins can play along in the Vitest environment? I only have a superficial understanding of the Vite / Vitest / Vite plugin internals, so I'm asking here. Thank you very much!

Reproduction URL

https://github.com/molily/vitest-test/tree/preact-and-svelte

Reproduction

  1. Check out repository & branch https://github.com/molily/vitest-test/tree/preact-and-svelte
  2. pnpm i
  3. npx vitest run

Logs

FAIL  src/Counter.test.jsx > renders the count
TypeError: Cannot read properties of undefined (reading '__H')
 ❯ d node_modules/.pnpm/preact@10.11.3/node_modules/preact/hooks/dist/hooks.module.js:2:321

 ❯ y node_modules/.pnpm/preact@10.11.3/node_modules/preact/hooks/dist/hooks.module.js:2:455
 ❯ Module.useState node_modules/.pnpm/preact@10.11.3/node_modules/preact/hooks/dist/hooks.module.js:2:424
 ❯ d.constructor src/Counter.jsx:6:29
 ❯ d.render node_modules/.pnpm/preact@10.11.3/node_modules/preact/src/diff/index.js:554:14
 ❯ diff node_modules/.pnpm/preact@10.11.3/node_modules/preact/src/diff/index.js:203:14
 ❯ diffChildren node_modules/.pnpm/preact@10.11.3/node_modules/preact/src/diff/children.js:137:3
 ❯ diff node_modules/.pnpm/preact@10.11.3/node_modules/preact/src/diff/index.js:225:4
 ❯ P node_modules/.pnpm/preact@10.11.3/node_modules/preact/src/render.js:39:2
 ❯ node_modules/.pnpm/@testing-library+preact@3.2.3_preact@10.11.3/node_modules/@testing-library/preact/dist/esm/pure.mjs:48:7

System Info

Node: 16.16.0 (also happens on 18.13.0)
@sveltejs/vite-plugin-svelte: 2.0.2
svelte: 3.55.1
vite: 4.0.4
vitest: 0.28.1
@molily molily added bug Something isn't working triage Awaiting triage by a project member labels Jan 24, 2023
@dominikg
Copy link
Member

dominikg commented Jan 24, 2023

workaround: you can use a separate vite plugin to remove the "module" value from the config again after vite-plugin-svelte added it.
https://stackblitz.com/edit/github-wvcjlz?file=vite.config.js

The reason we add this is that vite does not retain it's default values when you add your own value to resolve.mainFields, which is also the reason why vitest can just return an empty array in the config hook to get rid of the defaults, which is different from other vite config values that are merged.

If vitest can only work without this value present it would be best for them to set up a plugin in the "post" phase that checks the config value for resolve.mainFields and removes module there instead of returning an empty array in the "pre" phase, hoping that no other plugin adds it.

But ultimately i'm not sure why this is needed at all, vitests premise is that it works with vite config, not resolving the module field which is a default behavior of vite seems counter-intuitive to that.

Note that vite 4.1 is going to change the behavior for resolving mainFields, prefering exports map if it exists vitejs/vite#11595 - which would change the way preact is resolved https://github.com/preactjs/preact/blob/master/package.json#L12

@molily
Copy link
Author

molily commented Jan 25, 2023

Thank you very much!
@sheremet-va If you have some time, could you comment on the reasons behind vitest's logic?

The workaround makes the error in Preact disappear but leads to onMount of Svelte components not being called in the Vitest env. 🤪 It appears that Svelte components are then compiled in SSR mode. Setting transformMode: web for .svelte files explicitly does not help either.

Sorry to bother everyone involved, but I hope I can bring you experts together to find a solution. 😅 The alternative I currently see is to split the Svelte/Preact tests and generate three Vite configs (1. dev/prod with all plugins, 2. Vitest Svelte, 3. Vitest Preact), which kind of foils the goal of a unified Vite infrastructure for me.

Note that vite 4.1 is going to change the behavior for resolving mainFields, prefering exports map if it exists

Thanks. Unfortunately, I'm seeing the same behavior with 4.1.0-beta.0 as in 4.0.4.

@dominikg
Copy link
Member

onMount not being executed in component tests with vitest+svelte seems like a different issue and i'd prefer not to mix them.
Can confirm that transformMode web does not seem to have an effect, even if you also include .svelte.test.js files, though i am not sure if this is on vitest or maybe the way testing library svelte's render works.
It would be great if you filed a separare issue for that with a minimal reproduction that does not involve preact.

@sheremet-va
Copy link

onMount not being executed in component tests with vitest+svelte seems like a different issue and i'd prefer not to mix them.
Can confirm that transformMode web does not seem to have an effect, even if you also include .svelte.test.js files, though i am not sure if this is on vitest or maybe the way testing library svelte's render works.
It would be great if you filed a separare issue for that with a minimal reproduction that does not involve preact.

Probably because Vitest resolves Svelte to "module" file now, but the testing library is not inlined, so it fallbacks to default node resolution and gets resolved to SSR entry point where onMount is a noop. We already had this issue like 6 months ago.

This won't be a problem after Vitest adds a workaround for "module" field as we discussed in Discord.

@dominikg
Copy link
Member

the "module" field of svelte actually points at the browser version, but exports map has "ssr.mjs" for the node condition https://github.com/sveltejs/svelte/blob/master/package.json#L32

this usually works great and for application building this means you can use browser only code in onMount and be sure that it is never executed on the server because onMount is a noop in ssr.mjs and even unused imports of dependencies will just be removed during tree-shaking.

But vitest insisting on using node makes that a problem.

again the workaround for that could be to add the "browser" condition to resolve.conditions via an extra plugin, but ideally this would be done on a conditional basis. No clue how to do that without basically hijacking vite resolve. Which could be done with a pre plugin that uses this.resolve with extras... but ooh the maintenance nightmare....

@dominikg
Copy link
Member

Can confirm that unshifting "browser" to resolve.conditions works.
in a plugin config hook

if(process.env.VITEST) {
  config.resolve.conditions.unshift('browser');
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working triage Awaiting triage by a project member
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants