sveltejs / kit

web development, streamlined
https://kit.svelte.dev
MIT License
18.41k stars 1.88k forks source link

Transition cleanup happens too early if Kit is already running #2044

Open iltimasd opened 3 years ago

iltimasd commented 3 years ago

Describe the bug

On Chrome, if Svelte is already loaded on the page and refreshed (and sometimes routed to), any in: transition triggered by onMount will remove the animation before fully transitioned, causing the transition to jump to its final state.

https://user-images.githubusercontent.com/20071081/127706043-3e111ff8-4d19-4aa1-836a-7e5e115dd575.mov

Reproduction

https://jumpy-intros.vercel.app/some

https://github.com/iltimasd/jumpy-intros

Logs

No response

System Info

System:
    OS: macOS Mojave 10.14.6
    CPU: (8) x64 Intel(R) Core(TM) i7-8557U CPU @ 1.70GHz
    Memory: 52.05 MB / 16.00 GB
    Shell: 3.2.57 - /bin/bash
  Binaries:
    Node: 14.15.1 - /usr/local/bin/node
    Yarn: 1.22.4 - /usr/local/bin/yarn
    npm: 6.14.8 - /usr/local/bin/npm
  Browsers:
    Chrome: 92.0.4515.107
    Chrome Canary: 94.0.4590.2
    Edge: 92.0.902.62
    Firefox: 90.0
    Safari: 13.0.4
  npmPackages:
    @sveltejs/adapter-vercel: next => 1.0.0-next.26 
    @sveltejs/kit: next => 1.0.0-next.139 
    svelte: ^3.34.0 => 3.41.0

Severity

annoyance

Additional Information

I have yet to find a workaround.

iltimasd commented 3 years ago

I think I narrowed this down, but I REALLY need y'all to make this make sense to me. This jumpiness on refresh only seems to happen when I change the body background from linear-gradient app.css from the npm init starter template to ANYTHING else (or dont import the app.css in __layout).

here are 3 Vercel links generated from main branch and 2 others (each with different variations of the problem). This is a different repo than the OP because i was getting a weird fatal error I could not get rid of, so i just generated a new one from scratch.

1) Main branch | no problem observed yet This is the base code of all 3. It strips almost everything from the starter template. In terms of the files of interest; the app.css only has the background style for body and the layout.svelte file imports it . Again we're looking for smoothe transitions on browser refresh (which we get in this instance)

https://wait-wut-iltimasd.vercel.app/

app.css

body {
  background: white;
}

/* body {
  background: grey;
} */

layout

<script>
  //   import "../app.css";
</script>

<slot />

<style>
</style>

2) no-import branch | jumpy transitions on refresh

in this the app.css is the same from the main branch and in __layout.svelte the imported css is commented out

https://wait-wut-git-no-import-iltimasd.vercel.app/

  1. different-bg branch | jumpy transitions on refresh

in this the __layout.svelte is the same from the main branch and in app.css we change the background value to "white".

https://wait-wut-git-different-bg-iltimasd.vercel.app/


I would really appreciate if anyone could check these links on their browsers, refresh and see if you can repro, as I'd like to rule-out my browser first

The repo is below and I've opened PRs for the branches so you can see the diffs.

https://github.com/iltimasd/wait-wut

benmccann commented 3 years ago

Possibly related to https://github.com/sveltejs/kit/issues/628?

iltimasd commented 3 years ago

Perhaps, but I think that issue, if I'm understanding the thread correctly, is more specific to the out: directive being fired on nav.

This instance I only use the in: directive.

That said I'm not familiar with Svelte internals at all, so maybe it is something specific to how Svelte is slotting in the components on nav?

iltimasd commented 3 years ago

Went to go add |local like suggestions in #628, and realized I already did 😭

https://github.com/iltimasd/wait-wut/blob/no-import/src/routes/index.svelte

iltimasd commented 3 years ago

I'm hitting an interesting wall trying to debug what's going. I've decided to try adding some breakpoint in the transition cleanup portion of the bundled svelte. However if I add any breakpoints, the error goes away; presumably because the breakpoints are adding some sort of delay that allow the cleanup to happen on time. (edit: or! perhaps its the start time that's actually wrong??)

Edit:

And I should add commenting cleanup() out solves the visual bug

ethandpowers commented 2 years ago

I'm having this issue as well. I was able to get around it by wrapping the trigger in a setTimeout() call like such:

let mounted = false;

onMount(() => {
    setTimeout(() => {
        mounted = true;
    }, 500);
});

{#if mounted}
    <h1 transition:fly|local={{ x: 200, duration: 1500 }}>Hello World!</h1>
{/if}

The timeout is not super noticeable, but it's certainly not ideal. I've played around with a few different durations and found that 500 works best for me.

elron commented 1 year ago

I'm having this issue as well. I was able to get around it by wrapping the trigger in a setTimeout() call like such:

let mounted = false;

onMount(() => {
    setTimeout(() => {
        mounted = true;
    }, 500);
});

{#if mounted}
    <h1 transition:fly|local={{ x: 200, duration: 1500 }}>Hello World!</h1>
{/if}

The timeout is not super noticeable, but it's certainly not ideal. I've played around with a few different durations and found that 500 works best for me.

I suggest you try requestAnimationFrame instead of setTimeOut. Reason best explained in this talk

raythurnvoid commented 1 year ago

i've changed the parameters of the animations to duration: 100 and delay: 0.

in:fade|local={{ duration: 100, delay: 0 }}

looks like svelte performs the cleanup of the keyframe before the animation ends (sometime even before it starts)

image

This behavior is caused by the browser starting the animation with a significant delay for some unknown reason.

Though the issue doesn't have anything to do with sveltekit but is actually related to svelte