Updates to keyed lists break FLIP animations when they occur mid-animation

dead-claudia commented 4 years ago

Mithril version: 2.0.4

Browser and OS: Chrome 83.0.4103.116 on Windows 64-bit, Safari 13.1 on iOS, Firefox 78.0.2 on Windows 64-bit

Project:

Code

<button id="toggle">Toggle</button>
<div style="display:flex">
  <ul id="prev"><li>0</li><li>1</li><li>2</li><li>3</li><li>4</li><li>5</li><li>6</li><li>7</li></ul>
  <ul id="animate"></ul>
  <ul id="next"><li>0</li><li>1</li><li>2</li><li>3</li><li>4</li><li>5</li><li>6</li><li>7</li></ul>
</div>

const animateRoot = document.getElementById("animate")
let values = Array.from({length: 8}, (_, i) => i)

function render() {
  m.render(animateRoot, values.map(i => m("li", {key: i}, i)))
}

let interval = setTimeout(updateBody, 500)

function updateBody() {
  interval = setTimeout(updateBody, 500)
  const prev = values
  // https://en.wikipedia.org/wiki/Fisher%E2%80%93Yates_shuffle
  const next = values = prev.slice()
  for (let i = next.length - 1; i > 0; i--) {
    const j = Math.floor(Math.random() * (i + 1))
    const temp = next[j]
    next[j] = next[i]
    next[i] = temp
  }

  requestAnimationFrame(() => {
    const prevList = document.getElementById("prev").childNodes
    const nextList = document.getElementById("next").childNodes

    // Reorder elements in DOM
    for (let i = 0; i < prev.length; i++) {
      prevList[i].textContent = prev[i]
    }
    for (let i = 0; i < next.length; i++) {
      nextList[i].textContent = next[i]
    }

    const prevElems = Array.from(animateRoot.childNodes)

    // First
    const firstParent = animateRoot.getBoundingClientRect()
    const first = prevElems.map(e => {
      const child = e.getBoundingClientRect()
      return {
        top: child.top - firstParent.top,
        left: child.left - firstParent.left,
      }
    })

    render()

    // Last + Invert
    const lastParent = animateRoot.getBoundingClientRect()
    const movedElems = prevElems.filter((e, i) => {
      const last = e.getBoundingClientRect()
      const dx = first[i].left - (last.left - lastParent.left)
      const dy = first[i].top - (last.top - lastParent.top)
      if (dx === 0 && dy === 0) return false
      e.style.transform = `translate(${dx}px,${dy}px)`
      e.style.transitionDuration = "0s"
      return true
    })

    // Force re-layout
    document.body.offsetHeight

    // Play
    movedElems.forEach((e, i) => {
      const callback = () => {
        e.removeEventListener("transitionend", callback, false)
        e.classList.remove("transition")
      }
      e.classList.add("transition")
      e.addEventListener("transitionend", callback, false)
      e.style.transform = e.style.transitionDuration = ''
    })
  })
}

document.getElementById('toggle').onclick = () => {
  if (interval) {
    clearTimeout(interval)
    interval = null
  } else {
    updateBody()
  }
  p('toggle', interval != null)
}

render()

Steps to Reproduce

Open web page

You can enable and disable the animations at will using the "Toggle" button, but that's not part of the repro.

Expected Behavior

The animation to be smooth.

Current Behavior

The animation appears choppy whenever the subtree updates, and list items often unexpectedly jump.

Context

The following vanilla code works correctly. It's a near-exact clone of the above, but it simply uses appendChild instead of a full keyed diff.

Live Flems

<button id="toggle">Toggle</button>
<div style="display:flex">
  <ul id="prev"><li>0</li><li>1</li><li>2</li><li>3</li><li>4</li><li>5</li><li>6</li><li>7</li></ul>
  <ul id="animate"><li>0</li><li>1</li><li>2</li><li>3</li><li>4</li><li>5</li><li>6</li><li>7</li></ul>
  <ul id="next"><li>0</li><li>1</li><li>2</li><li>3</li><li>4</li><li>5</li><li>6</li><li>7</li></ul>
</div>

const animateRoot = document.getElementById("animate")
let cachedElems = Array.from(animateRoot.childNodes)
let interval = setTimeout(updateBody, 500)

function render() {
  for (const elem of cachedElems) animateRoot.appendChild(elem)
}

function updateBody() {
  interval = setTimeout(updateBody, 500)

  const prevElems = cachedElems
  const nextElems = cachedElems = prevElems.slice()
  // https://en.wikipedia.org/wiki/Fisher%E2%80%93Yates_shuffle
  for (let i = nextElems.length - 1; i > 0; i--) {
    const j = Math.floor(Math.random() * (i + 1))
    const temp = nextElems[j]
    nextElems[j] = nextElems[i]
    nextElems[i] = temp
  }

  requestAnimationFrame(() => {
    const prevList = document.getElementById("prev").childNodes
    const nextList = document.getElementById("next").childNodes

    // Reorder elements in DOM
    for (let i = 0; i < prevElems.length; i++) {
      prevList[i].textContent = prevElems[i].textContent
    }

    for (let i = 0; i < nextElems.length; i++) {
      nextList[i].textContent = nextElems[i].textContent
    }

    // First
    const firstParent = animateRoot.getBoundingClientRect()
    const first = prevElems.map(e => {
      const child = e.getBoundingClientRect()
      return {
        top: child.top - firstParent.top,
        left: child.left - firstParent.left,
      }
    })

    render()

    // Last + Invert
    const lastParent = animateRoot.getBoundingClientRect()
    const movedElems = prevElems.filter((e, i) => {
      const last = e.getBoundingClientRect()
      const dx = first[i].left - (last.left - lastParent.left)
      const dy = first[i].top - (last.top - lastParent.top)
      if (dx === 0 && dy === 0) return false
      e.style.transform = `translate(${dx}px,${dy}px)`
      e.style.transitionDuration = "0s"
      return true
    })

    // Force re-layout
    document.body.offsetHeight

    // Play
    movedElems.forEach((e, i) => {
      const callback = () => {
        e.removeEventListener("transitionend", callback, false)
        e.classList.remove("transition")
      }
      e.classList.add("transition")
      e.addEventListener("transitionend", callback, false)
      e.style.transform = e.style.transitionDuration = ''
    })
  })
}

// Utility bits
document.getElementById('toggle').onclick = () => {
  if (interval) {
    clearTimeout(interval)
    interval = null
  } else {
    updateBody()
  }
  p('toggle', interval != null)
}

render()

Edit:

Mithril's far from the only one with this issue: https://twitter.com/isiahmeadows1/status/1284726730574315522

React: https://github.com/facebook/react/issues/19406
Preact: https://github.com/preactjs/preact/issues/2637
Inferno: https://github.com/infernojs/inferno/issues/1519
And likely others

Note: Vue is not affected by this bug and carries the same behavior as the vanilla version - go here and spam the "Shuffle" button to see.

Edit 2: Also relevant: https://github.com/whatwg/html/issues/5742

dead-claudia commented 4 years ago

Note to whoever aims to fix this: it requires a lot of experimentation and will require significant knowledge of the internals. It'll be easiest if you set the bundler to watch mode and create a throwaway HTML file with the above body and keep the devtools open with caching disabled, so you can refresh the page frequently. And no, you won't be able to automatically test this until you can determine what causes the jumpiness.

yossicadaner commented 4 years ago

@isiahmeadows I am not sure if this helps, but I think it's related to something we discussed in the past https://github.com/MithrilJS/mithril.js/issues/2135

dead-claudia commented 4 years ago

@yossicadaner That's a separate issue, and this one's really rooted in a spec oddity (bug?).

MithrilJS / mithril.js