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

onMount isn't called when rendering components #222

Closed
Tracked by #296
connerdassen opened this issue Jun 8, 2023 · 26 comments
Closed
Tracked by #296

onMount isn't called when rendering components #222

connerdassen opened this issue Jun 8, 2023 · 26 comments

Comments

@connerdassen
Copy link

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
Copy link
Collaborator

yanick commented Jun 8, 2023

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
Copy link
Author

connerdassen commented Jun 9, 2023

@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
Copy link
Collaborator

yanick commented Jun 9, 2023

[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
Copy link
Collaborator

yanick commented Jun 9, 2023

(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
Copy link
Author

connerdassen commented Jun 9, 2023

@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
Copy link
Collaborator

yanick commented Jun 9, 2023

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
Copy link
Collaborator

yanick commented Jun 9, 2023

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

@connerdassen
Copy link
Author

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

@skokenes
Copy link

skokenes commented Jun 12, 2023

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
Copy link
Collaborator

yanick commented Jun 13, 2023 via email

@yanick
Copy link
Collaborator

yanick commented Jun 13, 2023 via email

@connerdassen
Copy link
Author

connerdassen commented Jun 13, 2023

@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
Copy link
Collaborator

yanick commented Jun 13, 2023 via email

@paoloricciuti
Copy link

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
Copy link

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

@yanick
Copy link
Collaborator

yanick commented Jul 28, 2023

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
Copy link

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
Copy link
Collaborator

yanick commented Jul 28, 2023

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
Copy link

opswiz commented Nov 4, 2023

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
Copy link
Collaborator

mcous commented Jan 25, 2024

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
Copy link

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
Copy link

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()]
})

@paoloricciuti
Copy link

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
Copy link
Collaborator

mcous commented Jan 25, 2024

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?

  • The "default" config is an empty list
  • Adding conditions: ['browser'] seems to prepend browser to the conditions added by vite-plugin-svelte and vitest 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
Copy link
Collaborator

mcous commented Feb 2, 2024

@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!

@yanick
Copy link
Collaborator

yanick commented Feb 2, 2024

💯

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants