WebReflection / hyperHTML

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

Uncaught TypeError: Cannot read property 'insertBefore' of null #263

Closed jaschaio closed 6 years ago

jaschaio commented 6 years ago

This is probably not a bug but me doing something wrong.

Still I can't seem to find the cause of the problem and I think its within my usage of HyperHTML.

I am trying to build a drag and drop editor with hyperHTML and Dragula.

I have draggable items on the left side that can be copied to the editor on the right side. Further more blocks can be reordered within the right side of the app.

Here is the codepen

But at some point of dragging and reordering I get the Follwing Error:

domdiff.js:87: Uncaught TypeError: Cannot read property 'insertBefore' of null

error

pinguxx commented 6 years ago

This happens on a specific browser?, using chrome i can't reproduce it

jaschaio commented 6 years ago

Have you tried reordering the items on the right side? Adding new items from the left does work for me without errors as well. But as soon as I reorder items on the right side I get the error.

Got this in Google Chrome Version 69.0.3497.42 (Official Build) beta (64-bit) on MacOS and Google Chrome Versión 68.0.3440.84 (Offical Build) (64 bits). Just tried in in Firefox 61.0.2 as well and get the same error.

pinguxx commented 6 years ago

I was, it sort of looked like it was doing it but it is failing sorry, it brakes after i try to move it the second time

pinguxx commented 6 years ago

changed the renderItem in Editor to

return hyperHTML.wire( this, `:Item-${ item }` )`<div data-index=${ index } data-item=${ item }>
      ${ item }
    </div>`;

seems to fix it

WebReflection commented 6 years ago

there are few things not really OK in that code, as example a .map(this.renderItem) without passing this as second argument so that the context is not known.

However, I need to know if Dragula clones or copy the element in a way that interfere with hyperHTML logic and I am on vacation now, so I won't have a look before next week and also I am not sure I should support issues causes with 3rd parts libraries unless it's obvious it's an hyperHTML issue.

WebReflection commented 6 years ago

P.S. setState returns the instance itself which is fine for rendering so you can write:

update(props) {
  return this.setState(props, false);
}

The engine will invoke this.render() for you which is better because you'll keep trac of the component, instead of keeping track of the resulting node (which also might be the issue in here)

jaschaio commented 6 years ago

Thanks for your reply and the hint with returning this.setState() instead of this.render().

I usually call renderItem() with an arrow function like tree.map( ( item, index ) => this.renderItem( item, index ) ) so that this is already bound to the right context. Not sure if I had this wrong by calling simply map( this.renderItem ) in another version of the codepen so that you would be right in not having the right context.

Anyway, unfortunately none of those actually resolves the issue.

But I think you are right, this is NOT hyperhtml related. In the real application I have implemented Redux and if I just emmit an action for reordering the items in the editor bypassing dragulas drag and drop completely no errors are thrown.

It seems that even when calling dragula.cancel() it still mutuates the DOM with insertBefore and removeChild methods and thus interferes with hyperHTML.

If I take a look at the DOM during a drag and drop reordering I notice that the <!--_hyper: 12345678 --> reference gets removed and thus hyperHTML looses track of it and throws an error on the next render.

The only thing I keep wondering about is whats the right way to use third party libraries that modify the DOM together with hyperHTML? For React, Vue or Angular there are usually "bridges" which take care of the framework specific logic. But I have no idea where I should even start when trying to implement something similar for hyperHTML. Would this be a case where the adopt implementation could help? Or should I just keep away from any third party libraries and always roll my own simpler solutions?

WebReflection commented 6 years ago

@jaschaio thanks a lot for taking time to investigate and realize it wasn't this library fault.

I keep wondering about is whats the right way to use third party libraries that modify the DOM

hyperHTML "owns" the content it handles and every third part library that greedily changes such content will have hard time unless such library exposes a way to visually see changes but also a way to not interfere with what's live, providing info/details about the desired operation.

The issue here seems indeed to be that:

It seems that even when calling dragula.cancel() it still mutuates the DOM

so beside hyperHTML, this library seems to break developers expectations.

If I were you, I'd file a bug to dragula library 😉