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
614 stars 33 forks source link

onMount isn't called when rendering components #222

Closed connerdassen closed 7 months ago

connerdassen commented 1 year ago

When I render a simple component like this:

Test.svelte:

<script>
    import { onMount } from "svelte";
    onMount(() => console.log("Mounted"));
</script>

<span>Hello World</span>

and render it through render(Test);

It never prints "Mounted" to the console. Why is this, and how can I test onMount?

yanick commented 1 year ago

It does for me.

You might have to add a await tick() after the render, as the onMount is called after the component is rendered, according to the docs.

connerdassen commented 1 year ago

@yanick

It still doesn't print anything for me even after adding await tick(), if I add a top level console.log that does print, it's purely the onMount function... I'm using Vitest, I don't know if that would make a difference

yanick commented 1 year ago

[Vitest] It shouldn't affect anything.

Hmmm... It still works for me. Can you create a repo with the minimal amount of code that reproduce the problem?

yanick commented 1 year ago

(also, I'll drop offline for a few days starting tomorrow. If I don't answer, I'm not ghosting, I'm just without Internet. :-) )

connerdassen commented 1 year ago

@yanick I've uploaded an example to https://github.com/connerdassen/svelte-test-example I asked in the official svelte discord server and was told

The render method mounts Svelte components to jsdom, which does not invoke lifecycle methods like onMount. Lifecycle methods are on the list of things to avoid when testing: https://testing-library.com/docs/#what-you-should-avoid-with-testing-library

So now I wonder why it works for you...

yanick commented 1 year ago

So now I wonder why you it works for you...

Oooh, I'm using happydom, which might be why. I'll try to, uh, try with jsdom later on to see if it makes a difference.

yanick commented 1 year ago

So with vitest, no console print, with jest, I get one. As for why, I haven't the foggiest. O.o

connerdassen commented 1 year ago

@yanick I have discovered something, when importing onMount from "svelte" it does not run, but when importing from "svelte/internal" it does...

skokenes commented 1 year ago

I am hitting this bug as well. This seems like a bug, not WAD?

I understand you should not test internal lifecycle events, but thats not what we are trying to do here. Even if you are just trying to test the user-facing surface area of a component, if your component internally uses onMount, then you need that to run in order for the user to see the right thing so you can test the output.

Also, I've had no success using vitest with either jest or happy-dom. In both environments, onMount never runs

yanick commented 1 year ago

Okay, that's, as Spock would say, fascinating. As soon as I'll have a real connection back, I'll check what import stuff is happening under the blanket there. Thanks for the info!

On Sun, 11 Jun 2023, at 3:39 PM, connerdassen wrote:

@yanick https://github.com/yanick I have discovered something, when importing onMount from "svelte" it does not run, but when importing from "svelte/internal" it does...

— Reply to this email directly, view it on GitHub https://github.com/testing-library/svelte-testing-library/issues/222#issuecomment-1586313936, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAE34RZ4CV22C2AYSKIQRDXKYNFPANCNFSM6AAAAAAY75IQUM. You are receiving this because you were mentioned.Message ID: @.***>

yanick commented 1 year ago

Yeah, Im just one of the maintainers, not the original author, but I tend to agree. I'll see what i can do when i have a real connection back.

On Mon, 12 Jun 2023, at 12:53 PM, Speros Kokenes wrote:

I am hitting this bug as well. This seems like a bug, not WAD?

I understand you should not test internal lifecycle events, but thats not what we are trying to do here. Even if you are just trying to test the user-facing surface area of a component, if your component internally uses onMount, then you need that to run in order for the user to see the right thing so you can test the output.

— Reply to this email directly, view it on GitHub https://github.com/testing-library/svelte-testing-library/issues/222#issuecomment-1587704823, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAE34RZYNUDA7IQ4CH6FCLXK5CSPANCNFSM6AAAAAAY75IQUM. You are receiving this because you were mentioned.Message ID: @.***>

connerdassen commented 1 year ago

@skokenes seems to have figured it out in the svelte server, I'll reiterate:

Since the test is running in a Node environment instead of a browser environment, it uses the "node" exports from svelte which points to the ssr import path: https://github.com/sveltejs/svelte/blob/master/package.json

For SSR, life cycle methods do not run and are exported as a noop: https://github.com/sveltejs/svelte/blob/3bc791bcba97f0810165c7a2e215563993a0989b/src/runtime/ssr.ts#L1

Importing from "svelte/internal" bypasses this.

Solutions are either mocking onMount in every test:

vi.mock("svelte", async () => {
    const actual = await vi.importActual("svelte") as object;
    return {
        ...actual,
        onMount: (await import("svelte/internal")).onMount
    };
});

Or a custom alias in vite.config.ts:

resolve: process.env.TEST ? {
  alias: [{
    find: /^svelte$/,
    replacement: path.join(__dirname, "node_modules/svelte/index.mjs") 
   }]
} : {};
yanick commented 1 year ago

Excellent! At the very least that should be added to the docs. Thanks for capturing the info in this thread!

On Tue, 13 Jun 2023, at 6:10 AM, connerdassen wrote:

@skokenes https://github.com/skokenes seems to have figured it out in the svelte server, I'll reiterate:

Since the test is running in a Node environment instead of a browser environment, it uses the "node" exports from svelte which points to the ssr import path: https://github.com/sveltejs/svelte/blob/master/package.json

For SSR, life cycle methods do not run and are exported as a noop: https://github.com/sveltejs/svelte/blob/3bc791bcba97f0810165c7a2e215563993a0989b/src/runtime/ssr.ts#L1

Importing from "svelte/internal" bypasses this.

Solutions are either mocking onMount in every test:

vi.mock("svelte", async () => { const actual = await vi.importActual("svelte") as object; return { ...actual, onMount: (await import("svelte/internal")).onMount }; }); Or a custom alias in vite.config.ts:

resolve: process.env.TEST ? { alias: [{ find: /^svelte$/, replacement: path.join(__dirname, node_modules/svelte/index.mjs) }] } : {};

— Reply to this email directly, view it on GitHub https://github.com/testing-library/svelte-testing-library/issues/222#issuecomment-1588987135, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAE34UJ3YF3PY5VDPOT3YLXLA4C7ANCNFSM6AAAAAAY75IQUM. You are receiving this because you were mentioned.Message ID: @.***>

paoloricciuti commented 1 year ago

Excellent! At the very least that should be added to the docs. Thanks for capturing the info in this thread! On Tue, 13 Jun 2023, at 6:10 AM, connerdassen wrote: @skokenes https://github.com/skokenes seems to have figured it out in the svelte server, I'll reiterate: Since the test is running in a Node environment instead of a browser environment, it uses the "node" exports from svelte which points to the ssr import path: https://github.com/sveltejs/svelte/blob/master/package.json For SSR, life cycle methods do not run and are exported as a noop: https://github.com/sveltejs/svelte/blob/3bc791bcba97f0810165c7a2e215563993a0989b/src/runtime/ssr.ts#L1 Importing from "svelte/internal" bypasses this. Solutions are either mocking onMount in every test: vi.mock("svelte", async () => { const actual = await vi.importActual("svelte") as object; return { ...actual, onMount: (await import("svelte/internal")).onMount }; }); Or a custom alias in vite.config.ts: resolve: process.env.TEST ? { alias: [{ find: /^svelte$/, replacement: path.join(__dirname, node_modules/svelte/index.mjs) }] } : {}; — Reply to this email directly, view it on GitHub <#222 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAE34UJ3YF3PY5VDPOT3YLXLA4C7ANCNFSM6AAAAAAY75IQUM. You are receiving this because you were mentioned.Message ID: @.***>

Just for info: svelte/internal got removed in svelte 4 so I think despite the workaround this should be fixed...if you have any suggestion on where to start to look I can see If I can craft a PR

huntabyte commented 1 year ago

I'm still having this issue, does anyone have a workaround for it?

yanick commented 1 year ago

Can peeps here give a try to mocking onMount as it's one in #254? svelte still export svelte/internal, and it seems to work for me (and the CI environment)

paoloricciuti commented 1 year ago

Can peeps here give a try to mocking onMount as it's one in #254? svelte still export svelte/internal, and it seems to work for me (and the CI environment)

Actually on mount from internal is still exported but they removed the types to discourage the usage. I feel like we should try to stuck with this decision given that svelte 5 will probably remove them once and for all

yanick commented 1 year ago

Actually on mount from internal is still exported but they removed the types to discourage the usage. I feel like we should try to stuck with this decision given that svelte 5 will probably remove them once and for all

Do you have an alternative? I somewhat suspect we have a wee bit of runway before svelte 5, and we need something. Preferably not a sketchy something, but sketchy still beats nothing.

opswiz commented 10 months ago
export default defineConfig({
  plugins: [sveltekit()],
  resolve: {
    ...(process.env.VITEST ? {
      conditions: ['default', 'module', 'import', 'browser']
    } : null)
  }
});

For vitest, this should do the trick. Add browser to conditions. For my setup, regardless of the order it worked, not sure on others

mcous commented 8 months ago

I've been investigating this frustrating issue, and unfortunately we're quite limited in what we're able to do here in svelte-testing-library, because the issue itself lies with decisions around module resolution made in Vitest and/or vite-plugin-svelte.

Vitest intentionally adds a node condition to resolve.conditions, and vite-plugin-svelte only adds svelte, which leaves you with an effective config of:

{
  resolve: {
    conditions: ['svelte', 'node']
  }
}

These conditions take priority over Vite's default conditions, and the node condition causes Svelte's default SSR export to be loaded instead of its browser export. The SSR bundle has a no-op onMount (among other differences), causing weird discrepancies between your tests and the code that actually runs in the browser.

There are a few workarounds available, ordered from "best" to "worst". Examples below tested on the latest versions all libraries involved as of the time of writing. YMMV unless you're up to date.

  1. Add a browser condition to resolve.conditions when testing
    • Feels like the easiest and most future-proof option
    • Also feels like the most semantically correct: you're using Vitest to test browser code, so you're configuring Vitest to prioritize loading modules' browser code
    • Works well, unless you have dependencies whose browser bundles cannot be loaded into Node.js with a jsdom or happy-dom environment
    • This is the approach I will personally be taking
import { svelte } from '@sveltejs/vite-plugin-svelte'
import { defineConfig } from 'vite'

export default defineConfig(({ mode }) => ({
  plugins: [svelte()],
  resolve: {
    conditions: mode === 'test' ? ['browser'] : [],
  },
  test: {
    environment: 'jsdom',
  },
}
  1. Manually force Vitest to load the browser version of with a resolve alias
    • Not very future proof; locks your config to the internals of the Svelte package
    • More targeted than option (1)
    • If you're considering this option because the browser condition breaks some other dependency, consider swapping that dependency with an alias instead, because it's probably less important than your Svelte dependency
import path from 'node:path'

import { svelte } from '@sveltejs/vite-plugin-svelte'
import { defineConfig } from 'vite'

export default defineConfig({
  plugins: [svelte()],
  test: {
    environment: 'jsdom',
    alias: [
      {
        find: /^svelte$/,
        replacement: path.join(
          __dirname,
          './node_modules/svelte/src/runtime/index.js'
        ),
      },
    ],
  },
})
  1. Do option (2), but replace svelte with svelte/internal

    • The Svelte maintainers don't want you to do this
    • It's a bad idea, don't do it
  2. Use vi.mock, vi.spyOn, or similar

    • This is a misuse of Vitest's mocking APIs
    • Also a bad idea, don't do it
paoloricciuti commented 8 months ago

I've been investigating this frustrating issue, and unfortunately we're quite limited in what we're able to do here in svelte-testing-library, because the issue itself lies with decisions around module resolution made in Vitest and/or vite-plugin-svelte.

Vitest intentionally adds a node condition to resolve.conditions, and vite-plugin-svelte only adds svelte, which leaves you with an effective config of:

{
  resolve: {
    conditions: ['svelte', 'node']
  }
}

These conditions take priority over Vite's default conditions, and the node condition causes Svelte's default SSR export to be loaded instead of its browser export. The SSR bundle has a no-op onMount (among other differences), causing weird discrepancies between your tests and the code that actually runs in the browser.

There are a few workarounds available, ordered from "best" to "worst". Examples below tested on the latest versions all libraries involved as of the time of writing. YMMV unless you're up to date.

  1. Add a browser condition to resolve.conditions when testing

    • Feels like the easiest and most future-proof option
    • Also feels like the most semantically correct: you're using Vitest to test browser code, so you're configuring Vitest to prioritize loading modules' browser code
    • Works well, unless you have dependencies whose browser bundles cannot be loaded into Node.js with a jsdom or happy-dom environment
    • This is the approach I will personally be taking
import { svelte } from '@sveltejs/vite-plugin-svelte'
import { defineConfig } from 'vite'

export default defineConfig(({ mode }) => ({
  plugins: [svelte()],
  resolve: {
    conditions: mode === 'test' ? ['browser'] : [],
  },
  test: {
    environment: 'jsdom',
  },
}
  1. Manually force Vitest to load the browser version of with a resolve alias

    • Not very future proof; locks your config to the internals of the Svelte package
    • More targeted than option (1)
    • If you're considering this option because the browser condition breaks some other dependency, consider swapping that dependency with an alias instead, because it's probably less important than your Svelte dependency
import path from 'node:path'

import { svelte } from '@sveltejs/vite-plugin-svelte'
import { defineConfig } from 'vite'

export default defineConfig({
  plugins: [svelte()],
  test: {
    environment: 'jsdom',
    alias: [
      {
        find: /^svelte$/,
        replacement: path.join(
          __dirname,
          './node_modules/svelte/src/runtime/index.js'
        ),
      },
    ],
  },
})
  1. Do option (2), but replace svelte with svelte/internal

    • The Svelte maintainers don't want you to do this
    • It's a bad idea, don't do it
  2. Use vi.mock, vi.spyOn, or similar

    • This is a misuse of Vitest's mocking APIs
    • Also a bad idea, don't do it

Thanks for this...this is an incredible amount of research that i'm sure will help many (i'll try to cover this in This week in svelte) to spread awareness...i also feel like the first option might be the best solution. I don't think is there an effective difference between this and a vite plugin that add the condition right? In the end you will end up with an updated config with browser added to the resolve conditions.

dominikg commented 8 months ago

please note that setting resolve.conditions in vite config overrides the default conditions, so the suggestion of ['browser'] above basically disables all other conditions. A more proper way to do this - including a more proper check for vitest - is to use a vite plugin with a config hook,

https://github.com/vitest-dev/vitest/issues/2834#issuecomment-1429707803

// ...
const vitestBrowserConditionPlugin = {
  name: 'vite-plugin-vitest-browser-condition',
  config({resolve}) {
    if(process.env.VITEST) {
      resolve.conditions.unshift('browser'); 
    }
  }
}
export default defineConfig({
  // ...
  plugins: [vitestBrowserConditionPlugin,svelte()]
})
paoloricciuti commented 7 months ago

please note that setting resolve.conditions in vite config overrides the default conditions, so the suggestion of ['browser'] above basically disables all other conditions. A more proper way to do this - including a more proper check for vitest - is to use a vite plugin with a config hook,

vitest-dev/vitest#2834 (comment)

// ...
const vitestBrowserConditionPlugin = {
  name: 'vite-plugin-vitest-browser-condition',
  config({resolve}) {
    if(process.env.VITEST) {
      resolve.conditions.unshift('browser'); 
    }
  }
}
export default defineConfig({
  // ...
  plugins: [vitestBrowserConditionPlugin,svelte()]
})

Thanks @dominikg if vite doesn't automatically merge the configs this is definitely a better solution

mcous commented 7 months ago

please note that setting resolve.conditions in vite config overrides the default conditions, so the suggestion of ['browser'] above basically disables all other conditions

@dominikg I tested by logging the effective config using DEBUG=vite:config npx ... before my writeup, and got some interesting results that made me question this. I think the default cases might be built into Vite itself?

For example, a completely empty config produces:

export default defineConfig({})
# DEBUG=vite:config npx vite build
# ...
vite:config   resolve: {
vite:config     mainFields: [ 'browser', 'module', 'jsnext:main', 'jsnext' ],
vite:config     conditions: [],
# ...
# DEBUG=vite:config npx vitest --run
# ...
vite:config   resolve: {
vite:config     mainFields: [],
vite:config     conditions: [ 'node' ],
# ...

Adding just the svelte plugin produces:

export default defineConfig({
  plugins: [svelte()],
})
# DEBUG=vite:config npx vite build
# ...
vite:config   resolve: {
vite:config     mainFields: [ 'svelte', 'browser', 'module', 'jsnext:main', 'jsnext' ],
vite:config     conditions: [ 'svelte' ],
# ...
# DEBUG=vite:config npx vitest --run
# ...
vite:config   resolve: {
vite:config     mainFields: [ 'svelte', 'browser', 'module', 'jsnext:main', 'jsnext' ],
vite:config     conditions: [ 'svelte', 'node' ],
# ...

And finally, adding browser during test produces:

export default defineConfig(({ mode }) => ({
  plugins: [svelte()],
  resolve: {
    conditions: mode === 'test' ? ['browser'] : [],
  },
}))
# DEBUG=vite:config npx vite build
# ...
vite:config   resolve: {
vite:config     mainFields: [ 'svelte', 'browser', 'module', 'jsnext:main', 'jsnext' ],
vite:config     conditions: [ 'svelte' ],
# ...
# DEBUG=vite:config npx vitest --run
# ...
vite:config   resolve: {
vite:config     mainFields: [ 'svelte', 'browser', 'module', 'jsnext:main', 'jsnext' ],
vite:config     conditions: [ 'browser', 'svelte', 'node' ],
# ...
mcous commented 7 months ago

@yanick I think we're good to officially close this one out 🧹

@dominikg if you have a minute, could you check over my work above? I compared the resolve.conditions: ['browser'] approach with the suggested plugin + unshift approach and got identical results - a resolve.conditions array with browser prepended to the conditions added by VPS and Vitest. I'd be really curious to know if I'm misunderstanding something here!

yanick commented 7 months ago

:100:

hardingjam commented 2 months ago

@yanick I think we're good to officially close this one out 🧹

  • The docs now have setup instructions to ensure Vitest is properly configured to load Svelte's browser bundler
  • The docs now have an onMount FAQ entry
  • onMount tests have been added to our suite here

@dominikg if you have a minute, could you check over my work above? I compared the resolve.conditions: ['browser'] approach with the suggested plugin + unshift approach and got identical results - a resolve.conditions array with browser prepended to the conditions added by VPS and Vitest. I'd be really curious to know if I'm misunderstanding something here!

I have an existing test suite with some components that use onMount. Adding the example from the FAQ to vite.config.js breaks the tests on any components that import modules using require().

Error: require() of ES Module ... is not supported.

Is there a way to selectively ignore the resolve option on a case-by-case basis?

mcous commented 2 months ago

@hardingjam no, the Vite configuration is all or nothing. You could try one of the other options listed above rather than the plugin.

Using require in Svelte modules seems suspicious, though. Are you able to use import instead and configure Vite to prebundle your CJS dependencies into ESM?

karbica commented 2 weeks ago

This is still an issue when testing functions that call onMount which are intended to be used inside components. Even with the first solution provided by @mcous.

https://github.com/sveltejs/svelte/issues/13136#issue-2507408044

The motivation for testing functions that contain onMount is because they can contain runes and lifecycle calls all encapsulated and then be used in any components - exactly like React hooks.

From what I recall, @testing-library provides ways to test React hooks in isolation and I'm trying to do exactly that with Svelte 5.

The issue is reproduced here: https://stackblitz.com/edit/vitejs-vite-2fltxz?file=vite.config.ts

mcous commented 2 weeks ago

Hey @karbica, onMount cannot be used outside a .svelte component. This is not a Svelte Testing Library thing, this is just a Svelte thing. onMount relies on the implicit environment brought along by the Svelte component to function. It is inherently not encapsulated.

The way React Testing Library's hooks testing works is by wrapping the hook under test up in a fixture component. You can do the same to test things that require a component environment to function:

<!-- use-mouse-coords.test.svelte (or whatever name you want) -->
<script lang="ts">
import { useMouseCoords } from '../src/lib/use-mouse-coords.svelte.ts';

export const mouseCoords = useMouseCoords()
</script>
import { it, expect } from 'vitest';
import { render, screen } from '@testing-library/svelte';

import WithUseMouseCoords from './use-mouse-coords.test.svelte';

it('runs correctly when rendering a component', () => {
  const { component } = render(WithUseMouseCoords);

  expect(component.mouseCoords.x).toBe(0);
  expect(component.mouseCoords.y).toBe(0);
});

Personally, I find these fixture components annoying in my own tests. It's not really unit testing, and instead is integration testing the whole stack of my logic, the Svelte compiler, the Svelte runtime, and whichever DOM library the test suite is using (or the browser). If I find myself in a situation where I "need" to use a fixture component, I try to restructure my code so the logic I care about is more directly testable and avoid coupling to the view library/framework.

(One final bit of housekeeping: I see that you've added both the svelteTesting plugin to your Vitest config as well as the resolve.conditions customization from earlier in this thread. You do not need both, because the svelteTesting plugin sets the resolve conditions for you. See #359 if you're curious!)

karbica commented 2 weeks ago

Thank you @mcous for the great write up.

This is not a Svelte Testing Library thing, this is just a Svelte thing. onMount relies on the implicit environment brought along by the Svelte component to function. It is inherently not encapsulated.

I agree and follow this entirely. This isn't for Testing Library to solve, it's a consequence of the framework. I was curious if there was any remedy this library could provide. The fixture component (which is also performed for React hooks) is exactly what came to mind for these kinds of functions in Svelte. I'm not a fan of the ceremony involved and it breaks away from the concept of unit testing, as you mentioned. Thanks for clearing that up and confirming that.

One final bit of housekeeping: I see that you've added both the svelteTesting plugin to your Vitest config as well as the resolve.conditions customization from earlier in this thread. You do not need both, because the svelteTesting plugin sets the resolve conditions for you. See https://github.com/testing-library/svelte-testing-library/issues/359 if you're curious!

Thanks for calling this out. I really didn't like how the escape hatch was leaking into the configuration file. Good to know this library is handling that for us.

I appreciate you taking a look into my concern.