onDestroy runs after the component is detached from DOM when it is wrapped in a DOM element

[giacomoran]

Describe the bug

Hi!

If you have something like:

{#if show}
  <div>
    <CustomComponent />
  </div>
{/if}

and show becomes false, the onDestroy method of CustomComponent will fire after it has disappeared from the DOM.
It works as expected if we remove the wrapping div.

The reason is that the if block calls node.parentNode.removeChild(node) on the div element before destroying the custom component. When the div element is removed from the DOM also the custom component is removed.

Our use case

We listen for blur events on a component within a list, we remove the listeners in onDestroy.
If the component is removed from the list while holding the focus, the blur event will be fired. The event is captured because onDestroy has not been called yet, we instead expected the blur event to be ignored.

Reproduction

Full repro (see App.svelte, lib/CustomComponent.svelte and the instructions below): https://stackblitz.com/edit/vitejs-vite-aaxjzf?file=src/App.svelte

Instructions:

• add a breakpoint inside Svelte's detach function (we search for node.parentNode.removeChild(node) in the "Source" tab of Chrome DevTools to find it)
• click the "Hide" button

Logs

No response

System Info

System:
    OS: Linux 5.4 Ubuntu 20.04.5 LTS (Focal Fossa)
    CPU: (8) x64 Intel(R) Core(TM) i7-8550U CPU @ 1.80GHz
    Memory: 18.60 GB / 31.24 GB
    Container: Yes
    Shell: 5.8 - /usr/bin/zsh
  Binaries:
    Node: 16.17.0 - ~/.asdf/installs/nodejs/16.17.0/bin/node
    npm: 8.15.0 - ~/.asdf/installs/nodejs/16.17.0/bin/npm
  Browsers:
    Brave Browser: 86.1.16.68
    Chrome: 105.0.5195.125
    Chromium: 105.0.5195.125
    Firefox: 104.0
  npmPackages:
    svelte: ^3.50.1 => 3.50.1

Severity

annoyance

[RaiVaibhav]

Findings - the destroy check whether it has some parent div and since block chunks read from top to bottom, it says to detach the div first then destroy the child component.

I would like to work on this issue

[RaiVaibhav]

Hey @tanhauhau can you guide me if this solution can cause any edge case?, in this case, I am checking if there is a parent node instead of push append to the first, and for nested components then the last child will be at the first index.

	if (parent_node) {
		block.chunks.destroy.unshift(b`
			@destroy_component(${name}, detaching);
		`);
	} else {
		block.chunks.destroy.push(b`
			@destroy_component(${name}, ${parent_node ? null : 'detaching'});
		`);
	}

[baseballyama]

I think current behaivior is correct.
I created REPL with valinna JS but blur event occured.
https://codepen.io/baseballyama/pen/VwdVXMY

We should have same behaivior between vanilla JS and Svelte.

-> This is not related to the issue.

[lucidNTR]

critical issue to make moveBefore() dom api work. onDestroy needs to run before any DOM nodes of the current component are touched or removed. This is still happening in svelte 5

[lucidNTR]

if anyone sees this there is a workaround with out:handler|global to have reliable pre dom removal handlers.