search

sveltejs / vite-plugin-svelte

Svelte plugin for http://vitejs.dev/
MIT License
864 stars 105 forks source link

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

Closed molily closed 1 year ago

molily commented 1 year ago

Describe the bug

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

https://github.com/vitest-dev/vitest/issues/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
dominikg commented 1 year ago

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 https://github.com/vitejs/vite/pull/11595 - which would change the way preact is resolved https://github.com/preactjs/preact/blob/master/package.json#L12

molily commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

if(process.env.VITEST) {
  config.resolve.conditions.unshift('browser');
}
tal-faitlov commented 3 months ago

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

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

Another possible solution is to use aliasing

/// vite.config.ts
test: {
  alias: {
    'quasar': 'quasar/dist/quasar.client.js'
  }
}

In my case, quasar was getting resolved to its ssr package via the node entry

  "exports": {
    ".": {
      "types": "./dist/types/index.d.ts",
      "require": "./dist/quasar.server.prod.cjs",
      "node": "./dist/quasar.server.prod.js",
      "import": "./dist/quasar.client.js"
    },