WebReflection / linkedom

A triple-linked lists based DOM implementation.
https://webreflection.medium.com/linkedom-a-jsdom-alternative-53dd8f699311
ISC License
1.71k stars 82 forks source link

Add test for templateElement.content.childNodes[0].replaceWith #279

Closed iacore closed 5 months ago

iacore commented 5 months ago

added a test. the test should not fail.

iacore commented 5 months ago

On HTMLTemplateElement.content

The following snippet from MDN implies that templateElement.content should not clone the node.

const templateElement = document.querySelector("#foo");
const documentFragment = templateElement.content.cloneNode(true);

I tried it in browser. It works as expected.

deno-dom works as expected.

Do you want to dig into the DocumentFragment hole without affecting performance?

I don't think it is possible to do that, given the current implementation seems a bit buggy.


class HTMLTemplateElement extends HTMLElement {
  constructor(ownerDocument) {
    super(ownerDocument, tagName);
    const content = this.ownerDocument.createDocumentFragment();
    (this[CONTENT] = content)[PRIVATE] = this;
  }

  get content() {
    if (this.hasChildNodes() && !this[CONTENT].hasChildNodes()) {
      for (const node of this.childNodes)
        this[CONTENT].appendChild(node);
    }
    return this[CONTENT];
  }
}

If I understand currently, get content() is cached once. Consider this example:

templateElement.content;
templateElement.childNodes.append(document.createElement('div'))
templateElement.content; // this would be stale
iacore commented 5 months ago

Not sure what to do with this, actually.

browser DOM says templateElement.childNodes should be empty, while linkedom seems to insert to template first, and then on first .content transfer to the inner DocumentFragment.

if there's a way to direct "adding elements" to the DocumentFragment in the first place...

this is complicated. @WebReflection can we do getter setter function proxy here? is it too complicated for linkedom?

WebReflection commented 5 months ago

It’s not complicated, rather about SSR performance where you never want to compute things more than once. May I ask your use case here? If it’s to test please read the non-goals section in the README. If it’s about something else please share, thanks.

P.S. I’m really focusing on other things these days and I won’t be able to help much but if it’s a quick thing I will try to help

iacore commented 5 months ago

The use case. It's about something else. It's already solved.

the PR: https://github.com/dringtech/lume/pull/1/files#diff-7ff608c5d5ab477d4ee84e3dd47fb9700039d91695614bb2e06968e824827094L53

the file: https://github.com/dringtech/lume/blob/1c382d983d62063e8d559af8e372273b58b5ddfc/plugins/inline.ts#L53

basically, in linkedom, template.querySelectorAll(selector) gets the right nodes, and template.content.querySelectorAll(selector) gets the wrong nodes.

i don't even know if that's the standard DOM usage. maybe it won't work in browser, but that's ok. lume a static site generator. the code only runs in Deno, which has no DOM API on its own. we (plan to) replace another DOM-in-JS library with linkedom to make the SSG go faster.

P.S. I’m really focusing on other things these days and I won’t be able to help much but if it’s a quick thing I will try to help

You can help at any time :+1: . At your own pace.