Issue when re-rendering empty

[keul]

note: Maybe this is not an issue, probably a wrong usage of the API, but as this wasted few hours I think it worth discussing.

I've a hooked-element/uhtml (however only uhtml seems involved in the issue) project where a component was rendering a set of items.

this.html`
      <ul>
        ${items.map((item) =>
            checkSomething(item) ? html`<li>${item.something}</li>` : html``
        )}
      </ul>`
  );

Some items must not be displayed, based on external factors.

I started this by using a react-like approach, where inside an items.map I was returning "null" to "render nothing".
I noted that returning "null" or empty strings is the wrong way, so I tried html`` and it worked.

But the issue arise when there's a re-render. In this case I get an error:

Uncaught DOMException: Node.insertBefore: Child to insert before is not a child of this node

I suspect the issue is related to multiple empty block one near another.

Here a codesandbox example:
https://codesandbox.io/s/uhtml-empty-issue-c7otb?file=/src/index.js

• first run at page load: everything is OK
• after click on "run" again: you (can) get the error

Question: although I easily fixed this by filtering out elements I don't want to display I'm wondering if this is an issue, or maybe something to be better documented. In any case I'm curious about what's wrong with html``.

Thanks!

[WebReflection]

I easily fixed this by filtering out elements I don't want to display

It's not just that ... if you don't filter you render <li></li> for no reasons, and it doesn't matter if these are empty or not, these are DOM nodes.

However, I am not sure I understand what is item.something ... if it's a string, fallback to "", if you want the LI, otherwise filter upfront, 'cause with less nodes, you'll have better performance, despite the double loop.

[keul]

It's not just that ... if you don't filter you render

for no reasons, and it doesn't matter if these are empty or not, these are DOM nodes.

Uhm… I don't think you get what I mean. I'm "just" returning html`` , no empty LI. (Also LI is just an example, I'm not even rendering a list in my real case).

Sorry, forget the code above, probably It's just confusing. try to look at the codesandbox

[WebReflection]

Also LI is just an example, I'm not even rendering a list in my real case

I just commented the example you provided

try to look at the codesandbox

OK, but it's the same issue ... holes are for nodes, you are still creating empty nodes for no reason, because you should filter upfront:

        ${numbers.filter(n => n > 0.6).map((n, index) =>
          html`<li>${index} - ${n.toFixed(2)}</li>`
        )}

Regardless if uhtml should, or should not, behave magically (which I don't like, in general), it's conceptually wrong to just render nothing.

Filtering holed Arrays (that's what you have there, basically a [node, null, node]) means degrading performance for everyone, because when the hole is an Array, uhtml expects it to be homogeneous, otherwise it cannot diff null with anything else, as null is one reference.

In few words, I don't think I am interested in slowing uhtml due sloppy, holed, arrays, but this is surely something I could put in the README/documentation.

I hope this answers, even if it's not what you expected 👋

[WebReflection]

P.S. by "holes" I mean template interpolations

[WebReflection]

P.S.2 ... if you really want to use that pattern, you can always returns instead a comment node, as that is preserved

thing ? html`<li>thing</li>` : html`<!---->`

this doesn't throw 👍 (but I still strongly suggest filtering instead, it's more logical and clean)

[WebReflection]

P.S.3 the issue, if you'd like to know details, is that:

html``

that is emptyness ... so it cannot find any node to reproduce:

• not a text node
• nothing at all

which is why comment work, it's used as placeholder to DOM diff next render.

Since there is a trim involved, not even \n would do, or an empty space, because templates are trimmed due the way these are used most of the time:

render(document.body, html`
  <div>hello</div>
`);

However, as I think it's developer's duty to pass meaningful data to render, instead of expecting automatic placeholders or pass null and expect nothing, the comment hacks clearly shows that the data is wrong, not the rendering engine, as it receives data that it shouldn't, so I hope this is actually a good thing, for cleaner layouts/DOM as result.

I will check if there is an easy way to skip null while rendering though, but I won't support empty templates (or, if I do, I will convert these as null).

[WebReflection]

P.S.4 (last one, I promise)

I had a look at udomdiff and this null idea would go on the critical path for rendering and diffing, as I need to check that b value is not null in various places ... otherwise I need to filter per each Array in search of null values ... and that's also critical path.

So, long story short, please don't pass data you are not supposed to render, and I'll explain this in the readme, after filing an issue.

[keul]

…and I'll explain this in the readme, after filing an issue.

This was my targeting opening the issue: I was pretty sure it was a wrong usage of the lib (.filter.map just works) but it was not easy to know why it is forbidden without even getting a console warning or something.

Thanks for the long explanation the emptyness stuff, very appreciated 🙇

[WebReflection]

trade offs for code size and performance:

• I could put renders within a try/catch
• I could inform users where they've done something wrong
• I rather keep this project as tiny, and fast, as possible, 'cause it's one of its main strength

👋

[WebReflection]

Added notes under About data purity within the README
https://github.com/WebReflection/uhtml#api-documentation