tbranyen / diffhtml

diffHTML is a web framework that helps you build applications and other interactive content
https://diffhtml.org
MIT License
868 stars 47 forks source link

innerHTML fails to diff element when its children have been diffed previously #314

Open h5gq3 opened 1 year ago

h5gq3 commented 1 year ago

illustrated by this test:

it("will diff an element when element's children have been diffed before", function (cb) {
  var p = document.createElement("p");
  diff.innerHTML(p, "<span>Test</span>");
  // this.fixture is <div></div>
  diff.innerHTML(this.fixture, p); // <div><p><span>Test</span></p></div>

  // diff element p child span
  diff.innerHTML(this.fixture.querySelector("span"), "Test 2"); // this works: <div><p><span>Test 2</span></p></div>

  // our test case: diff element p when child span has been diffed previously
  diff.innerHTML(this.fixture.querySelector("p"), "<span>Test 3</span>"); // this doesn't work - still <div><p><span>Test 2</span></p></div>

  setTimeout(() => {
    assert.equal(this.fixture.querySelector("span").innerHTML, "Test 3"); // fails because it's still Test 2
    cb();
  });
});
CetinSert commented 1 year ago

Is this a regression in v1.0.0-beta.30 released on 2023-07-12 (3 days ago)?

h5gq3 commented 1 year ago

no, it's been this way since i encountered this few months ago

tbranyen commented 1 year ago

I dropped this test into the dom node section of the unit tests and was able to reproduce the failure:

image

Definitely looks like a bug, thanks for reporting!

tbranyen commented 1 year ago

This is the offending line: https://github.com/tbranyen/diffhtml/blob/master/packages/diffhtml/lib/transaction.js#L135

Looks like we're reusing the state from a previous render, instead of creating a new transaction state for the updates. When I remove: StateCache.get(mount) || the test passes:

image

tbranyen commented 1 year ago

I think I have a fix. When we encounter a previous mount being added into a new mount, we remove the previous StateCache. This occurred when you put p into fixture.

PR here: https://github.com/tbranyen/diffhtml/pull/315