ProjectEvergreen / wcc

Experimental native Web Components compiler.
https://merry-caramel-524e61.netlify.app
83 stars 6 forks source link

browser warning about "declarative Shadow DOM has not been enabled by includeShadowRoots" #130

Closed thescientist13 closed 10 months ago

thescientist13 commented 10 months ago

Summary

Have been noticing this warning lately in Chrome , like if you go here and click the Load Products button at the bottom of the page https://greenwood-demo-adapter-vercel.vercel.app/

Found declarative shadowrootmode attribute on a template, but declarative Shadow DOM has not been enabled by includeShadowRoots.

Screenshot 2024-01-03 at 11 16 56 AM Screenshot 2024-01-03 at 11 18 08 AM

It doesn't actually seem to be an issue with WCC per se, but rather just how we are loading it on the page? 🤔 https://github.com/ProjectEvergreen/greenwood-demo-adapter-vercel/blob/main/src/pages/index.html#L18C1-L32C14

<!-- Fragment API Setup -->
<script>
  globalThis.addEventListener('DOMContentLoaded', () => {
    const limit = 10;
    const page = limit;
    let offset = 0 - page;

    globalThis.document.getElementById('load-products').addEventListener('click', async () => {
      offset = offset += page;
      const html = await fetch(`/api/fragment?offset=${offset}&limit=10`).then(resp => resp.text());

      document.getElementById('load-products-output').insertAdjacentHTML('beforeend', html);
    });
  });
</script>

Interestingly, the same implementation using HTMX does not complain about it. https://greenwood-htmx.vercel.app/

Screenshot 2024-01-03 at 11 20 25 AM

Details

Per some reading around, it does seem that this is related to the use of insertAdjacentHTML, as per these docs, it suggests that it would be required to use DOMParser instead

To avoid some important security considerations, Declarative Shadow Roots also can't be created using fragment parsing APIs like innerHTML or nsertAdjacentHTML(). The only way to parse HTML with Declarative Shadow Roots applied is to pass a new includeShadowRoots option to DOMParser:

<script>
const html = `
<div>
<template shadowrootmode="open"></template>
</div>
`;
const div = document.createElement('div');
div.innerHTML = html; // No shadow root here
const fragment = new DOMParser().parseFromString(html, 'text/html', {
includeShadowRoots: true
}); // Shadow root here
</script>

I can see for example that HTMX is using DOMParser, but just not with the includeShadowRoots option.

function parseHTML(resp, depth) {
  var parser = new DOMParser();
  var responseDoc = parser.parseFromString(resp, "text/html");

   // ...
}

(more context here - https://github.com/whatwg/dom/issues/912#issuecomment-732476002 )


I was seeing some of this behavior even with HTMX as part of the solution for #128 ( see #129) so want to conduct some more tests to arrive at a proper conclusion and if this is just a userland issue, or anything WCC itself actually needs to worry about.

So going to play around with some of our implementations and ascribe any relevant documentation if needed and report back.

thescientist13 commented 10 months ago

Can confirm converting to use DOMParser makes the issue go away as seen in https://github.com/ProjectEvergreen/greenwood-demo-adapter-vercel/pull/25, though technically includeShadowRoots isn't needed since we are constructing our shadowRoot through a <template> tag?

export default class Greeting extends HTMLElement {

  connectedCallback() {
    if (!this.shadowRoot) {
      const template = document.createElement('template');

      template.innerHTML = `
        <div>
          <h1>Hello World!</h1>
        </div>
      `;
      this.attachShadow({ mode: 'open' });
      this.shadowRoot.appendChild(template.content.cloneNode(true));
    }
  }
}

customElements.define('app-greeting', Greeting);

If we do something like this though, then we will get the error and we will have to use includeShadowRoots it seems

export default class Greeting extends HTMLElement {
  connectedCallback() {
    if (!this.shadowRoot) {
      this.attachShadow({ mode: 'open' });
      this.shadowRoot.innerHTML = `
        <div>
          <h1>Hello World!</h1>
        </div>
      `
    }
  }
}

customElements.define('app-greeting', Greeting);

hmm, looks like it happens even when manually setting innerHTML, even in the component definition itself? Screenshot 2024-01-03 at 2 15 58 PM

So not sure how much this does / doesn't muck with any assumptions we are making about how to construct "dynamic" templates I guess, or at least we should emphasize the work arounds needed if you don't use <template>.

thescientist13 commented 10 months ago

So yeah, for #128 , we can't use innerHTML even in the component definition, so best option would be to use <template> elements, otherwise the browser will complain about that too Screenshot 2024-01-03 at 6 53 46 PM

Will see if there are some docs to add around this, but not sure there's much more for us to do here at this point. I did open up an issue to the MDN docs about documenting includeShadowRoots - https://github.com/mdn/content/issues/31501