superfine: Unexpected reordering after keyed removal

it appears the infamous Rasmus’ Bug has come to an end

it’s the end of an Era 😃

This is great @jorgebucaran! Are you going to incorporate this version of superfine into Hyperapp 1.x?

The example you made is very useful to diagnose and play with this bug. It’s actually fun. It’s also frustrating haha.

I attempted a fix but I’m out of time, and I had to give up.

I can provide some clues as to what won’t work.

My idea was to flag the old node for preservation inside removeElement:

function removeElement(parent, child, element) {
  var props = child.props

  if (
    props &&
    props.onremove &&
    typeof (props = props.onremove(element)) === "function"
  ) {
    child.preserve = true
    props(function () {
      child.preserve = false
      remove()
    })
  } else {
    remove()
  }

  function remove() {
    parent.removeChild(element)
  }
}

Note that I change the function signature from (parent, element, childProps) to (parent, child, element) so as to get access to the VNode.

Inside the main loop I figured I’d then splice the preserved node back into the list of new nodes:

    while (j < len) {
      var oldElement = oldElements[i]
      var oldChild = oldNode.children[i]
      var newChild = node.children[j]

      if (oldChild && oldChild.preserve) {
        node.children.splice(j, 0, oldChild)
        i++
        len++
        continue
      }

From doing a lot of console.log() statements to try to understand the order of events, I can say though, this approach is not going to work, because, by the time we flag the node for preservation, it has in fact already been removed.

I think what needs to happen is we must somehow preserve the node as soon as we encounter it - like that means removeNode needs to return a flag indicating whether or not the node in question may actually be removed.

function removeElement(parent, child, element) {
  var props = child.props

  if (
    props &&
    props.onremove &&
    !child.destroying &&
    typeof (props = props.onremove(element)) === "function"
  ) {
    child.destroying = true // flag to avoid triggerin onremove handler again
    props(function () {
      child.destroy = true // flag for actual destruction in main loop
      remove()
    })
    return false // not removed yet
  } else {
    remove()
    return true // directly removed
  }

  function remove() {
    parent.removeChild(element)
  }
}

Unfortunately there are three different calls to removeElement which may all need to act differently depending on whether or not the node was actually removed.

Note that we can’t simply pass the list of new nodes to this function, because the len in the main loop will also change if we carry over an old removed node.

That’s all I got, and I won’t get to this again until next week most likely, so…

@JorgeBucaran what leeoniya said.

You are aware, of course, that when we see that a keyed node is removed from the vdom, and we go to see if there’s an onremove handler to call.

Then if there is we don’t immediately remove the element. So the element is still in the dom for a while - but where in the dom? At the bottom of the sibling list.

The effect of that is, if you naively use a css-transition to fade out the element, the element will first skip to the bottom of the list, and from there start fading out.

What @mindplay-dk is asking for, is that the element fades out from it’s original position.

@mindplay-dk I am not looking for excuses to forego this feature, but I first need to understand what we want. 😄

@zaceno https://github.com/leeoniya/domvm/issues/159 & https://jsfiddle.net/cup40vu7/