Removing render optimizations from core

[tbranyen]

I'm considering (and have already implemented) removing render skipping from
the core flow. Currently if diffHTML is rendering and another render is
scheduled it may be dropped. This has negative consequences for things like
Web Components which may have elements randomly schedule renders as children
are added to the DOM.

Consider the synchronous flow:

innerHTML(document.body, 'Hello world 1!');
innerHTML(document.body, 'Hello world 2!');
innerHTML(document.body, 'Hello world 3!');

This will render each operation synchronously and in full. Using devtools would
produce the following:

Render transaction #1: Started
Render transaction #1: Completed

Render transaction #2: Started
Render transaction #2: Completed

Render transaction #3: Started
Render transaction #3: Completed

However, if you were to add in a transition hook, the results could look
different:

addTransitionState('attached', domNode => new Promise(resolve => {
  // Resolve the transaction-blocking Promise after one second.
  setTimeout(resolve, 1000);
}));

innerHTML(document.body, 'Hello world 1!');
innerHTML(document.body, 'Hello world 2!');
innerHTML(document.body, 'Hello world 3!');

Using devtools with this flow would result in the following:

Render transaction #1: Started
Render transaction #1: Completed

Render transaction #2: Started
Render transaction #3: Started

Render transaction #2: Aborted
Render transaction #3: Completed

As we can see, the second transaction was "skipped over" in favor of the third
transaction that ran. This is designed to batch the rendering to improve
performance. I think it is very valuable as a default, since that is what many
developers roll with in their applications and benchmarks.

From an API design perspective, this makes the predictability of diffHTML hard
to master and some tasks impossible. For instance, I wanted to create an
animation consisting of an array of Virtual Trees that have
onattached/ondetached hooks to animate in and out, but found that every frame
in between the first and last was dropped. Code from that experiment is shown
below. I've since changed the behavior to work how I'd expect in the
fix-tests-and-devtools branch. The above would show each transaction started and
rendered serially even if they contain transitions that block rendering from
being synchronous.

Example code with this new change:

<stage></stage>

<script src="dist/diffhtml.js"></script>
<script src="../diffhtml-middleware-inline-transitions/dist/inline-transitions.js"></script>
<script>
  const { use, html, innerHTML } = diff;
  const stage = document.querySelector('stage');

  use(inlineTransitions());

  const animateIn = node => new Promise(resolve => {
    const animation = node.animate([
      { opacity: 0 },
      { opacity: 1 },
    ], { duration: 500 });

    animation.onfinish = resolve;
  });

  const panLeft = node => new Promise(resolve => {
    const animation = node.animate([
      { transform: 'translateX(-300px)' },
      { transform: 'translateX(0px)' },
    ], { duration: 500 });

    animation.onfinish = resolve;
  });

  const animateOut = node => new Promise(resolve => {
    const animation = node.animate([
      { opacity: 1 },
      { opacity: 0 },
    ], { duration: 500 });

    animation.onfinish = resolve;
  });

  const frames = [
    html`
      <div onattached=${animateIn} ondetached=${animateOut}>
        <h1>Hey, thanks for checking out diffHTML!</h1>
      </div>
    `,
    html`
      <div onattached=${panLeft}>
        <h1>Hey, thanks for checking out diffHTML!</h1>
      </div>
    `
  ].map((vTree, i) => (vTree.key = i, vTree));

  frames.forEach(frame => innerHTML(stage, frame)); 
</script>

[tbranyen]

I think we could implement a better batching algorithm with middleware.

[mAAdhaTTah]

Yeah, this is something I haven't spent a lot of time working on/thinking about yet. My implementation on top of diffhtml has the previous animations being cancelled when a new render is scheduled, but that's probably not going to work, as an attached animation would get cancelled on the way in, but provide no way to continue the animation on the next render. I haven't done a lot yet with animations to really battle test this way I need to, but I don't think there's any impact from my perspective to removing the batch handling by default. It will certainly simply some code, I think.

[tbranyen]

Gonna close this out. I much prefer this new set up, and I think it will make for fewer surprises once we launch.