svelte icon indicating copy to clipboard operation
svelte copied to clipboard

Transition within component prevents component unmount

Open jonom opened this issue 3 years ago • 4 comments

Describe the bug

It seems that a transition on an element within a component can prevent the component from destroying, if the element is transitioned in right before unmount.

I noted some existing issues with similar symptoms but the conditions seemed quite different to me so I opted to open a new issue - hope that's okay.

BTW going through the Svelte tutorials blew my mind just as much as React did when I first learned it. It will be hard to go back to a React project now. Thank you for all your hard work! ❤️

Reproduction

I've tried to demonstrate and describe the issue in detail here: https://svelte.dev/repl/36132eb6293843bf99a5427b14c26f82?version=3.43.1

Logs

No response

System Info

System:
    OS: macOS 11.6
    CPU: (12) x64 Intel(R) Core(TM) i7-8700B CPU @ 3.20GHz
    Memory: 888.46 MB / 16.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 10.23.2 - ~/.nvm/versions/node/v10.23.2/bin/node
    Yarn: 1.22.11 - /usr/local/bin/yarn
    npm: 6.14.10 - ~/.nvm/versions/node/v10.23.2/bin/npm
  Browsers:
    Chrome: 94.0.4606.71
    Firefox: 82.0.3
    Safari: 15.0
  npmPackages:
    rollup: ^2.3.4 => 2.58.0 
    svelte: ^3.0.0 => 3.43.1

Severity

annoyance

jonom avatar Oct 05 '21 05:10 jonom

@jonom - If you add the |local modifier it fixes/works around this problem.

Before:

<p transition:fade>I shouldn't be readable (for long)!</p>

After:

<p transition:fade|local>I shouldn't be readable (for long)!</p>

This is likely related to https://github.com/sveltejs/svelte/issues/6686 (which also affects transitions on SvelteKit pages). I agree with that issue that |local should be the default, and you opt into |global or something similar. I've sprinkled |local across most of my transitions for this very reason.

techniq avatar Oct 05 '21 11:10 techniq

@techniq thanks for the workaround!!

jonom avatar Oct 05 '21 16:10 jonom

I have a bunch of conditionally rendered components with animations in them and other child components. This resulted in double sections of UI rendering where there should just be one. It was an absolute pain to debug, and I only found this thanks to a colleague who remembered this issue.

In my case this was a UI breaking bug. If |local behavior is not made default, I would really like there to be warnings or errors when a component does not get destroyed due to a transition. Otherwise there is no straightforward way of fixing bugs like the one I had.

Twijgjes avatar Dec 21 '21 09:12 Twijgjes

@jonom - If you add the |local modifier it fixes/works around this problem.

Before:

<p transition:fade>I shouldn't be readable (for long)!</p>

After:

<p transition:fade|local>I shouldn't be readable (for long)!</p>

This is likely related to #6686 (which also affects transitions on SvelteKit pages). I agree with that issue that |local should be the default, and you opt into |global or something similar. I've sprinkled |local across most of my transitions for this very reason.

I have similar issue with the structure of my web app resembling this: ISSUE: repl

It was impossible to unmount the Quiz component because there was a fly transition used on its child component.

Thanks to you i used instead in:fly|local to sort this out: FIX: repl

induratized avatar Aug 04 '22 15:08 induratized

I have a similar problem, but I wasn't able to solve it myself by just adding the |local modifier to my transition. My code looks something like this:

<script>
	export let features: Array<Features>;
</script>

{#each features as feature}
	{#if $activeFeatureId == feature.id}
		<!-- Adding |local here didn't help -->
		<div transition:slide>
			<table class="w-full text-sm" cols={2}>
				<tbody>
					{#if feature.properties.type === "CAVITY"}
                        <!-- FeaturePropertyRow creates two <td> elements -->
						<FeaturePropertyRow name="Depth" value={feature.properties.height} unit="mm" />
						<FeaturePropertyRow
							name="Min Corner Radius"
							value={feature.properties.wallCornerRadiuses}
							unit="mm"
						/>
						<FeaturePropertyRow
							name="Min Bottom Radius"
							value={feature.properties.bottomCornerRadiuses}
							unit="mm"
						/>

                        ...many more features
					{:else if feature.properties.type === "PLANAR"}
						<FeaturePropertyRow name="Depth" value={feature.properties.height} unit="mm" />
					{/if}
				</tbody>
			</table>
		</div>
	{/if}
{/each}

The features property comes from a parent component and the $activeFeaturesId is a value from a store. Now i want to change features to another array that contains only features of type "PLANAR". When I do the change, the first element's ID gets automatically set as the $activeFeatureId.

The problem that I have, is that whenever I do this change, the feature properties of type "CAVITY" (which were shown before) stay visible, so they don't get unmounted. This means that in the end I have a table which contains a mixture of properties from "CAVITY" and "PLANAR".

See this video as an example. https://user-images.githubusercontent.com/17907799/217027508-fd4dab3d-e2d9-4fc4-a85a-a0e4c74acc32.mp4

When I remove transition:slide completely, this doesn't happen anymore, so it really seems to be some problem with the transition. Does anybody have an idea why |local doesn't help in my case and how I can get this to work without losing the transition?

Unfortunately, I wasn't able to create a small REPL to easily demonstrate the problem.

major-mayer avatar Feb 06 '23 16:02 major-mayer

This should be fixed in 3.57.0 - https://svelte.dev/repl/36132eb6293843bf99a5427b14c26f82?version=3.57.0

Conduitry avatar Mar 15 '23 17:03 Conduitry

Hmm, at least in my case the issue is still present, even though I'm not 100% sure if it's related to the bug that's reported here :/

major-mayer avatar May 15 '23 08:05 major-mayer

` |local modifier is now by default on all animations in [email protected]. check it out

induratized avatar Jun 15 '23 18:06 induratized