crash in domdiff

[atirip]

This crashes after second or third rerender, depending of random(). Also here: https://codepen.io/atirip/pen/oNgxXmR?editors=0011

var props = {}

var a = hyperHTML.wire()`<p>A</p><p>A2</p>`;
var b = hyperHTML.wire()`<p>B</p><p>B2</p>`;
var c = hyperHTML.wire()`<p>C</p><p>C2</p>`;
var d = hyperHTML.wire()`<p>D</p><p>D2</p>`;

function render() {
    console.log('rendering');
    var x;

    if (Math.random() < 0.5) {
        x = a
        a = c
        c = x
    }

    var r = hyperHTML.wire(props, ':page')`
        <div>
            ${a}
            ${b}
        </div>
    `;
    var r2 = hyperHTML.wire(props, ':page2')`
        <div>
            ${c}
            ${d}
        </div>
    `;
    return [r, r2];
}

hyperHTML.bind(document.body)`<section>${render()}</section>`;
setInterval(render, 1000);  

The culprit as far as I have debugged is that when domdiff gets Wire as futureNodes, its also returns the same futureNodes and these are stored into childNodes. Later, on second render, that crashes as childNodes are not live DOM nodes.

        } else if (canDiff(value)) {
          childNodes = domdiff(node.parentNode, childNodes, value.nodeType === DOCUMENT_FRAGMENT_NODE ? slice.call(value.childNodes) : [value], diffOptions);

[atirip]

So far, inserting this:

let l = futureNodes.length;
while (l--) if (futureNodes[l] instanceof Wire) futureNodes.splice(l, 1, ...futureNodes[l].childNodes);

in the beginning of the domdiff() seems to fix it for me.

[WebReflection]

domdiff knows nothing about wires, but I am struggling to understand what are you trying to do here.

hyperHTML owns its content, so that you should never mess up with it manually.

However, if you try this instead, I see no errors:

const {bind, wire} = hyperHTML;
var props = {}

var a = wire()`<p>A</p><p>A2</p>`;
var b = wire()`<p>B</p><p>B2</p>`;
var c = wire()`<p>C</p><p>C2</p>`;
var d = wire()`<p>D</p><p>D2</p>`;

function render() {
  console.log('rendering');
  var x;

  if (Math.random() < 0.5) {
    x = a
    a = c
    c = x
  }

  var r = wire()`
    <div>
      ${[a, b]}
    </div>
  `;
  var r2 = wire()`
    <div>
      ${[c, d]}
    </div>
  `;
  return [r, r2];
}

setInterval(
  bound => bound`<section>${render()}</section>`,
  1000,
  bind(document.body)
);

The explanation is quite simple: if you hard wire nodes and you manually move hard wired nodes around, the engine wouldn't know where these nodes are: 1 wire, 1 or more nodes. Move manually the node somewhere else, the wire doesn't know anymore what to handle so that dropping the hard wired content fixes this.

Since in 2+ years I've never had to do anything like you did here, and this library has been in production for more than 1 year and a half already, and since you are manually interfering with hard wired nodes, I suggest you take a look at lighterhtml instead, because it deals with non-keyed results in a way that hyperHTML might never do.

Your example would look like the following one:

const {render, html} = lighterhtml;

let a = html`<p>A</p><p>A2</p>`;
let b = html`<p>B</p><p>B2</p>`;
let c = html`<p>C</p><p>C2</p>`;
let d = html`<p>D</p><p>D2</p>`;

function update() {
  console.log('rendering');
  let x;

  if (Math.random() < 0.5) {
    x = a
    a = c
    c = x
  }

  const r = html`
    <div>
      ${a}
      ${b}
    </div>
  `;
  const r2 = html`
    <div>
      ${c}
      ${d}
    </div>
  `;
  return [r, r2];
}

setInterval(
  () => render(document.body, html`<section>${update()}</section>`),
  1000
);

and you can test it live on CodePen.

[atirip]

This is just and example that exposes specific case where domdiff crashes. It just bite me lately, while I have been using hyperHTML for a long time. Just I have DOM that occasionally changes a lot. Also I need to use props and wireId's, because I have a lots of form elements and I need to avoid layout trashing at any cost.

1. when futureNodes are Wire's instead of dom nodes, domdiff returns Wire's
2. these Wire's are stored into childNodes
3. if and only if some next call needs to rearrange nodes, then domdiff uses these childNodes, but that fails.
4. it fails, because Wire has 'dead', parentless nodes inside.

I do not see why you accuse me manually moving nodes, I do not move nothing manually, even in the example there. And I can fix that with the short code above myself. How can you explain that, my 'dewire' fix magically heals my 'manual' moves.

Anyway, you are too abrasive and unpleasant to work with. I can just fork.

[WebReflection]

Once again, domdiff has nothing to do with this: it accepts a get(node) function provided by hyperHTML.

domdiff also doesn't crash, as it's heavily tested and 100% code covered with all possible weird cases you want, including this one.

I do not move nothing manually

You are swapping hard wired internal content via random, but it's not about accusing anyone, it's just the pattern you used that is not a common one. You create wires somewhere else and you use these in any hole arbitrarily, which is not a scenario well handled by hyperHTML, because it works best with keyed nodes.

When you move nodes arbitrarily, you could have same node in two different places, as example, and that's an issue.

On top of that, you are using fragments (more than a node within a template literal and within the same non-existent root), so fragments are like that, you move a fragment around, you'll leave its nodes whenever they were, and this messes up with the tagger.

However, this is the issue in a nutshell:

• first div has a and b, then it has c and b, but a was moved somewhere else ... the moment hyperHTML remove a because its diff says it shouldn't be there, a could be removed from another parentNode that is not the one owned by the hard wired container
• when that happens, you have issues, because the other container had c, now it doesn't anymore, so that diffing/moving any node around c at that point is not possible, as c is somewhere else

With lighterhtml handling non keyed cases, like this one, you don't have DOM trashing, just updates when things are different per node, and you'll never have the issue a node that supposed to be in a specific parent, is now somewhere else.

That being said, if you believe there is a domdiff bug in here, then this is the wrong repository for that, but I still suggest you move to lighterhtml which has handling of non keyed cases in core, while hyperHTML doesn't.

I might re-open this just to have a better look, but it's clear to me already why things aren't working as expected, and I think that's by design: you should not move wired content around, you should rather create unique wires per container. That's a keyed case that should work without issues.

[WebReflection]

Anyway, you are too abrasive and unpleasant to work with. I can just fork.

I actually replied before reading this ... and now I feel bad because I've wasted time trying to explain things to you so ... I am closing this, as I have no interest in being insulted for free.

[WebReflection]

Explaining, investigating, suggest and create alternative live examples => you are too abrasive and unpleasant to work with

Congratulations for your awesome Open Source contribution, remember that once you fork this project, which is used in production by more than a company, you should also follow its LICENSE terms.

I will block you next time you are incapable of having a respectful conversation.