AFASSoftware / maquette

Pure and simple virtual DOM library
https://maquettejs.org
MIT License
774 stars 63 forks source link

Feature request: print nodes when checkDistinguishable throws #174

Closed solowt closed 2 years ago

solowt commented 2 years ago

When maquette can't distinguish between vnodes with the same parent it throws an error. In a big app with a complicated DOM it can be hard to figure out which nodes need to have keys added to make them distinguishable. Including the nodes with the error would make it easier.

Here's a possible approach: https://github.com/solowt/maquette/commit/de79b565a9133601fabdbac2fe3c6c6bd73d2467

What do you think? And thanks for the library, it's a pleasure to use.

johan-gorter commented 2 years ago

I think you are right that these errors are usually hard to pinpoint. I am a bit reluctant to use throw with something else than Error, because using Error seems to be the standard. Nontheless, after experimenting a bit, throwing an object seems to be the only way to add VNodes (and real nodes) to the stacktrace in the console. An alternative would be to allow customizing the error-throwing mechanism using a ProjectorOptions.handleError(...args: any[]):never but this would not benefit other users.

I am inclined to agree with your solution of throwing an Object instead of an Error. Anyone else with strong opinions about this? @jvanoostveen @jcfranco ?

jcfranco commented 2 years ago

I agree that throwing something that is not an Error instance would be unexpected. Would throwing a custom error instance with a custom detail property, or similar, be an acceptable alternative? The (v)nodes still won't show up in the console, from what I can tell, but at least they'll be on the error instance.

https://codepen.io/jcfranco/pen/JjOJJaO?editors=1010

johan-gorter commented 2 years ago

After experimenting with window.onerror a bit I am becoming more convinced that throwing an object instead of an Error is the easiest way and benefits devs the most. A configurable custom error handler would not help beginners. It will not break anything, because catching this error can only be done from within the requestAnimationFrame in the projector.