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

Empty components are not properly destroyed #7488

Open
Rich-Harris opened this issue Apr 26, 2022 · 1 comment
Open

Empty components are not properly destroyed #7488

Rich-Harris opened this issue Apr 26, 2022 · 1 comment
Labels

Comments

@Rich-Harris
Copy link
Member

Describe the bug

Via sveltejs/kit#2302: because DOM-less components do not have a fragment, they do not get properly destroyed when they're used with <svelte:component>:

<svelte:component this={active ? Empty : null}/>

The culprit is here:

export function transition_out(block: Fragment, local: 0 | 1, detach?: 0 | 1, callback?) {
if (block && block.o) {
if (outroing.has(block)) return;
outroing.add(block);
outros.c.push(() => {
outroing.delete(block);
if (callback) {
if (detach) block.d(1);
callback();
}
});
block.o(local);
}
}

I haven't yet investigated whether this would break other stuff, but adding the following fixes this specific bug:

export function transition_out(block: Fragment, local: 0 | 1, detach?: 0 | 1, callback?) {
	if (block && block.o) {
		if (outroing.has(block)) return;
		outroing.add(block);

		outros.c.push(() => {
			outroing.delete(block);
			if (callback) {
				if (detach) block.d(1);
				callback();
			}
		});

		block.o(local);
+	} else if (callback) {
+		callback();
	}
}

Reproduction

Can't show it in the REPL because it only appears in prod mode (DOM-less components still have a fragment in dev mode, because that's where we put certain warnings):

Logs

No response

System Info

System:
    OS: macOS 12.0.1
    CPU: (10) arm64 Apple M1 Max
    Memory: 148.25 MB / 32.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 16.13.1 - ~/.nvm/versions/node/v16.13.1/bin/node
    Yarn: 1.22.17 - ~/.nvm/versions/node/v16.13.1/bin/yarn
    npm: 8.1.2 - ~/.nvm/versions/node/v16.13.1/bin/npm
  Browsers:
    Chrome: 100.0.4896.127
    Firefox: 99.0.1
    Safari: 15.1
  npmPackages:
    rollup: ^2.3.4 => 2.70.2 
    svelte: ^3.0.0 => 3.47.0

Severity

annoyance

@magentaqin
Copy link
Contributor

@Rich-Harris After investigating it, I have found differences between dev and production environment.

In dev environment, the fragment property does have value, but in production environment, the fragment value is "false".

Screen Shot 2022-04-28 at 11 33 21 AM

Screen Shot 2022-04-28 at 11 34 21 AM

callback() on else branch is a workable alternative. Besides, fragment value on DOM-less components on both dev and production environment should behave the same. I will create a PR to fix this one.

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

No branches or pull requests

2 participants