Skip to content
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

Reused element binding (bind:this) updates to null long after destruction #11920

Open
kaizjen opened this issue Jun 5, 2024 · 3 comments
Open

Comments

@kaizjen
Copy link

kaizjen commented Jun 5, 2024

Describe the bug

This might be too convoluted, see the REPL for a better explanation.

A component, after being destroyed, retains its bindings to the elements inside (via bind:this={element}). This wouldn't be a problem, but:

  1. if this component somehow updates its binding after it's been destroyed and
  2. it reuses its binding, either inside an {#each} block with the binding dependent on the index, or just reuses the variable
    then the component's element will be updated after the element has been destroyed and shouldn't have any rights to do so.

The 1st condition happens if the element has a outro transition. After the transition is done, the component schedules a binding callback via its {tag}_binding function. If, at this point, any update happens in the component, the destroyed element will update its binding with the component, which is unexpected behavior, since after the element is destroyed it shouldn't cause any updates in its component.

In the REPL, I proposed a solution, which may or may not be what Svelte needs. In short, it will replace the {tag}_binding function with:

let X_binding_active = true;
function X_binding($$value) {
	if (!X_binding_active) return;

	binding_callbacks[$$value ? 'unshift' : 'push'](() => {
		element = $$value;
		$$invalidate(0, element);
	});
}
$$self.$$.on_destroy.push(() => X_binding_active = false);

X is replaced by the element name. This code should be in the bind_this.js file on lines 29 and 80. I, unfortunately, haven't figured out how to make the X_binding_active variable named like that, because acorn spaghettifies its name 😅.

Anyway, to recap:
You need an element, which: 1) has a outro transition on itself or one of its children, 2) has a bind:this binding
And you need to 1) destroy the element, 2) update the component ($$invalidate) after the transition has played.

Edit: as I realized, you don't need multiple components

Reproduction

https://svelte.dev/repl/b1160a5db49943ac88bde30323cea348?version=4.2.17
Reproduction and even a possible solution inside the REPL (though duplicated here)

Edit: A much simpler test case can be found here.

There doesn't have to be an {#each}, i think, but this just serves as an example of why this bug can be disastrous.
Sorry if I overstated the Severity field below, by the way.

Logs

No response

System Info

System:
    OS: Windows 10 10.0.19045
    CPU: (12) x64 AMD Ryzen 5 3600 6-Core Processor
    Memory: 2.97 GB / 15.93 GB
Binaries:
    Node: 18.17.1 - ~\AppData\Roaming\node.EXE
    npm: 9.6.7 - ~\AppData\Roaming\npm.CMD

Tested both in Firefox and Chrome

Severity

application-breaking

@kaizjen kaizjen changed the title Component updating its exported binding (bind:this) to null after destruction Component updating its exported binding (bind:this) to null long after destruction Jun 5, 2024
@dummdidumm
Copy link
Member

This is fixed in Svelte 5. Because this is a real edge case I'm tempted to say we're not going to backport this.

@kaizjen
Copy link
Author

kaizjen commented Jun 8, 2024

@dummdidumm I disagree. I'm sorry for such a convoluted test case (I didn't have enough time to test out more possibilities), but here's a much simpler and probably a more believable situation:
https://svelte.dev/repl/5f80fe1b03494b5bafab5e738fee8e5e

I'd argue that reusing element bindings, although maybe uncommon, shouldn't be considered an edge case.

@kaizjen kaizjen changed the title Component updating its exported binding (bind:this) to null long after destruction Reused element binding (bind:this) updates to null long after destruction Jun 8, 2024
@laszlokorte
Copy link

laszlokorte commented Jul 3, 2024

Actually this seems to be not fixed in svelte5, at least not in production builds (vite build), whereas in dev mode (`vite dev') it seems to be working.

Here is an example showing it not working:
https://static.laszlokorte.de/svelte-debug/

use the select box to switch to component D and then back to some other (A, B or C) and look into the browser console to see how a null value is set.
The corresponding code can be found here:
https://github.com/laszlokorte/svelte5-debug-component-binding/blob/main/src/App.svelte#L39

This might be the place where the null value is set:
https://github.com/sveltejs/svelte/blob/main/packages/svelte/src/internal/client/dom/elements/bindings/this.js#L54

But I am not sure why it only happens in production builds.

I opened a separate issue for svelte 5:
#12287

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants