google / incremental-dom

An in-place DOM diffing library
http://google.github.io/incremental-dom/
Apache License 2.0
3.54k stars 180 forks source link

Monotonic Increasing TypeId #307

Closed jridgewell closed 6 years ago

jridgewell commented 8 years ago

I love the typeId idea, but we still have a problem in one direction:

While going from true to false currently recreates the large subtree, Incremental DOM may support handling a single deletion.

if (condition) {
  open('div', null, 1);
    text('Hello');
  close('div');
}
open('div', null, 2);
  // Some large subtree
close('div');

What if we enforce that the typeId is a monotonic increasing int? That way we have 3 scenarios:

Scenario currentNode's typeId typeId Behavior
1 5 5 Match!
2 5 4 (anything less than 5) No match, will not find match in nextSibling direction
3 5 6 (anything more than 5) No match, may find match in nextSibling direction

The current typeId handles scenario 1 perfectly, and bumbles 2 (it treats any typeId mismatch as a "insert this element before the current"). But it utterly fails 3. But with monotonic increasing, we have the information to decide what to do:

Scenario Behavior
1 Nothing!
2 Insert element before current element, since its typeId is less
3 Remove current element, progress to next and recheck

Additionally, these should not be some runtime int that increases with every call to open. It has to be statically assigned for us to reap the rewards (though the runtime increasing int will just get us back to the current behavior).

// Good example
// Will never recreate large subtree
function render() {
  if (condition) {
    open('div', null, 1); // Regardless of runtime state, this element is statically "1"
      text('Hello');
    close('div');
  }
  open('div', null, 2); // Ditto
    // Some large subtree
  close('div');  
}

// Bad example (Gives us current behavior)
// Will end up recreating large subtree every transition
function render() {
  var i = 0;
  if (condition) {
    open('div', null, i++); // Runtime state determines if this is "1"
      text('Hello');
    close('div');
  }
  open('div', null, i++); // Runtime state determines if this is "1" or "2".
    // Some large subtree
  close('div');  
}

Note that the typeId int can be repeated, so this is totally fine:

function li() {
  open('li', null, 1);
  close('li');
}

function ul(lis) {
  open('ul', null, 1);
    for (var i = 0; i < lis.length; i++) li();
  close('ul');
}

So, in dev mode:

sparhami commented 7 years ago

It would be nice to be able to also gracefully handle helper functions. For example, you could have:

function render() {
  elementOpen('div');
    if (...) { helperOne(); }
    if (...) { helperTwo(); }
  elementClose('div');

  elementOpen('div');
    if (...) { helperTwo(); }
    if (...) { helperOne(); }
  elementClose('div');
}

So you wouldn't necessarily be able to have a monotonic increasing typeId for both helper functions. This could be solved by setting or passing some sort of "base" typeId, which the helper could add to, but would make generating the code a bit more difficult.

I think the idea good as-is though, just might want to have some mechanism or guidance on how to handle helper functions.

jridgewell commented 7 years ago

Hmm, good thought.

How about assuming, but not enforcing, monotonic? We warn (in dev mode) when it's violated, and reset the "expected" to the lower value?

sparhami commented 6 years ago

Closing since we no longer have typeId and keys no longer need to be unique. We could revisit this for keys, but I think it wouldbe better to handle always reusing, regardless of if the keys are increasing or not.