choojs / nanocomponent

🚃 - create performant HTML components
https://leaflet.choo.io/
MIT License
366 stars 30 forks source link

Can't handle svg #78

Closed YerkoPalma closed 6 years ago

YerkoPalma commented 6 years ago

I have a simple component that should render an svg. But nanocomponent throws

Uncaught Error: nanocomponent: createElement should return a DOM node
    at assert (index.js:21)
    at Block.Nanocomponent._handleRender (index.js:93)
    at Block.Nanocomponent.render (index.js:71)
    at Object.values.map.block (blockPanel.js:33)
    at Array.map (<anonymous>)
    at BlockPanel.createElement (blockPanel.js:33)
    at BlockPanel.Nanocomponent._handleRender (index.js:90)
    at BlockPanel.Nanocomponent.render (index.js:71)
    at Choo.mainView [as _handler] (index.js:21)
    at Choo._prerender (index.js:244)

that's because this line fails. I think this assertion should be fixed to include svg (and actually everything that can be handled by bel itself)

bcomnes commented 6 years ago

Oh good catch. Is there a similar assertion that covers all node types?

YerkoPalma commented 6 years ago

Well, for svg it could be

svg instanceof SVGElement

Now, both, SVGElement and HTMLElement inherits from Element, so for both this works

el instanceof Element
bcomnes commented 6 years ago

I’m cool with it

ungoldman commented 6 years ago

buuuut theoretically '' should be valid too right? or any text fragment?

YerkoPalma commented 6 years ago

Why should they be valid?

bcomnes commented 6 years ago

A nanocomponent consisting of a text node? Eh. Wrap it it in a span

bcomnes commented 6 years ago

Nanocomponent requires attributes and IDs on the root node

YerkoPalma commented 6 years ago

Well, if checking against the Element interface is a good solution, I could submit a PR, but I'm not sure if I should wait for #77 or something?

bcomnes commented 6 years ago

No we can land this sooner than later