observablehq / htl

A tagged template literal that allows safe interpolation of values into HTML, following the HTML5 spec
https://observablehq.com/@observablehq/htl
ISC License
305 stars 24 forks source link

Fix bug with interpolating the same (reference-equal) node >1 times #48

Closed lionel-rowe closed 1 year ago

lionel-rowe commented 1 year ago

Fixes https://github.com/observablehq/htl/issues/47:

const textNode = htl.html`x`
htl.html`${textNode}${textNode}` // renders `x` (expected `xx`)
htl.html`${[textNode, textNode]}` // renders `x` (expected `xx`)

This can be fixed by calling Node#cloneNode(true) on all interpolated DOM nodes or arrays of DOM nodes.

j-f1 commented 1 year ago

I worry that this could be expensive if you were to pass a large/deep DOM node in. Would it make sense to only do the clone if the node has an existing parent? Or even more specifically, if the node has already been inserted into this template literal?

lionel-rowe commented 1 year ago

I worry that this could be expensive if you were to pass a large/deep DOM node in. Would it make sense to only do the clone if the node has an existing parent? Or even more specifically, if the node has already been inserted into this template literal?

I've added a performance tests section to the notebook: https://observablehq.com/d/a56e4d1c697a45e7#performance-tests

Results vary quite a lot between runs and seem to be mainly based on the ordering of the cells (the cell that's placed 2nd when the page is loaded will render roughly twice as fast), but there's no noticeable difference between the clone version and the original non-clone version, at least for me on latest Chrome/Windows 11.

mbostock commented 1 year ago

The current behavior is the intended behavior. If you want to interpolate a copy, you’ll have to clone the node yourself. If we clone the node upon interpolation, then the node that you passed-in (intending it to be interpolated into the DOM) will not be the same as the node that that’s inserted into the DOM (only a copy will be inserted).