`SvelteSet` does not behave correctly in Vitest

[abdel-17]

Describe the bug

The following test fails.

import { SvelteSet } from "svelte/reactivity";
import { expect, test } from "vitest";

class Group {
	selected = new SvelteSet<string>();
}

class Item {
	group: Group;
	id: string;

	constructor(group: Group, id: string) {
		this.group = group;
		this.id = id;
	}

	#selected = $derived.by(() => this.group.selected.has(this.id));

	get selected() {
		return this.#selected;
	}

	set selected(value: boolean) {
		if (value) {
			this.group.selected.add(this.id);
		} else {
			this.group.selected.delete(this.id);
		}
	}
}

test("Item.selected", () => {
	const cleanup = $effect.root(() => {
		const group = new Group();
		const item = new Item(group, "foo");

		expect(group.selected.has("foo")).toBe(false);
		expect(item.selected).toBe(false);

		item.selected = true;
		expect(group.selected.has("foo")).toBe(true);	// this is fine
		expect(item.selected).toBe(true);		// but this throws an error

		item.selected = false;
		expect(group.selected.has("foo")).toBe(false);
		expect(item.selected).toBe(false);
	});

	cleanup();
});

This issue only happens in Vitest. If you run the reproduction provided below, you will find that the same code works fine in the browser. Both values are the same, as expected.

Reproduction

https://github.com/abdel-17/svelte-vitest-set-bug-repro

Logs

No response

System Info

System:
    OS: macOS 15.0.1
    CPU: (8) x64 Intel(R) Core(TM) i5-1038NG7 CPU @ 2.00GHz
    Memory: 3.80 GB / 16.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 22.9.0 - /usr/local/bin/node
    Yarn: 1.22.22 - /usr/local/bin/yarn
    npm: 10.8.3 - /usr/local/bin/npm
    pnpm: 9.12.0 - /usr/local/bin/pnpm
    bun: 1.1.31 - /usr/local/bin/bun
  Browsers:
    Safari: 18.0.1

Severity

annoyance

[trueadm]

Do you have JSDOM set as your Vitest env?

[abdel-17]

Yes, it's enabled in the reproduction repo. Also, side note, it's not very clear from the docs that jsdom is needed for effects to work.

[trueadm]

Svelte 5 uses microtasks for scheduling, you can force things to be sync by using flushSync() after mutations:

item.selected = true;
flushSync();

However, given you're not using effects, I doubt that will change things here. You can try wrapping your logic in an effect (within the $effect.root, such as $effect.pre too).

[abdel-17]

I did try flushSync but it didn't work. I'm a bit confused though. The code is already wrapped in an effect root.

[trueadm]

Yeah it likely won't make a difference. I'll take a look into this on Monday :) I'm away without my laptop this weekend

[brunnerh]

I copied that code into my local SvelteKit project and it just worked. The project has barely any config changes regarding tests:

export default defineConfig((cfg) => ({
  // ...
  test: {
    // (added before a change was made that allows `.svelte.` in other positions)
    include: ['src/**/*.test.svelte.{js,ts}'],
  }
}));

[trueadm]

@brunnerh What if you do this without SvelteKit? i.e default sv?

[brunnerh]

sv does not have a non-SvelteKit option right now as far as I know (https://github.com/sveltejs/cli/issues/168)

[brunnerh]

Did npm create vite with the Svelte option and it works there as well.

[abdel-17]

Did npm create vite with the Svelte option and it works there as well.

Can you clone the reproduction repo and test if it doesn't work for you? That seems very strange...

[trueadm]

I see an issue though:

https://github.com/abdel-17/svelte-vitest-set-bug-repro/blob/main/src/demo.svelte.spec.ts

This path won't work AFAIK. The vite-plugin-svelte module only handles *.svelte.ts and *.svelte.js files.

Maybe try:

https://github.com/abdel-17/svelte-vitest-set-bug-repro/blob/main/src/demo.spec.svelte.ts

[brunnerh]

No, that was changed (as noted in my comment in my config). And it would error way earlier than it does due to the runes not being defined.

---

Tested the reproduction and it fails for whatever reason.

- Expected
+ Received

- true
+ false

 ❯ src/demo.svelte.spec.ts:42:25
     40|   item.selected = true;
     41|   expect(group.selected.has("foo")).toBe(true);
     42|   expect(item.selected).toBe(true);
       |                         ^
     43|
     44|   item.selected = false;
 ❯ update_reaction node_modules/.pnpm/[email protected]/node_modules/svelte/src/internal/client/runtime.js:327:56
 ❯ Module.update_effect node_modules/.pnpm/[email protected]/node_modules/svelte/src/internal/client/runtime.js:455:18
 ❯ create_effect node_modules/.pnpm/[email protected]/node_modules/svelte/src/internal/client/reactivity/effects.js:98:26
 ❯ Module.effect_root node_modules/.pnpm/[email protected]/node_modules/svelte/src/internal/client/reactivity/effects.js:222:17
 ❯ src/demo.svelte.spec.ts:33:8

[abdel-17]

I see an issue though:

https://github.com/abdel-17/svelte-vitest-set-bug-repro/blob/main/src/demo.svelte.spec.ts

This path won't work AFAIK. The vite-plugin-svelte module only handles *.svelte.ts and *.svelte.js files.

Maybe try:

https://github.com/abdel-17/svelte-vitest-set-bug-repro/blob/main/src/demo.spec.svelte.ts

Also tried it and it doesn't work.

[trueadm]

None of this makes sense, unless vite is doing some module isolation logic that means that the runtime and the tests aren't connected. Like I said, I have no computer till Monday :/

[benmccann]

I think the issue is that vitest is running the server runtime (https://github.com/vitest-dev/vitest/issues/2603). If you set resolve: { conditions: ['browser'] } then the test passes.

[benmccann]

I really don't know what the right solution is here. I guess that Svelte needs browser to get the reactive version. However, vitest specifies node because it's running on Node.js. In general, the browser condition is going to cause problems because you'll end up with code that uses window, etc. And in fact, I think vite won't even let you specify both browser and node at the same time.

It's also possible that you would have some luck with browser mode: https://vitest.dev/guide/browser/

[benmccann]

Hmm. What wasn't this an issue with Svelte 4 though?

[dummdidumm]

This sums it up well: https://github.com/testing-library/svelte-testing-library/issues/222#issuecomment-1909993331

We should add something along those lines to our documentation. The aliasing is a bit hacky, ideally we'd expose it in a robust way, but I'm not sure if there's a good way to do that via vite-plugin-svelte for example (maybe @dominikg has a good idea how to go about it).

[dominikg]

svelte testing library published a utility plugin for this, but if it is needed for unit testing runic code in .svelte.ts files too we should think about adding it to vps. needs options to disable it though bc there can be cases where you don't want the browser condition to apply (testing ssr, dependencies that use browser globals with it)

https://testing-library.com/docs/svelte-testing-library/setup#vitest

[paoloricciuti]

I need to open an issue on testing library about this but we should also include it in the docs: if you set resolve codition browser env-esm will rightrully return browser true...but this means that all the tests that don't need JSDom will fail because svelte is trying to access requestAnimationFrame when browser is true which is not defined. I solved this my mocking the function to a noop but that might not be enough if you also need to test transitions and animations (in the sense that you might need a more nuanced mock or just mock them on the specific pages where you don't need JSDom.

All of this imho is worth adding to the docs, it might be a bit complicated but i spent 4 hrs last day trying to figure out what was happening.

[dummdidumm]

The issue I referenced goes into detail about this, and proposes using alias as a solution (which in fact we're doing too in our test suite). @dominikg is this something we could somehow add to v-p-s, too, in a robust way?

Example:

User does

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

export default defineConfig({
  plugins: [svelte({ testing: { browserAlias: true } })] // name open to bikeshedding
});

That means that v-p-s does something like this

const pkg = JSON.parse(fs.readFileSync('svelte/package.json', 'utf8'));

export default defineConfig({
	resolve: {
		alias: [
			{
				find: /^svelte\/?/,
				customResolver: (id, importer) => {
					// For some reason this turns up as "undefined" instead of "svelte/"
					const exported = pkg.exports[id === 'undefined' ? '.' : id.replace('undefined', './')];
					if (!exported) return;

					// When running the server version of the Svelte files,
					// we also want to use the server export of the Svelte package
					return path.resolve(
						'packages/svelte',
						/* some conditional if people don't want this for certain files */
							? exported.default
							: exported.browser ?? exported.default
					);
				}
			}
		]
	}
}); 

[dominikg]

according to https://github.com/rollup/plugins/tree/master/packages/alias#custom-resolvers customResolver is a customized instance of rollup plugin node-resolve, not a resolve function. Also not sure what packages/svelte refers to here as our published package does not contain that.

The alias solution seems more hackish than adding the browser condition where needed. Is svelte really eagerly accessing rAF when browser is set or is that just for transitions/animations? and jsdom does not supply it? wtf

[paoloricciuti]

Is svelte really eagerly accessing rAF when browser is set or is that just for transitions/animations? and jsdom does not supply it? wtf

Eagerly in this module

https://github.com/sveltejs/svelte/blob/430c2bb129214aed17ca27db8bef5c9e9c897428/packages/svelte/src/internal/client/timing.js#L6

which is re-exported in /client/internal

https://github.com/sveltejs/svelte/blob/430c2bb129214aed17ca27db8bef5c9e9c897428/packages/svelte/src/internal/client/index.js#L148

But JSDom actually supply it...the problem is when you have a file that doesn't need JSDom (because it's just testing some utility for example but that utility is exported from a file where svelte is also imported from some unrelated stuff)

[dominikg]

https://github.com/sveltejs/svelte/issues/12133 mentions you can override raf.tick.

why do we rely on BROWSER instead of requestAnimationFrame ?? noop though ?

[brunnerh]

That would throw a reference error, you would at least need:

globalThis.requestAnimationFrame ?? noop
// or
typeof requestAnimationFrame == 'undefined' ? noop : requestAnimationFrame

[paoloricciuti]

#12133 mentions you can override raf.tick.

why do we rely on BROWSER instead of requestAnimationFrame ?? noop though ?

Yeah but the problem here is just accessing requestAnimationFrame which, as @brunnerh suggested, will throw on non JSDom environments.

[benmccann]

Does jsdom mock browser globals like window and document?

why do we rely on BROWSER instead of requestAnimationFrame ?? noop though ?

One reason is for smaller bundle size. You're making the browser runtime larger if you have to add requestAnimationFrame ?? noop instead of just being able to assume it.

[paoloricciuti]

Does jsdom mock browser globals like window and document?

I think it does but again the problem is for the files where you don't have to use JSDom because is just js.

[benmccann]

Really what we want is a way to say that we're running browser code in Node.

What if we had testing library svelte specify resolve: { conditions: ['browser', 'node'] } and then updated use of requestAnimationFrame to check BROWSER && !NODE?

[abdel-17]

Why was this issue closed? It still hasn't been fixed in the latest Svelte version.