testing-library / svelte-testing-library

:chipmunk: Simple and complete Svelte DOM testing utilities that encourage good testing practices
https://testing-library.com/docs/svelte-testing-library/intro
MIT License
620 stars 33 forks source link

Error when testing Svelte + Preact components #381

Closed molily closed 5 months ago

molily commented 5 months ago

Hello, first of all, thank you for this great project.

I have a Vite project with Preact and Svelte components, both tested with Vitest (vitest, jsdom, @testing-library/svelte, @testing-library/preact).

For some reason, this seemingly simple setup causes problems again and again. :-D Possible context: https://github.com/vitest-dev/vitest/issues/737 https://github.com/sveltejs/vite-plugin-svelte/issues/581

The Svelte tests alone work fine, the Preact tests alone also work fine. But when I'm adding import { svelteTesting } from '@testing-library/svelte/vite' to the Vite plugins, the Preact tests fail with this error:

FAIL  src/PreactTest.test.jsx [ src/PreactTest.test.jsx ]
SyntaxError: Named export 'options' not found. The requested module 'preact' is a CommonJS module, which may not support all module.exports as named exports.
CommonJS modules can always be imported via the default export, for example using:

import pkg from 'preact';
const { options } = pkg;

 ❯ src/PreactTest.test.jsx:1:1
      1| import { render } from '@testing-library/preact';

Steps to reproduce:

Minimal repo: https://github.com/molily/vitest-preact-svelte Clone, npm install, then run npm run test

I think the last time we had to bring the maintainers of Vitest and Preact together to solve this. I'm happy to open an issue on the Preact side if necessary. I'm wondering if there is anything that can be done on the Svelte Testing Library side.

Thanks a lot for your time.

mcous commented 5 months ago

Oh no! I can take some time to look into this today, thanks for setting up a repro.

Does anything improve if you set the resolveBrowser option of the svelteTesting plugin to false?

svelteTesting({
  // disable browser resolution condition
  resolveBrowser: false,
})

That commonjs stuff in your logs also seems like a bad sign!

mcous commented 5 months ago

@molily I traced the issue to an apparent error in how Preact creates and specifies its browser bundles: https://github.com/preactjs/preact/issues/4406

As a workaround, I was able to get the tests running with the following Vite config:

import { defineConfig } from 'vite';
import { svelte } from '@sveltejs/vite-plugin-svelte';
import { svelteTesting } from '@testing-library/svelte/vite';
import preact from '@preact/preset-vite';

// https://vitejs.dev/config/
export default defineConfig({
  plugins: [preact(), svelte(), svelteTesting()],
  test: {
    environment: 'jsdom',
    deps: {
      optimizer: {
        web: {
          enabled: true,
          include: ['@testing-library/preact'],
        },
      },
    },
  },
});
mcous commented 5 months ago

@molily Preact has indicated that the ticket I filed above is a wontfix, see the thread for more details. Since there's nothing we can do here that won't compromise Svelte testing, I'm going to do the same here.

Unfortunately, for Preact v10, this leaves us with a fundamental incompatibility with how the two projects specify their exports:

I recommend you either use the workaround I listed above or create separate suites for your Preact and Svelte components. Preact v11 may not have this same issue, but I have not tested it and it's not a production release yet

molily commented 5 months ago

Thanks a lot for your tremendous efforts – thanks for digging into this, discussing this with the Preact maintainer and explaining everything. I'm sorry for bringing this issue up again and again every six months for the last two years, bothering the maintainers of several projects. 🫣😅

Regarding this solution:

  deps: {
      optimizer: {
        web: {
          enabled: true,
          include: ['@testing-library/preact'],
        },
      },
    },

This still seems to cause an error in a Preact component with useState: https://github.com/molily/vitest-preact-svelte/blob/counter/src/Counter.jsx https://github.com/molily/vitest-preact-svelte/blob/counter/src/Counter.test.jsx

TypeError: Cannot read properties of undefined (reading '__H')
 ❯ h node_modules/preact/hooks/dist/hooks.module.js:2:164
 ❯ y node_modules/preact/hooks/dist/hooks.module.js:2:298
 ❯ Module.useState node_modules/preact/hooks/dist/hooks.module.js:2:267
 ❯ b.constructor src/Counter.jsx:5:29
      3|
      4| export function Counter() {
      5|   const [value, setValue] = useState(0);

What continues to work for me is the tip you gave in https://github.com/testing-library/svelte-testing-library/issues/222#issuecomment-1909993331, with all the constraints you described:

  alias: [
      {
        find: /^svelte$/,
        replacement: join(currentDir, 'node_modules/svelte/src/runtime/'),
      },
    ],

with

import { dirname, join } from 'node:path';
import { fileURLToPath } from 'node:url';
const currentDir = dirname(fileURLToPath(import.meta.url));

create separate suites for your Preact and Svelte components

I think this is the safest solution in the long term. 😕 Thanks again!

rschristian commented 5 months ago

This still seems to cause an error in a Preact component with useState:

TypeError: Cannot read properties of undefined (reading '__H')
 ❯ h node_modules/preact/hooks/dist/hooks.module.js:2:164
 ❯ y node_modules/preact/hooks/dist/hooks.module.js:2:298
 ❯ Module.useState node_modules/preact/hooks/dist/hooks.module.js:2:267
 ❯ b.constructor src/Counter.jsx:5:29
      3|
      4| export function Counter() {
      5|   const [value, setValue] = useState(0);

That error means you're loading two (or more) copies of Preact at once, when hooks by design (in both React & Preact) require to be loaded as singletons. Vitest seems to struggle with this quite a bit IME. That workaround is probably causing both the "browser" and "import" condition to be loaded simultaneously.

There's a fair number of threads regarding this over on the Vitest thread IIRC, though it sounds like you might already have a workaround.

I'm sorry for bringing this issue up again and again every six months for the last two years, bothering the maintainers of several projects.

Can't speak for Svelte, but as the maintainer for a fair few of those conversations on the Preact side of things, no need to apologize! It's not a bother at all. I (and most other OSS maintainers) are always happy to help those who are grateful/not rude.

Sorry we couldn't figure out a better solution for you though

mcous commented 5 months ago

@molily thanks for all the feedback on workarounds, I'm definitely keeping an eye on this to look for potential improvements. I use Svelte at work, but Preact is my library of choice for personal projects, so I selfishly would love this to be better, too.

If you're sticking with Svelte v4 and you don't mind certain limitations listed in #222, you can set the Svelte testing plugin to svelteTesting({ resolveBrowser: false }). This will remove browser from Vitest's resolve.conditions array. I think this will cause Preact's regular ESM export to be used consistently. Unfortunately, it will also cause Svelte's SSR code to be used. For a lot of component tests, though, this can be sufficient. Also unfortunately, this does not currently work with the Svelte 5 RC, if that's something that matters to you.

@rschristian thanks for your feedback over in https://github.com/preactjs/preact/issues/4406 and taking the time to hop in here, too. Please let me know if you think of anything I could do here - or conversations I could start/continue with the Svelte or Vitest folks - to improve interop here!