window.addEventListener overwritten by on: directive, :Window not working correctly

[coussej]

Hi, I came across the following situation:

I used a <:Window /> component to listen to some events like this:

<:Window on:load="router()" on:hashchange="router()" />

Svelte generates the following code for the above:

function onwindowload ( event ) {
	component.router();
}
window.addEventListener( 'load', onwindowload );

function onwindowhashchange ( event ) {
	component.router();
}
window.addEventListener( 'hashchange', onwindowhashchange );

This worked perfectly, until somewhere else in my application (in a different component) I added the following code:

<button on:click='login()'>Login</button>

This resulted in error in the (previously working) :Window component:

The on: directive here resulted in the compiler adding the following javascript to the output. Both functions overwrite the default functions on the window object.

function addEventListener ( node, event, handler ) {
	node.addEventListener( event, handler, false );
}

function removeEventListener ( node, event, handler ) {
	node.removeEventListener( event, handler, false );
}

The code generated from the :Window component calls addEventListeren with a string as first argument, hence the ...is not a function error.

Is this a bug in the :Window component or am I doing something wrong here? I'm using svelte 1.22.1 with rollup (es format).

[Conduitry]

Oh, ha, that's interesting. It took me a bit to put this together. So the addEventListener helper function is overwriting the regular window.addEventListener method because all the functions and whatnot that Svelte's generating are getting stuck on the global scope. I'm not sure whether that can be considered expected behavior, because I'm not sure how to get browsers to run code as a module instead of as a script, or how the global scope is handled if you do that.

What I would suggest is to just use the 'iife' output format in Rollup. This wraps the bundled code in an IIFE, which prevents any of the identifiers inside from leaking out into the global scope, which would also let you minify better because all of those names could be mangled.

[Rich-Harris]

@Conduitry is spot on — it's the format: 'es' that's causing this problem. es is designed for distributing libraries as modules, or (eventually) targeting browsers that natively support modules.

I wonder if we should rename those helpers. On the one hand, it would prevent errors like this. On the other, it would make it harder to detect errors like this...

[coussej]

Yeah, compiling as an iife solves the issue. Personally, I would prefer renaming the helpers, both to avoid overriding the global ones and to avoid confusion, but that's just my two cents :-)

[PaulBGD]

So I had this issue too with Electron, which internally uses some of these global functions. IIFE does solve the issue, but I feel like we should solve it within Svelte too. Maybe we could define addEventListener/removeEventListener as already used identifiers?

[constgen]

This is not the first and not the last time this issue rises.

[Rich-Harris]

As of 1.22.3, the helpers are called addListener and removeListener. You should still avoid format: 'es' though unless you're bundling a library for distribution or targeting Canary/Safari TP 😀

[blurymind]

with addListener or addEventListener => still crashes with same error :/

[Conduitry]

@blurymind I'm not sure why you'd still be seeing this issue - Can you give more details about how we can reproduce it?