Name collision with globals in files named Object.svelte, Error.svelte, console.svelte…

[trbrc]

A component file named Object.svelte, causes name collisions with the global Object.

Repro: https://svelte.dev/repl/dc1ccb7cbd8f47df8aa4ea4f3f318a0d?version=3.4.4

( ! )  Object$1.keys is not a function

This wouldn't be unexpected if users explicitly named components, but when they are named by the compiler based on the file name you'd expect it to deal with this.

[trbrc]

Looks like you run into the same problem with some other globals, e.g. Error.svelte or console.svelte. The conflict appears to happen both in the compiled output of the component (which includes e.g. class Error extends SvelteComponentDev {...}), as well as in the importing component if it does import Error from './Error.svelte';.

[Conduitry]

Related issue: #2612. I think it's a reasonable solution to make Svelte avoid these well-known globals when automatically choosing a component name from the filename, but to say that you're on your own if you explicitly choose one for the component name.

[trbrc]

Is it acceptable if the name of the component is changed? One possibility to avoid that could be to export these globals from svelte/internal and import them under an alias when a conflict can happen, so the component name is always identical to the file name.

[Conduitry]

That might work, I haven't thought all the way through what would happen exactly. Running this code in Node:

'use strict';
const Map = 42;
console.log(global.Map);

does display [Function: Map], so that's good.

I think the only things that would need updating are places where we introduce a new dependency on a global variable in the generated code. There is already a nice mechanism in place in the code generation for importing things from svelte/internal and renaming them as we do so if necessary.

svelte/internal itself would of course also have to be updated to export these values. The ESM spec does not allow you to export things that are implicitly coming from the global scope, so we'd have to write this in a cross-platform way that grabs them from either window or global.

[mrkishi]

Since the only contract we have on the API is:

name (default "Component"): string that sets the name of the resulting JavaScript class (though the compiler will rename it if it would otherwise conflict with other variables in scope). It will normally be inferred from filename.

I think it'd make more sense to just mangle names that conflict with globals, or even just always add Component to filename (eg. MapComponent or SomethingComponent). Otherwise we'll have to always keep internals in sync with globals.. and what if someone wants to use svelte outside the browser/node?

Use-cases that depend on a component having a specific class name should really define name when compiling, imo.

Am I missing a common pattern where comp_class.name is regularly used?

[Conduitry]

There are other situations besides the component name colliding with a useful global that could cause issues. For example, in the keyed each example, if you add something like let Map = 42; at the top of the component, this will break, as the component will then later try to do new Map() as part of the keyed each implementation. This is why I'm now thinking it's safer to make sure we have pristine(-ish) references to things like Map via window.Map or global.Map, independent of whatever local variables (or component names) are shadowing them. We can't do anything about people mucking with the global Map/etc references, but - for everything that is part of the generated code and which lives directly alongside code written by the user - we can at least make sure we're getting the actual global variable. If you look at the WIP commit I pushed, there aren't that many instances where this happens. It wouldn't surprise me if I'm still missing some instances, but there probably still won't be very many.

[Conduitry]

I don't know why typescript doesn't think that window has Error or Map or Object defined on it, so I'm currently using a nasty hack to get the build to succeed, but the tests on this branch (comparison with master) are currently passing. We'd obviously still need new tests and some stuff needs tidying up, but I think this is the core of a good idea.

[Conduitry]

Closed by #2963