Statics Diffing

[jridgewell]

Resolves #150.

---

So I think the way forward is to add an assert when reusing a node that has statics to check if either a key is defined or if statics have the same reference.

Looking at this last night, I actually think this is the worst approach. Consider:

var statics = ['id', 'id'];
function render(condition) {
  if (condition) {
    elementVoid('div');
  }
  elementVoid('div', null, statics);
}

On their own, both divs pass "statics have the same reference". But used together, they're a time bomb waiting for production.

This can be gotten around by requiring a key to use statics, but I don't think that's a scalable solution for templating libraries. And requiring a dev to provide a key just to write something as simple as <a href="/foo" /> is a nonstarter.

Instead, I think treating statics as special dynamics is the best approach. Since we're provided an array reference, just check strict equality of the array. If it matches, we're done. If not, perform the same attribute-change algorithm we use for dynamics.

[jridgewell]

Note that this'll create an unfortunate slow down for pre-rendered nodes that where previous sibling has a key.

<ul id="container">
  <li key="1" class="list">...</li>
  <li key="2" class="list">...</li>
  <li key="3" class="list">...</li>
  ...
</ul>

var listStatics = ['class', 'list'];
patch(container, (list) => {
  list.each((item, i) => {
    elementVoid('li', i, list);
  });
}, list);

That's because alignWithDOM will call getChild, which will initialize a keyMap without any of the known statics (listStatics). Once we actually get around to the second and third list items, they'll have a NodeData with statics = null, not listStatics.

With something like #127, this can be easily worked around.

[sparhami]

After some thinking about it, this is really not the right thing to do. If statics change, it is a pretty strong signal that something has appeared or disappeared.

Take the following example:

if (condition) {
   // subtree1
   elementOpen('div', null, staticsOne, ...);
     // lots of descendants
   elementClose('div');
}
// subtree2
elementOpen('div', null, staticsTwo, ...);
  // lots of descendants
elementClose('div');

If condition goes from 'false' to true, we definitely don't want to try to transform subtree2 into subtree1 because it might result in the whole thing being a diff and we'll need to recreate subtree2 afterwards anyway. This could also incorrectly move focus. Ideally we would have a key, but that might not be desirable in some cases (for example if this is a shared template function that might be called multiple times within the same parent).

Instead, the right thing is to check if statics has changed and if so, don't consider the current node as a match. In the above example, this would cause subtree1 to be freshly created and subtree2 to be left alone. Since we might not have statics yet (e.g. patching a server-side rendered tree), it is a little more complicated. What should be done is something like

function matches(node, nodeName, key, statics) {
  if (data.nodeName !== nodeName || key != data.key) {
    return false;
  }

  if (data.statics === statics) {
    return true;
  }

  var matches = // true if statics are the same

  if (matches) {
    if (data.statics) {
      // maybe add a warning (though we probably don't want to do this if we
      // created a statics array from pre-existing DOM)
    }

    data.statics = statics;
  }

  return matches;
}

We would also need have a way of importing what the statics are from a server-side render to make sure it is always correct if the first diff is done with different data. This could be done with a special attribute that lists out what the static attributes for the current node are.

We also need to import the attributes when encountering a DOM node that doesn't have a NodeData associated with it because attributes can disappear on the first diff. Importing attributes is really a separate change.

I'll work on this change when I get a chance.

[jridgewell]

Instead, the right thing is to check if statics has changed and if so, don't consider the current node as a match. In the above example, this would cause subtree1 to be freshly created and subtree2 to be left alone.

I'd be ok with that. But note that this will immensely impact anyone using inline statics, including any build step that's not advanced enough to hoist.

Since we might not have statics yet (e.g. patching a server-side rendered tree), it is a little more complicated.
We would also need have a way of importing what the statics are from a server-side render to make sure it is always correct if the first diff is done with different data.

If this is to highlight the case I brought up, I think not eagerly creating the NodeData would be a better (when creating a keyMap, if the child doesn't have a NodeData just try child.getAttribute('key')). That way, we can tell if this is a brand-new server-rendered node in matches.

[sparhami]

I'd be ok with that. But note that this will immensely impact anyone using inline statics, including any build step that's not advanced enough to hoist.

Yeah, I'm still re-thinking that part. I'll probably just check that the element has all the attributes with the same value as listed in statics. It isn't 100% correct, but hopefully shouldn't really be an issue. You could always use a marker attribute, something like data-species that is unique that would always prevent it from being a problem without needing a key.

If this is to highlight the case I brought up, I think not eagerly creating the NodeData would be a better (when creating a keyMap, if the child doesn't have a NodeData just try child.getAttribute('key')). That way, we can tell if this is a brand-new server-rendered node in matches.

The whole importing an existing DOM tree definitely needs a bit more thought. If there is a clean way, having a cache-free implementation might be nice too.