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

Transition within component prevents component unmount #6812

Closed
jonom opened this issue Oct 5, 2021 · 8 comments
Closed

Transition within component prevents component unmount #6812

jonom opened this issue Oct 5, 2021 · 8 comments

Comments

@jonom
Copy link

jonom commented Oct 5, 2021

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

@techniq
Copy link

techniq commented Oct 5, 2021

@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.

@jonom
Copy link
Author

jonom commented Oct 5, 2021

@techniq thanks for the workaround!!

@Twijgjes
Copy link

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.

@induratized
Copy link

induratized commented Aug 4, 2022

@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

@major-mayer
Copy link

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.

@Conduitry
Copy link
Member

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

@major-mayer
Copy link

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 :/

@induratized
Copy link

induratized commented Jun 15, 2023

` |local modifier is now by default on all animations in svelte@4.0.0.next. check it out

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

No branches or pull requests

7 participants