transition property on any element breaks navigation scroll preservation

Describe the bug

When navigating from a page that has a transition defined on any component, scroll preservation of a previously visited site is broken, and the scroll is "all over the place" (depending on browser and kind of navigation [e.g. to anchor tag]). This only occurs if the element with the transition property is unmounted during the navigation. If the transition property was defined on e.g. a navbar that is in a parent layout between the navigated pages, the bug will not occur.

This came up as an issue here: https://github.com/themesberg/flowbite-svelte/issues/950

Reproduction

https://stackblitz.com/edit/sveltejs-kit-template-default-atnnds

Please scroll down until you find the test buttons and wait for hydration on first load (scroll position will not show as undefined) :)

Simple example

Any element with a transition:

<div transition:fade></div>

Even if the transition is empty (null):

<script>
  const null_trans = () => null
</script>
<div transition:null_trans></div>

(Examples taken from @jjagielka)

Logs

n.a.

System Info

This is the StackBlitz instance:

System:
    OS: Linux 5.0 undefined
    CPU: (8) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 0 Bytes / 0 Bytes
    Shell: 1.0 - /bin/jsh
  Binaries:
    Node: 16.20.0 - /usr/local/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 9.4.2 - /usr/local/bin/npm
    pnpm: 8.6.3 - /usr/local/bin/pnpm
  npmPackages:
    svelte: ^4.0.4 => 4.0.4

Severity

annoyance

After some triage, I think I found a race condition between any out transition (also bidirectional-transition) and the kit navigation code.

The out transition will run before navigation. It then enters loop(fn) which dispatches fn to requestAnimtionFrame. This causes other JS to run, namely the navigation code in kit: src/runtime/client.js:navigate which will now call scrollTo(scroll.x, scroll.y). This is called while the element that is transitioned out is still present in the DOM and the new content has been added to the DOM. The DOM is therefore now "longer" and calling scrollTo will result in visible Y scroll after the transitioned element is removed from the DOM.

If we were to wrap kit's scrollTo(scroll.x, scroll.y) and deep_linked.scrollIntoView() in requestAnimtionFrame this would be at least fixed for null_transition's.

For a visualization, see video below:

https://github.com/sveltejs/svelte/assets/33808743/0f992a62-c80b-421a-8110-04635f0c4788

Update:

This is my first time looking at Svelte's source code, so I am not very familiar with its internal workings. But does it even make sense to out run transitions and therefore keep elements in the DOM when navigating away? If the transition is defined on a scope that stays in the tree like this:

(show simply changes from true → false)

{#if show}
  <p out:fade>Hello there :P</p>
{/if}

it makes sense to me to run the transition. But if we change the whole page, I wouldn't expect that transition to be run.

After understanding the forces at play a little better and looking at the documentation again, this seems to be one of those "it's a feature, not a bug" cases:

The relevant documentation tutorial states: "Ordinarily, transitions will only play on elements when their direct containing block is added or destroyed". While this is technically true for page navigation, there should probably be an option to explicitly disable it on navigation (or make that the default and let users enable it) to avoid this issue. I as a user would not expect a simple slide animation like in the example or even a null_animation to "mess up" my navigation ^^

I just experimented with a |nonav modifier (file) for transitions that works just like |local but wraps the transition block in a check for !globalThis.__sveltekit_is_navigating (file). I then simply set globalThis.__sveltekit_is_navigating everywhere where we set navigating in kit's runtime client (file). This fixes my issue like a charm!

Is there any cleaner way to communicate between Svelte and SvelteKit?

sveltejs / svelte