WebReflection / hyperHTML

A Fast & Light Virtual DOM Alternative
ISC License
3.06k stars 112 forks source link

Failed to execute 'insertBefore' on 'Node': The node before which the new node is to be inserted is not a child of this node. #336

Closed BlueWater86 closed 5 years ago

BlueWater86 commented 5 years ago

I'm using hyperHTML to build tables and everything that goes along with a table.

I understand this type of issue has been reported in the past without a reproducible sample. The bug was found during unit testing of one of our products and then a minimum reproducible sample created: https://codepen.io/bluewater86/pen/EGMRPe

On line 54:

    //This fails after sorting the rows array
    render() {
        return this.html`<tr class="${this.classes}" onclick=${this}>${this.cells}</tr>${this.expanded && childRow.for(this)}`
    }
    //This will also fail
    /*render() {
        return this.html`<tr class="${this.classes}" onclick=${this}>${this.cells}</tr><tr></tr>`
    }*/
    //This will be fine
    /*render() {
        return this.html`<tr class="${this.classes}" onclick=${this}>${this.cells}</tr>`
    }*/

On line 68, removing any 1 of those array items also stops the error from occurring.

I have tried to follow the error through hyperHTML but can't find the cause.

WebReflection commented 5 years ago

I can't see any error 🤷‍♂️

BlueWater86 commented 5 years ago

You checked in console? Please tell me this isn’t a chrome specific bug.

WebReflection commented 5 years ago

not in Chrome, not in Safari, what should I do to see any error? can you change the Pen to do so automatically ?

WebReflection commented 5 years ago

also @BlueWater86 are you sure you are using latest hyperHTML as in the pen case?

BlueWater86 commented 5 years ago

I've seen bugs with previous (forked) version of DomDiff that you were using, this is your current from https://unpkg.com/hyperhtml@latest/min.js.

Wow, looking now from my home PC, no error either. This appears to be browser specific. So much for being reproducible!

WebReflection commented 5 years ago

This appears to be browser specific

Which browser would show errors there? The only one I could imagine is one without native template element, but I'll wait for you to confirm how to see the failure there.

BlueWater86 commented 5 years ago

I can't be sure of what is going on now. The version of Chrome deployed to our dev PCs at work all exhibit the error. Usually (and incorrectly) I assume that a bug found in Chrome will be a bug in all browsers.

My version of chrome on private PC not showing the error.

BlueWater86 commented 5 years ago

I'm going to dive into the work repository now and increase the number of rows in the pen in a hope that that causes the error. Appreciate your time and once I have concrete reproduction (on many PCs) I will bug you again.

WebReflection commented 5 years ago

do you know which version of Chrome is deployed at work?

BlueWater86 commented 5 years ago

I've just updated the pen. I must have been in too much of a hurry and not saved the correct version. Now errors in Chrome, FireFox and Edge.

firefox chrome edge

WebReflection commented 5 years ago

quite possibly the best bug finding of the domdiff utility ... it's embarrassing how this got unnoticed until now, I guess not many uses wires, after all, to create their content.

WebReflection commented 5 years ago

P.S. you should be good to go now

BlueWater86 commented 5 years ago

Thanks very much. The work you’ve put together have given me a completely fresh view on JS.

BlueWater86 commented 5 years ago

Thanks for the prompt resolution. Browser UI tests all passing again.

tests passing