-
Notifications
You must be signed in to change notification settings - Fork 33
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
Comments
It does for me. You might have to add a |
It still doesn't print anything for me even after adding |
[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? |
(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. :-) ) |
@yanick
So now I wonder why 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. |
So with vitest, no console print, with jest, I get one. As for why, I haven't the foggiest. O.o |
@yanick I have discovered something, when importing onMount from "svelte" it does not run, but when importing from "svelte/internal" it does... |
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 Also, I've had no success using vitest with either jest or happy-dom. In both environments, |
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 <#222 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAE34RZ4CV22C2AYSKIQRDXKYNFPANCNFSM6AAAAAAY75IQUM>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
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 <#222 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAE34RZYNUDA7IQ4CH6FCLXK5CSPANCNFSM6AAAAAAY75IQUM>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
@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:
Or a custom alias in vite.config.ts:
|
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 |
I'm still having this issue, does anyone have a workaround for it? |
Can peeps here give a try to mocking |
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. |
export default defineConfig({
plugins: [sveltekit()],
resolve: {
...(process.env.VITEST ? {
conditions: ['default', 'module', 'import', 'browser']
} : null)
}
}); For vitest, this should do the trick. Add |
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 {
resolve: {
conditions: ['svelte', 'node']
}
} These conditions take priority over Vite's default conditions, and the 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.
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',
},
}
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'
),
},
],
},
})
|
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. |
please note that setting 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 |
@dominikg I tested by logging the effective config using
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 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' ],
# ... |
@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 |
💯 |
I have an existing test suite with some components that use onMount.
Is there a way to selectively ignore the resolve option on a case-by-case basis? |
@hardingjam no, the Vite configuration is all or nothing. You could try one of the other options listed above rather than the plugin. Using |
This is still an issue when testing functions that call sveltejs/svelte#13136 (comment) The motivation for testing functions that contain From what I recall, The issue is reproduced here: https://stackblitz.com/edit/vitejs-vite-2fltxz?file=vite.config.ts |
Hey @karbica, 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 |
Thank you @mcous for the great write up.
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.
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. |
@mcous Thanks for your great research and writeup! You suggest swapping dependencies broken by the |
for svelte5, i made an example using vitests workspace feature here: https://github.com/dominikg/vitest-example-svelte5 note that this uses A setup like this is going to be added to |
@dominikg that setup looks great! Let me know if there's anything on the testing-library side that could be updated to help the effort of getting this sort of thing into @Stadly see Dominik's suggestion above; my comment about aliases predates Vitest's workspace configuration option. To throw my extra 2 cents into the mix, I also recommend that you explore structuring your application code such that auth and other framework concerns don't leak down to the level of component tests, and instead are tested at much higher levels, like E2E tests in playwright. You may be able to bypass this Vitest problem entirely with more separation between your own interesting logic that you want to test and framework stuff that you wire into |
Thanks for the suggestion! I'm actually unit testing my |
When I render a simple component like this:
Test.svelte:
and render it through
render(Test);
It never prints "Mounted" to the console. Why is this, and how can I test
onMount
?The text was updated successfully, but these errors were encountered: