shikijs / shiki-magic-move

Smoothly animated code blocks with Shiki
https://shiki-magic-move.netlify.app/
MIT License
1.11k stars 32 forks source link

Animations not working as expected in Svelte #8

Closed mattcroat closed 2 months ago

mattcroat commented 2 months ago

I'm trying to await code block changes in Svelte.

console.log('start')
await renderer.render(result.current)
console.log('end')

Instead of resolving the promises as expected it instantly logs start and end.

https://github.com/shikijs/shiki-magic-move/assets/38083522/8c7dbdb4-718f-4250-9be0-176974e184dd

I tried Svelte's tick lifecycle function as the Vue example used it, because I thought it might be related to DOM updates but it made no difference.

await tick()
await renderer.render(result.current)
await tick()

After some debugging I noticed these styles in one of my reproductions made it work, so I decided to investigate further.

body {
  display: grid;
  place-content: center;
}

I'm not sure how grid fixes it but the actual reason seems to be that <pre> is block-level, and making it inline solves the problem — another solution I found was to set containerStyle to false.

pre {
  display: inline-block;
}

It works as expected.

https://github.com/shikijs/shiki-magic-move/assets/38083522/a5b63dd5-47ce-4f71-ad43-494cfc491971

I also found this code el.addEventListener('transitionend', finish) from Shiki Magic Move never ran because of this line — which I had to remove to make it work before.

const finish = (e) => {
  // if (!e || e.target !== el) return
  resolve()
}

You can look at the reproduction on StackBlitz where the entire code is located in src/app.svelte.

It might be a skill issue though.

paoloricciuti commented 2 months ago

I've identified the problem: when containerStyle is set to true and it's not the first render shiky-magic-move does this https://github.com/shikijs/shiki-magic-move/blob/b67b003b9f7010f84e519955b82a996944d4522d/src/renderer.ts#L359-L368 to apply the style change and wait for the transition to end before resolving the whole render promise.

However this poses a problem when the bg doesn't actually change because if you apply the same style the transition never start (and thus never end). This means that now we are awaiting for a promise that will never resolve (hence the error)

Here's an even more minimal reproduction of the situation in codepen.

https://codepen.io/paoloricciuti/pen/eYoKZKo

I wonder if this could happen for other stuff too and not only the bg.

A possible solution could be to set a timeout for options.duration * options.delayContainer and resolve the promise in this timeout and also set a transitionstart that cancel the timeout. Basically this:

  private registerTransitionEnd(el: HTMLElement, cb: () => void) {
    let resolved = false
    let resolve = () => { }
    const promise = new Promise<void>((_resolve) => {
      const finish = (e: TransitionEvent) => {
        if (!e || e.target !== el)
          return
        resolve()
      }
      resolve = () => {
        if (resolved)
          return
        resolved = true
        el.removeEventListener('transitionend', finish)
        cb()
        _resolve()
      }
      const timeout = setTimeout(()=>{
        resolve();
      }, this.options.duration * this.options.delayContainer);
      el.addEventListener('transitionstart', ()=> clearTimeout(timeout))      
      el.addEventListener('transitionend', finish)
    }) as PromiseWithResolve<void>
    promise.resolve = resolve
    return promise
  }

this basically should ensure that if the transition never starts the promise will be resolved nonetheless.

I can PR this change if needed.

antfu commented 2 months ago

I think it's likely the integration issue as the core works well with vanilla JavaScript. I am not familiar with Svelte enough to help sadly

paoloricciuti commented 2 months ago

I think it's likely the integration issue as the core works well with vanilla JavaScript. I am not familiar with Svelte enough to help sadly

Do you have a vanilla example? I investigated further and from what i'm seeing the problem is exactly what i've explained. But that should be a problem with vanilla too. It's possible that the integration with svelte surfaced this problem but i'm confident enough to say that the problem is there in vanilla too.

paoloricciuti commented 2 months ago

@antfu the moment you add containerStyle it breaks also for vue (i didn't test react but i would assume the same)

  1. go here https://stackblitz.com/edit/github-b1z8zy
  2. open the console
  3. change something in the playground
  4. observe that Animating continues to blink even after the animation is done and that in the console you can see the start log but not the end log
  5. from this moment on every change will show a start log followed immediately by an end log (this is the previous unfinished promises that are resolved when the new render starts)

i will try to PR this change...i have a decent idea of how to fix it but it might require some change when the promise is created. Will see

mattcroat commented 2 months ago

I think it's likely the integration issue as the core works well with vanilla JavaScript. I am not familiar with Svelte enough to help sadly

In the reproduction I provided I'm using shiki-magic-move/core and there's no Svelte magic besides running render() when code updates.

<script>
  // ...
  async function render() {
    const result = machine.commit(code)
    if (result.previous) renderer.replace(result.previous)
    console.log('start')
    await renderer.render(result.current)
    console.log('end')
  }

  $: if (code) {
    render()
  }
</script>

Thank you for making this great project because this is something I dreamed about integrating into Animotion which is only possible because of your work on Shiki. 🙏