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

fix: bind:this on changing transitioning element #8930

Open
wants to merge 7 commits into
base: svelte-4
Choose a base branch
from

Conversation

robertadamsonsmith
Copy link

Fixes #8891

When an outro starts, a bind:this variable is now nulled immediately, instead of when the element is destroyed. This is a breaking change.

This makes the behaviour consistent with, and appropriate for, the case where the bind:this value is swapping from one transitioning element to another (such as in an if/else block), and avoids the bug in that situation where the outroing component nulls the bind:this variable which has since been bound to another element. This bug specifically breaks snapshotting in SvelteKit when using transitions on pages.

The new binding-this-transition-js-if-else-block test highlights the bug most simply (in practice the bug is more subtle - since the unwanted nulling doesn't cause the element to be invalidated, it isn't usually until some other change occurs that the change in the value is reactively apparent).

Things to do still (if appropriate):

  • Three existing transition tests, that relied on bind:this not being nulled for an outroing component, have been modified so that they succeed. They are less thorough as a result though, and so further changes may be appropriate to those tests.
  • It may also be suitable to add a test that proves that unmounting an element with a local transition, but such that the transition doesn't fire, also causes bind:this to be nulled appropriately (I am not sure if any existing tests would prove that.)
  • Also, in bind_this.js, the branch for when there are contextual_dependencies, would likely need similar changes if this approach is appropriate.

Because this pull request affects code that is possibly undergoing considerable change for Svelte 5, as well as it being a breaking change, it may be appropriate for this pull request to inform that process rather than be used directly.

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

Successfully merging this pull request may close these issues.

Transitions cause bind:this bindings to be incorrect
1 participant