observablehq / stdlib

The Observable standard library.
https://observablehq.com/@observablehq/standard-library
ISC License
966 stars 83 forks source link

Adopt createContextualFragment. #18

Closed mbostock closed 6 years ago

mbostock commented 6 years ago

Fixes #17. Reference: https://talk.observablehq.com/t/video-element-not-displaying-in-safari-most-of-the-time/398/3

mbostock commented 6 years ago

So this totally breaks nested html templates. Glad I checked!

html`<table><tbody>${[0, 1, 2].map(i => html`<tr><td>${i}</i></tr>`)}</tbody></table>`

The problem is that the Template element is properly non-contextual, but createContextualFragment (as the name suggests) is contextual. So this:

document.createRange().createContextualFragment(`<tr><td>foo</i></tr>`)

Returns a DocumentFragment consisting of a single Text (“foo”)! Whereas this:

{
  const template = document.createElement("template");
  template.innerHTML = `<tr><td>foo</i></tr>`;
  return template.content;
}

Returns a DocumentFragment containing the expected HTMLTableRowElement.

So clearly we can’t just adopt createContextualFragment across the board if we want nested html templates to work correctly.

jashkenas commented 6 years ago

Good thing you checked!

For html at least, doing it a little more oldschool seems to work. (Although there's probably a case that I'm not thinking of...)

{
  const fragment = document.createDocumentFragment();
  const div = document.createElement("div");
  div.innerHTML = `OUR_HTML_STRING`;
  let child;
  while (child = div.firstElementChild) {
    fragment.appendChild(child);
  }
  return fragment;
}

image