Render Failed for dom diff

[liming]

While using hyperHTML, domdiff.js introduced several exceptions.

Exceptions are caused by code from this line.

      if (currentNodes[currentStart] == null) currentStart++;
      if (currentStart === currentEnd) {
        parentNode.removeChild(get(currentNodes[currentStart], -1));
      }
      else {
        const range = parentNode.ownerDocument.createRange();
        range.setStartBefore(get(currentNodes[currentStart], -1));
        range.setEndAfter(get(currentNodes[currentEnd], -1));
        range.deleteContents();
      }

Below code is to avoid empty node. However, it would cause exception if currentNodes[++currentStart] is empty as well. get(currentNodes[currentStart]) would cause exception.

if (currentNodes[currentStart] == null) currentStart++;

Another exception because I'm trying to using nested component. At this moment, if children component existed in currentNodes[currentStart] or currentNodes[currentEnd]. get(currentNodes[currentEnd] would render children components, which already use "future" properties and states. Children components might cause exception in this situation.

My implementation is complicated so it's difficult to write sample code. Usually it happens when nested components has dramatical changes. I will try to reproduce it with sample code when have time.

[WebReflection]

Hi @liming, domdiff is a repository a part so this ticket would rather belong there, not here.

I've covered 100% of the code with all possible edge cases I could think of, it'd be awesome to have a single use case that fails that is not covered.

My implementation is complicated so it's difficult to write sample code.

I understand, but also I cannot help much if I cannot reproduce a bug. domdiff logic is a bit articulated and without a perfect reproduction step it's like witch hunting.

Snabdom also should be warned if something goes wrong because we share basically same algo.

Usually it happens when nested components has dramatical changes.

Remember: if you ever use innerHTML or textContent or wipe in any other way content owned by hyperHTML you'll always have problems.

Usually these messes are caused by obtrusive 3rd parts libraries too, be sure that's not the case.

I will try to reproduce it with sample code when have time.

Whenever you'll find a way to reproduce, please write the example code in the ticket I've just created for you in the domdiff repo: https://github.com/WebReflection/domdiff/issues/3

Thanks.

[atirip]

I think I have same issue just now and it is a bitch to debug and reproduce. It occures randomly e.g. the content is rendered fine unless it is not and remove() in domdiff crashes, because it it operates with comment that at this stage has no parentNode. In the code, here when any is called, node is lone comment node.

        // in a hyper(node)`<div>${content}</div>` case
        // everything could happen:
        //  * it's a JS primitive, stored as text
        //  * it's null or undefined, the node should be cleaned
        //  * it's a component, update the content by rendering it
        //  * it's a promise, update the content once resolved
        //  * it's an explicit intent, perform the desired operation
        //  * it's an Array, resolve all values if Promises and/or
        //    update the node with the resulting list of content
        any: function any(node, childNodes) {

When I go up in Call Stack, however:

                return function() {
                    var length = arguments.length;
                    var values = length - 1;
                    var i = 1;

                    if (len !== values) {
                        throw new Error(values + ' values instead of ' + len + '\n' + template.join(', '));
                    }

                    while (i < length) {
                        callbacks[i - 1](arguments[i++]);
                    }
                    return content;
                };

here none of the arguments is comment (https://imgur.com/a/bTACUEs), that means DOM is updated and comment nodes are left in (https://imgur.com/a/O6ZEO7B) and they mess things up?

I wish I could give you better info, but that is what I have now. The piece of DOM I operate is changing its content fully, depending of things elsewhere (this is panel for editing options and they are different for all the objects I have in my editor). Root of the code is like this:

    renderOptions: function(props) {
        if (!props.options.length) return;
        return hyperHTML.wire(props, this.wireId(props.type, 'parameters'))`
                    <h2>${props.optionTitle}</h2>
                    ${props.options.map(function(opt, index) {
                        var type = opt.type;
                        //debugger;
                        return hyperHTML.wire(props, this.wireId(opt.name, 'parameter', index))`
                            <row>
                                <p>${opt.caption}</p>
                                <p>${opt.sublabel}</p>
                                ${this['render_' + type](props, opt)}
                            </row>
                        `;
                    }, this)}
                `;
    },

    render: function(props) {
        return hyperHTML.wire(props, this.wireId(props.type))`
            <row>
            <h2>${props.title}</h2>
            <p class=caption>${props.caption}</p>
            <p>${props.description}</p>
            <p>${props.description1}</p>
            </row>
            ${this.renderOptions(props)}
        `;
    }

[WebReflection]

To start with, the snabdom logic has gone long time ago, and there are no issues with current petit-dom like one.

The comments are also something you shouldn't care about, 'cause are necessary to pin changes/holes in your page, so it's not messing, it's how the engine works (same with every other alternative).

The main issue I see, beside a lot of fragmented content that should, anyway, be handled, is the undefined return when props.options.length happens.

That return is not bound to any wire, so it wipes out everything without recovery ability.

I am quite sure if you have props.options.length, then you don't, then you have it again, is where you have issues.

So how about you always return a wire instead ?

var html = hyperHTML.wire(props, this.wireId(props.type, 'parameters'));
if (!props.options.length) return html``;
return html`
  <h2>${props.optionTitle}</h2>
  ${props.options.map(function(opt, index) {
    var type = opt.type;
    //debugger;
    return hyperHTML.wire(props, this.wireId(opt.name, 'parameter', index))`
      <row>
        <p>${opt.caption}</p>
        <p>${opt.sublabel}</p>
        ${this['render_' + type](props, opt)}
      </row>`;
    }, this)}`;

In that case the comment won't ever be wiped out from an undefined hole.

[atirip]

I remember I had problems with that earlier, that is why i switched to undefined in the first place and so far that has worked. But I did as you suggested and, pretty soon (I just switch between my edit tabs and change some select options inside: https://imgur.com/a/RVWfZ2c - I am 99% sure (I debugged that some time ago), this is the same issue when comment node parent is null.

[atirip]

To add. returning empty wire now crashes every time, even in first render.

[WebReflection]

could you please try with this?

if (!props.options.length) return html`${[]}`;

Is there any other library interfering (modifying) the content ?

[WebReflection]

P.S. alternatively, what if you always return the whole thing instead ? you'll have an empty map which is OK.

If you could reduce to the minimum the example it would also help, instead of guessing 😉

[WebReflection]

P.P.S. I hope you are running the latest version of the library, since I've recently updated domdiff too. Please be sure domdiff is at 2.0.3

[WebReflection]

Example, if everything else fails:

var html = hyperHTML.wire(props, this.wireId(props.type, 'parameters'));
return html`
  <h2>${props.options.length ? props.optionTitle : ''}</h2>
  ${props.options.map(function(opt, index) {
    var type = opt.type;
    return hyperHTML.wire(props, this.wireId(opt.name, 'parameter', index))`
      <row>
        <p>${opt.caption}</p>
        <p>${opt.sublabel}</p>
        ${this['render_' + type](props, opt)}
      </row>`;
    }, this)}`;

[atirip]

So far so good, that empty array seems to work. No crashes so far. Thanks a lot.

PS! No, I am not using anything else to write to DOM, whenever I need to, I write the styles into stylesheet even e.g. I dynamically add/remove tgem :-)

[WebReflection]

So far so good, that empty array seems to work. No crashes so far. Thanks a lot.

Glad to hear that. Yes, fragmented to single node, or vice-versa, can be tricky.

One day I'll try to see how bad is for performance to keep everything like a fragment by default, but for the time being I hope that's not too bad as compromise to make everything works.