The exported names of the reactive Map, Set, and others conflict with platform globals

[ryanatkn]

Describe the problem

I like the functionality of svelte/reactivity's reactive classes that extend globals like Map and Set, and I appreciate terseness and aesthetics of naming them the same as the globals, but shadowing globals like this is troublesome.

Consider that you can't look at code and know if you're dealing with a reactive Svelte class or platform global without looking at imports:

class A {
  b = new Map(); // is this reactive? check the imports or intellisense
}
function create_c(): Set<string> {
  // does this return a reactive set? check the imports or intellisense
}

The two kinds of classes differ in behavior and performance. Adding an import changes the semantics of code that otherwise uses normal JS globals, which seems surprising. Code is therefore not portable across modules with different imports, which can silently cause bugs that tools like TypeScript can't help with. (as @hanszoons describes below) (can probably be mitigated, see replies) This ambiguity feels like the uncanny valley Svelte is trying to avoid as discussed in the feature's main issue.

This is editor-specific, but the intelliense isn't obvious. With the global:

var Set: SetConstructor 
new <string>(iterable?: Iterable<string> | null | undefined) => Set<string> (+1 overload)

When imported from svelte/reactivity:

(alias) new Set<string>(value?: Iterable<string> | null | undefined): Set<string>
import Set

The indications this is the reactive one are subtle until you learn the difference.

A workaround to most issues is aliasing yourself. Using import {Set as ReactiveSet}:

(alias) new ReactiveSet<string>(value?: Iterable<string> | null | undefined): ReactiveSet<string>
import ReactiveSet

But unless you create your own re-exports of svelte/reactivity, you lose autoimports because of the alias. You also lose autoimports in some cases if you don't alias, e.g. pressing ctrl+. on a Set to import it automatically from svelte/reactivity does not work, although it does include the import in the suggestion dropdown when typing out the global name.

To emphasize the point, the Svelte codebase internally prefixes them with Reactive presumably because the code uses regular maps and sets often, as is common in JS, and the reactive collections are referenced less. By conflicting with the globals for the convenience of authoring reactive UIs, it's creating fragmentation where some modules and codebases prefer aliasing or not. For un-aliased code, the default, you don't know if a class is reactive without checking the imports, and code cannot be moved around safely without carefully handling imports (can probably be mitigated, see replies).

Describe the proposed solution

The easy fix (a breaking change) is removing the export aliases:

// types/index.d.ts
// from this:
export { ReactiveDate as Date, ReactiveSet as Set, ReactiveMap as Map, ReactiveURL as URL, ReactiveURLSearchParams as URLSearchParams };
// to this:
export { ReactiveDate, ReactiveSet, ReactiveMap, ReactiveURL, ReactiveURLSearchParams };

Importance

would make my life easier

[david-plugge]

100% agree. Runes make reactivity more obvious, the classes however do not

[Antonio-Bennett]

Why don’t you just rename it when you import it?

[david-plugge]

Do you also need to rename the state rune to make it obvious? Its just about understanding reactivity when you quickly ready through the Code. No need to check for an import. The simple let declaration in svelte Files where replaced by runes for this exact reason (and many others)

[hanszoons]

The compiler/editor should at least warn when you are updating the non-reactive maps, sets etc. It does that already when you supposedly forgot to declare a variable with $state when you update it somewhere.

I have spent hours debugging a complex app, turned out i refactored out functions that use reactive maps to a seperate file that didn't import them. It is far, far too easy to forget this (and have your code fail silently).

[Rich-Harris]

Just to provide some reasoning for why things are the way they are (which isn't to say that they're set in stone):

• It aids discoverability. Without reading the docs in their entirety, you could completely miss ReactiveSet et al, whereas if you (perhaps assuming that built-in sets somehow work this way) type new Set and are prompted to auto-import from svelte/reactivity, you're more likely to learn about them
• It's clearer that the API matches the built-ins
• We have an ambition, not yet written up in public, to add reactive variants of things like window (so that you can read window.innerWidth reactively without the indirection of <svelte:window bind:innerWidth={w} />, even outside components). I'm not sure what that would look like in this world. reactiveWindow? Kinda ugly!

There are some potential mitigations for the issues you describe. For example we don't have any inline documentation on these classes, and that could easily be fixed:

Slightly trickier, due to how the types get generated (methods that have the same signature as the super class get omitted when TS generates the .d.ts file), but we could probably figure out a way to make it work on methods as well:

Admittedly you don't get any indication on the instance itself, only its constructor and methods.

It would also be possible to monkey-patch built-ins at dev time so that if you do something like {set.has(x)} it warns you that you probably wanted to use the reactive Set rather than the built-in. Of course, that would yield occasional false positives so we would at minimum need a way to opt out:

<!-- svelte-ignore non_reactive_set -->
<p>{set.has(x)}</p>

(Today, svelte-ignore only controls compiler warnings, but we could make it work with runtime warnings too.)

[ryanatkn]

Thanks for the in-depth reply, I understand better now. I think the point made by @hanszoons is the most important implication of this choice. (I later edited it into the OP) But if monkey-patching in dev to warn on usage could catch all silent errors, I was wrong to say code cannot be moved across modules safely.

My personal preference would be to reduce ambiguity and avoid the headaches of shadowing globals, even with window, but my opinions have a history of trending the direction Svelte goes. I'm re-exporting for now but I don't want to go against the grain.

Some thoughts:

• is shadowing globals a good practice in the long run?
• do the learning and aesthetic benefits outweigh the costs?
• it feels intrusive to have to write globalThis.Set when the reactive class is imported, or care about what's imported when using maps/sets/etc (though I think the downsides of getting it wrong will usually just be a small perf hit, but there's an ick factor - speculating, I think with shadowing we'll see over-use of the reactive collections in non-reactive contexts)
• the monkey-patch will add friction for users who choose to alias to silence the warnings unless it's somehow detected or added as an option
• maybe incompatible with runes for other builtins but $window looks better than reactiveWindow

The reactive version is currently assignable to the builtin, but this can be changed by extending the globals with TypeScript, although sometimes you may want the reactive version to be assignable to the builtin, like in helpers:

TypeScript REPL with assignment change

view TypeScript assignment change example

// Pretend this was imported from 'svelte/reactivity':
export class SvelteSet<T> extends Set<T> {
	// Existing properties like this make `Set` not assignable to `SvelteSet`:
	#sources = new Map();
	// Add a conflicting property to make `SvelteSet` not assignable to `Set`.
	// @ts-expect-error - this error is silenced because otherwise we couldn't extend the builtin `Set`
	readonly [reactive]: true; // comment this out to see the desired errors go away
}

// Extend the global with a conflicting type to make `SvelteSet` not assignable to `Set`:
declare global {
	interface Set<T> {
		readonly [reactive]: false; // comment this out to see the desired errors go away
	}
}
declare const reactive: unique symbol; // one way to "brand" types

export const a = new Set<string>();
export const b = new SvelteSet<string>();

export const b_from_a: SvelteSet<string> = a; // errors as desired without the brand
export const a_from_b: Set<string> = b; // requires the brand to get this error

takes_b(a); // errors as desired without the brand
takes_a(b); // requires the brand to get this error

function takes_a(a: Set<string>): void {}
function takes_b(b: SvelteSet<string>): void {}

This assignment change fixes some problems but causes new ones, like functions that don't care if it's reactive would require casting. There may only be downsides if the monkey-patching catches the same kinds of errors.

(aside: TypeScript 5.5 reports the unalised ReactiveSet in the error messages, not the exported Set or locally aliased SvelteSet, just another tooling quirk to fix or accept - the simple fix is removing Reactive, but is that desirable?)