Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace deprecated method getInnerHTML with getHTML #177

Closed
DannyMoerkerke opened this issue Nov 20, 2024 · 12 comments · Fixed by #171
Closed

Replace deprecated method getInnerHTML with getHTML #177

DannyMoerkerke opened this issue Nov 20, 2024 · 12 comments · Fixed by #171
Assignees
Labels
0.16.0 breaking documentation Improvements or additions to documentation enhancement Improvement to existing functionality
Milestone

Comments

@DannyMoerkerke
Copy link
Contributor

Type of Change

Alignment with specs

Summary

Replace the deprecated Element.prototype.getInnerHTML with Element.prototype.getHTML in dom-shim.js

Details

This issue is in addition to my PR #171

Element.prototype.getInnerHTML has been deprecated in favor of Element.prototype.getHTML (https://developer.mozilla.org/en-US/docs/Web/API/Element/getHTML).

Currently in dom-shim.js, the setter for innerHTML for the HTMLTemplateElement automatically and incorrectly adds a <template> tag with a shadowrootmode attribute to the HTML content of the HTMLTemplateElement itself.

The result is that the innerHTML property of the HTMLTemplateElement will return its HTML contents including the <template> tag which is incorrect and not according to the specs as it should only return its contents without this <template> tag.

Element.prototype.getInnerHTML should be replaced with Element.prototype.getHTML which provides an options argument that enables the serialization of child nodes that are shadow roots:

<section>
  <template shadowrootmode="open" shadowrootserializable>
    <p>foo</p>
  </template>
  <p>bar</p>
</section>

section.getHTML();

// returns:
<p>bar</p>

section.getHTML({{serializableShadowRoots: true});

// returns:
<template shadowrootmode="open" shadowrootserializable>
   <p>foo</p>
</template>
<p>bar</p>

This also makes sure that Custom Elements that have their shadow roots set through innerHTML can be server-side rendered:

const shadowRoot = this.attachShadow({mode: 'open'});
shadowRoot.innerHTML = `...`;
@thescientist13
Copy link
Member

thescientist13 commented Nov 21, 2024

Hey @DannyMoerkerke thanks for much for reporting this and providing that example of getHTML, definitely helping me to better understand the issue.

So if I were to summarize in my own words what you are proposing:

  1. move auto-wrapping out of the setter
  2. instead, enable auto-wrapping through the getter (now using a combination of getHTML + serializableShadowRoots: true)

Regarding point 1, I think this aligns with an observation @briangrider had while working on #170 that he saw some double wrapping of <template> tags, like in this test case. Does that align with your testing / validation as well, and was double wrapping happening in those two other test cases in your PR as well? Since it seems like the auto-wrapping is just moving, just wanted to make sure it explains those other two test case changes in your PR, for my own understanding

So in general, with the changes in your PR, no user would ever have to manually do this anymore, yes?

this.shadowRoot.innerHTML = `<template shadowrootmode="open"> ... </template>`

And the example from the README (the "Get HTML!" output) of manually appending a template would still work? (which based on no other test cases changing in your PR, seems like it would?)

const template = document.createElement('template');

template.innerHTML = `
  <footer class="footer">
    <h4>My Blog &copy; ${new Date().getFullYear()}</h4>
  </footer>
`;

class Footer extends HTMLElement {
  connectedCallback() {
    if (!this.shadowRoot) {
      this.attachShadow({ mode: 'open' });
      this.shadowRoot.appendChild(template.content.cloneNode(true));
    }
  }
}

export default Footer;

customElements.define('wcc-footer', Footer);

And so if WCC is going to do auto-wrapping now, at least it would based on actual standard API specifications?


Reading the MDN docs, I wasn't sure how to interpret what they were saying, if it would auto-wrap, or just include the <template> tag in the return value if a <template> tag already existed, be it manually through innerHTML or appending a template.

I did create a demo to familiarize with getHTML behavior but couldn't get it to work in returning a <template> tag when calling getHTML({ serializableShadowRoots: true }), but I'm probably just doing it wrong 😅
https://github.com/thescientist13/serializable-shadow-roots

Screenshot 2024-11-21 at 10 53 03 AM

If it is intended to do auto-wrapping (or we're proposing this as an explicit value added feature of WCC) I think it would be important to call that out in the documentation / examples as part of the PR, since it's probably not obvious from the outside that setting innerHTML would generate a <template shadowrootmode="open"> automatically.

Just want to avoid any "self-inflicted" double wrapping by user's (still) doing this by educating and informing of WCC's behavior

this.shadowRoot.innerHTML = `<template shadowrootmode="open">...</template>`

As a bit of an aside, I'm curious if you have any thoughts on this issue - #65?

At the time I was stuck on how to get access to mode, so just wanted to throw that one out there to see if you have any thoughts on how to make that possible (and we can discuss over in that issue if it's of interest to you). 🤞

getHTML({ serializableShadowRoots = false }) {
  const mode = this.shadowRoot.mode ?? false;
   
  return this.shadowRoot && serializableShadowRoots ?
    `<template shadowrootmode="${mode}">${this.shadowRoot.innerHTML}</template>` : this.innerHTML;
}

Thanks again for putting all this together, and apologies in advance for having such a lengthy reply, but definitely excited to see what we can achieve here, and certainly appreciate the opportunity to get learn about new web APIs! 🧠

@DannyMoerkerke
Copy link
Contributor Author

Regarding point 1, I think this aligns with an observation @briangrider had while working on #170 that he saw some double wrapping of <template> tags, like in this test case. Does that align with your testing / validation as well, and was double wrapping happening in those two other test cases in your PR as well? Since it seems like the auto-wrapping is just moving, just wanted to make sure it explains those other two test case changes in your PR, for my own understanding

I haven't been able to log the HTML of those two test cases but I noticed that they failed because the elements that were asserted were not found in the DOM. Removing the hard-coded adding of the <template> fixed the tests.

So in general, with the changes in your PR, no user would ever have to manually do this anymore, yes?

this.shadowRoot.innerHTML = `<template shadowrootmode="open"> ... </template>`

Exactly, the <template> will now be returned when getHTML({serializableShadowRoots: true}) is invoked.

And the example from the README (the "Get HTML!" output) of manually appending a template would still work? (which based on no other test cases changing in your PR, seems like it would?)

const template = document.createElement('template');

template.innerHTML = `
  <footer class="footer">
    <h4>My Blog &copy; ${new Date().getFullYear()}</h4>
  </footer>
`;

class Footer extends HTMLElement {
  connectedCallback() {
    if (!this.shadowRoot) {
      this.attachShadow({ mode: 'open' });
      this.shadowRoot.appendChild(template.content.cloneNode(true));
    }
  }
}

export default Footer;

customElements.define('wcc-footer', Footer);

Yes, this works as expected.

And so if WCC is going to do auto-wrapping now, at least it would based on actual standard API specifications?

Yes, it is: https://developer.mozilla.org/en-US/docs/Web/API/Element/getHTML

Reading the MDN docs, I wasn't sure how to interpret what they were saying, if it would auto-wrap, or just include the <template> tag in the return value if a <template> tag already existed, be it manually through innerHTML or appending a template.

No, the <template shadowrootmode="open"> is returned when the custom element has a serializable shadow root.

I did create a demo to familiarize with getHTML behavior but couldn't get it to work in returning a <template> tag when calling getHTML({ serializableShadowRoots: true }), but I'm probably just doing it wrong 😅 https://github.com/thescientist13/serializable-shadow-roots

There are some issues here. First, a shadow root is only serializable (https://developer.mozilla.org/en-US/docs/Web/API/ShadowRoot/serializable) if it's created with the options.serializable parameter set to true when created with Element.attachShadow():

this.attachShadow({ mode: "open", serializable: true });

or when the shadowrootserializable attribute is set on the <template>:

<template shadowrootmode="open" shadowrootserializable>

Also, getHTML() is called on the element itself, not its shadow root:

const htmlSerializedTemplate = cardWithTemplate.getHTML({ serializableShadowRoots: true });

If it is intended to do auto-wrapping (or we're proposing this as an explicit value added feature of WCC) I think it would be important to call that out in the documentation / examples as part of the PR, since it's probably not obvious from the outside that setting innerHTML would generate a <template shadowrootmode="open"> automatically.

Setting innerHTML does not add a <template> it's only returned when getHTML() is called with the right options and the shadow root is serializable.

Just want to avoid any "self-inflicted" double wrapping by user's (still) doing this by educating and informing of WCC's behavior

this.shadowRoot.innerHTML = `<template shadowrootmode="open">...</template>`

They should never do this for the reason stated above.

Strictly speaking, we should only render the shadow root when it's serializable but users may not specify this when calling attachShadow and only the component's light DOM would then be server-side rendered. I'm afraid this will lead to unexpected results.

On the other hand, we should follow the specs so we should consider adding this to the docs.

As a bit of an aside, I'm curious if you have any thoughts on this issue - #65?

At the time I was stuck on how to get access to mode, so just wanted to throw that one out there to see if you have any thoughts on how to make that possible (and we can discuss over in that issue if it's of interest to you). 🤞

getHTML({ serializableShadowRoots = false }) {
  const mode = this.shadowRoot.mode ?? false;
   
  return this.shadowRoot && serializableShadowRoots ?
    `<template shadowrootmode="${mode}">${this.shadowRoot.innerHTML}</template>` : this.innerHTML;
}

I'll have a look.

Thanks again for putting all this together, and apologies in advance for having such a lengthy reply, but definitely excited to see what we can achieve here, and certainly appreciate the opportunity to get learn about new web APIs! 🧠

You're welcome! Thanks for this great project, I'm excited about it as well!

@briangrider
Copy link

briangrider commented Nov 22, 2024

Hi @DannyMoerkerke,

It looks like we're tackling some of the same issues and I wanted to show you what I've been working on with @thescientist13 and get your take.

In an attempt to get child element properties working in wcc (and in turn many features of various frameworks like litHTML's child element properties, etc.), I've spent a lot of time updating the DOM shim to work more like the browser implementation of the DOM, specifically, storing childNodes as objects and serializing them when accessing innerHTML rather than storing everything as strings. This simplifies many operations in wcc itself, improves performance by cutting down on the number of parses and serializes in wcc when appending templates and childNodes (instead of using innerHTML), and ultimately, makes it easier to align things with the spec.

For example, in regards to simplifying wcc, I was able to simplify renderComponentRoots (and renderToString) pretty significantly. You can see we can remove getInnerHTML (or getHTML), innerHTML, elementHTML, elementTree, hasLight, renderLightDomChildren, and VOID_ELEMENTS with this new implementation:
https://github.com/briangrider/wcc/blob/master/src/wcc.js

Here's the new dom shim:
https://github.com/briangrider/wcc/blob/master/src/dom-shim.js

With this implementation, we don't even need to shim getHTML (but we can if there's a good reason).

I would argue that in working toward true isomorphic web components, we should generally be matching the user's browser-based expectations and be forgiving of leaving out the serializable property when instantiating a shadowRoot but maybe respect it if it's specifically set to false.

btw, this line here: https://github.com/briangrider/wcc/blob/master/src/dom-shim.js#L230
is just there to pass those tests where innerHTML strings are already wrapped in template tags. When those tests are fixed like you proposed, that will be removed

If we don't shim getHTML, we could achieve a forgiving implementation of the spec like this:

class ShadowRoot extends DocumentFragment {
  constructor(options) {
    super();
    this.mode = options.mode ?? 'closed';
    // Unlike the spec, default to true to better match browser experience/expectations
    this.serializable = options.serializable ?? true;
    this.adoptedStyleSheets = [];
  }

  get innerHTML() {
    return this.childNodes ? serialize({ childNodes: this.childNodes }) : '';
  }

  set innerHTML(html) {
    html = this.serializable ? `<template shadowrootmode="${this.mode}" shadowrootserializable>${html}</template>` : html;
    this.childNodes = parseFragment(html).childNodes;
  }
}

Again, I think users should be able to take a complex project (with many web components) that was built for the browser, and render it on the server without modifying each and every component. There are also frameworks people may be using where they don't even have access to the serializable option for their shadow roots.

In my opinion, it is worth the slight deviation from spec on this (specifically, defaulting serializable to true instead of false). As a user coming to wcc, I would expect I can just take my working code in the browser and then render it on the server without issue. I imagine having to remind everyone to set serializable to true whenever attaching a shadow seems like it could hurt user adoption. Let me know your thoughts!

@thescientist13
Copy link
Member

thescientist13 commented Nov 23, 2024

@DannyMoerkerke

There are some issues here. First, a shadow root is only serializable (https://developer.mozilla.org/en-US/docs/Web/API/ShadowRoot/serializable) if it's created with the options.serializable parameter set to true when created with Element.attachShadow():

Ah, this is definitely the part I was missing, and was able to confirm as such in @briangrider 's demo, so this all excellent to hear!
Screenshot 2024-11-22 at 7 53 16 PM


So know that I've had a chance to dig into #178 a bit deeper, it does seem to cover everything we've been discussing so far with a bonus

I can understand that #178 certainly makes it much easier to achieve all those, and in fact might make it "harder" to not implement them, so while I would traditionally prefer to see mode and serializable land as their own PRs in addition, I do see how taking them on as a big-bang is probably the happiest path towards the end state we are all looking for.

So I'll leave it up to you both @briangrider and @DannyMoerkerke how you feel about landing this all at once, and that if we do go with the big bang, that @briangrider you would also need to absorb some of the extra feedback for #171 , mainly adding documentation and / or updating any examples on the WCC website, and validating all the sandbox examples still work and / or need updating.

Thoughts? Also happy to setup a chat or start a thread in the Project Evergreen Discord if that will help, but sounds like we have everything we need, and just want to make sure we're all aligned on the path to get there. If we feel good about #178 then I am happy to give it a thorough test against Greenwood's test suite and a couple sample projects, and then we can land that and incorporate part 2 of @briangrider work, which I can revalidate in the same way. From there seems like we should be on fast track to release a new minor version of WCC (v0.16.0). 💯


So cool and super excited for both of your contributions and input here, thank you so much! 🙇‍♂️

@briangrider
Copy link

So I'll leave it up to you both @briangrider and @DannyMoerkerke how you feel about landing this all at once, and that if we do go with the big bang, that @briangrider you would also need to absorb some of the extra feedback for #171 , mainly adding documentation and / or updating any examples on the WCC website, and validating all the sandbox examples still work and / or need updating.

Sounds good! So @DannyMoerkerke, what I'd love to take from your PR (since getInnerHTML/getHTML won't exist in WCC anymore) is:

  1. Updating the ShadowRoot class as mentioned above so that serializable can be set to false when attaching a shadowRoot. This deviates from spec slightly since serializable usually defaults to false but it a) better matches user expectations when server rendering web components built for the browser, b) cuts down on potential issue reports, and c) more easily allows for isomorphic web components without refactoring for server rendering. Since serializable can only be set when attaching a shadow root, and a shadow root can only be attached once (we should probably throw an error for this in a future PR btw), that means we can determine right away whether the declarative shadow dom should be serialized for a particular element. I put together a little codepen here testing various cases:
    https://codepen.io/scmrecordings/pen/NWQQqOy?editors=1111 If we were to do things exactly to spec, it seems like setting serializable to false, or setting serializableShadowRoots to false with getHTML leads to nothing being rendered in most cases, not just a lack of a wrapping template, so maybe down the line we should look at reproducing each of those conditions with the dom shim. But since getHTML won't be a part of the dom shim for now, I think we can let it slide.

  2. Remove the wrapping template tag from all of the tests mentioned in your PR. When I issued my PR, I put in code to pass those tests by de-wrapping strings that start with a template tag:
    https://github.com/briangrider/wcc/blob/master/src/dom-shim.js#L232
    This was with the intention of fixing the tests and removing that later but if we want to push all of this in a single PR, we can just incorporate all of your modified tests in one go.

  3. The missing closing bracket that you caught in counter.js. Good find!

Let me know if this sounds good to everyone!

@thescientist13
Copy link
Member

thescientist13 commented Nov 23, 2024

@briangrider

Just to make sure I understand what you are saying in point 1 above, auto-wrapping should only happen if the user authors their custom element as such:

this.attachShadow({ serializable: true })

If that condition is present, than I think we are ok to match that internally with getHTML + serializableShadowRoots: true, and then then just document this as a compliment / convenience as opposed to using document.createElement('template') and appending into the shadow root.


As far as the order of operations, I would just like to clarify that I would not like the refactor introducing any new behavior / fixes that are not already in the system. I would prefer fixes and changes in behavior of this significance / meaning get isolated to their own change, to avoid having too much context going on as part of one review.

So I see two options in regards negotiating the two open PRs that I think we would all want to be on the same page for:

Either:

  1. Land @DannyMoerkerke PR first, in which case the refactor can now rebase and account for this new serialization behavior

or

  1. Land the refactor first, and have Danny rebase and introduce the new serialization detection / intent behavior

My hunch is that we probably want the refactor to land first, which I can handle since it is so foundational, (even if it means slight disruption to the serializable PR) and from there new PRs can come in parallel (element properties, mode, etc).

I know it might sounds a little cumbersome, but I just generally prefer atomic commits where possible, in particular with so many moving parts and especially with these new behaviors we're adding / fixing, it will be preferable to only have to review / test / validate one thing at a time. (and also we can start moving individual conversations / feedback back to individual issues as now this issue has kind of taken on the burden of co-ordinating multiple moving parts, and so kind of getting caught in the crossfire).


Let's move this operational chat to Discord and / or a call if needed, since I think we all mainly agree on implementation details and outcomes, and so just need to sequence the open items together.

@briangrider
Copy link

briangrider commented Nov 23, 2024

Let's move this operational chat to Discord and / or a call if needed, since I think we all mainly agree on implementation details and outcomes, and so just need to sequence the open items together.

Yes, I think moving this to Discord is a good idea!

@DannyMoerkerke
Copy link
Contributor Author

Hi @briangrider,
I think it ultimately comes down to what parts of dom-shim.js we expect to be used when server-side rendering web components.

What I mean by that is if we expect to server-side render web components that use a method like getHTML internally, it should follow the specs. Having said that, I do think we should be pragmatic and make shadow roots serializable by default or not use getHTML altogether.
However, a good reason to implement getHTML according to the specs is that web components that are server-side rendered with wcc might use it internally. That chance might be slim, but I think we should account for it.

My initial motivation to create the PR was that web components that have their shadow root set through this.shadowRoot.innerHTML = '...' were not properly server-side rendered and when I noticed the dom shim was using the deprecated getInnerHTML I decided to replace it with getHTML.

@thescientist13
Copy link
Member

thescientist13 commented Dec 2, 2024

So taking into account everything we've been discussing, at a high level, here are my closing thoughts:

  • After reviewing the WCC docs, we should definitely try and maintain the "public" API of the DOM shim, which includes exposing getInnerHTML, now being replaced by getHTML. There was a poignent example of using WCC's DOM shim standalone that was helpful in my early demos of using WCC in edge environments, where things like fs being used in wcc.js would not have worked, see https://merry-caramel-524e61.netlify.app/examples/#serverless-and-edge-functions
  • The above does not necessarily invalidate the refactoring changes, only that the refactoring changes should not remove any useful APIs that currently exist. To be clear, I am totally OK with refactoring the underlying implementation details, since its clear there was a lot of wasted operations happening in the current implementation between the DOM Shim <> wcc.js and so that's definitely a significant benefit to all users of WCC.
  • Probably more controversially, I am still on the fence of whether to assume any of these APIs should have their serialization options set to true by default. I understand the convenience, but given how much WCC embraces / encourages writing to standards, flipping all these bits "silently" feels like it goes against the grain of everything else we document / demonstrate. I don't think it's that much more inconvenient to just have to set the flag programmatically, e.g. this.attachShadow({ serializable: true })
  • That said, I would totally be open to a compiler level option to apply this automatically so users can set that flag and WCC will assume whatever value gets passed in, e.g. renderToString(url, { serializeAllTheThings: true }) would be the same as setting serializable: true for all your custom elements

So, with that stated, here is the path I see for us to get there, building up one PR at a time from what we have, in this order:

  1. Update to use getHTML (@DannyMoerkerke ) - feature/issue 177 Replaced deprecated method getInnerHTML with getHTML #171
  2. Refactor DOM Shim (@briangrider ) - Rework of DOM shim (using parse5) to allow for element properties in the future #178
    • rebase after merging of getInnerHTML refactor PR
    • preserve getHTML in the DOM shim / make sure no other APIs are being removed (and we should probably add some test cases here, which I can help with)
    • I am OK to let the mode fix come in as part of this PR (just needs a test case) - support configurable shadowrootmode attribute for <template> tags #65
    • Sanity test in Greenwood (I will help with this)
  3. Fully enable element properties (@briangrider ) - Issue Setting Element Properties #170
  4. Require intent of setting serializable: true (@thescientist13 ) - DOM Shim is automatically inferring serializable option as true when calling attachShadow #179
    • please feel free to reply in that issue with making a case against being spec compliant

Please let me know your thoughts and if all sounds good, I will provide the formal review feedback on each PR individually and happy to help where I can with each of them so we can start landing them and get a new WCC release out soon.

For easy reference, I've created a filter with all the issues under consideration that we would bundle into the next WCC release as 0.16.0
https://github.com/ProjectEvergreen/wcc/labels/0.16.0

@DannyMoerkerke
Copy link
Contributor Author

  • After reviewing the WCC docs, we should definitely try and maintain the "public" API of the DOM shim, which includes exposing getInnerHTML, now being replaced by getHTML.

That makes sense, let's keep it for now.

  • Probably more controversially, I am still on the fence of whether to assume any of these APIs should have their serialization options set to true by default. I understand the convenience, but given how much WCC embraces / encourages writing to standards, flipping all these bits "silently" feels like it goes against the grain of everything else we document / demonstrate. I don't think it's that much more inconvenient to just have to set the flag programmatically, e.g. this.attachShadow({ serializable: true })
  • That said, I would totally be open to a compiler level option to apply this automatically so users can set that flag and WCC will assume whatever value gets passed in, e.g. renderToString(url, { serializeAllTheThings: true }) would be the same as setting serializable: true for all your custom elements

I think that's an excellent way to keep adhering to the standards while providing convenience. To me, this is the best option.

  1. Update to use getInnerHTML (@DannyMoerkerke ) - Replaced deprecated method getInnerHTML with getHTML #171

I assume you mean update to getHTML here?

  • preserve getHTML in the DOM shim / make sure no other APIs are being removed (and we should probably add some test cases here, which I can help with)

And here you mean preserve getInnerHTML?

@briangrider
Copy link

All of this sounds good to me!

Probably more controversially, I am still on the fence of whether to assume any of these APIs should have their serialization options set to true by default. I understand the convenience, but given how much WCC embraces / encourages writing to standards, flipping all these bits "silently" feels like it goes against the grain of everything else we document / demonstrate. I don't think it's that much more inconvenient to just have to set the flag programmatically, e.g. this.attachShadow({ serializable: true })
That said, I would totally be open to a compiler level option to apply this automatically so users can set that flag and WCC will assume whatever value gets passed in, e.g. renderToString(url, { serializeAllTheThings: true }) would be the same as setting serializable: true for all your custom elements

Sure, we can leave the default as serializable: false. There's no need for a compiler option here because the refactoring of WCC doesn't use getHTML. The content is serialized directly with Parse5 (similar to how a serializable flag isn't necessary for the DOM to render declarative shadow doms.) getHTML would just be a part of the DOM shim/API that people can use for situations like the one you mentioned here: https://merry-caramel-524e61.netlify.app/examples/#serverless-and-edge-functions

In that situation, yes, the greeting component will need to set serializable: true when attaching the shadow.

However, renderToString or renderFromHTML won't need the serializable flag set for components because again, we'd be rendering the DOM to a string just like the browser does (not using getHTML). I think everyone wins here, WCC will render components as the browser does (without requiring serializable: true on every component) and getHTML will be spec compliant.

Please let me know your thoughts and if all sounds good, I will provide the formal review feedback on each PR individually and happy to help where I can with each of them so we can start landing them and get a new WCC release out soon.

For easy reference, I've created a filter with all the issues under consideration that we would bundle into the next WCC release as 0.16.0

I think the timeline sounds good. Let me know as soon as you're ready for me to update the new DOM shim to work with getHTML and I'll get it done!

@thescientist13
Copy link
Member

Update to use getInnerHTML (@DannyMoerkerke ) - #171

I assume you mean update to getHTML here?

Indeed! 😄 (updated my comment)

preserve getHTML in the DOM shim / make sure no other APIs are being removed (and we should probably add some test cases here, which I can help with)

And here you mean preserve getInnerHTML?

It would be getHTML in this case as to my understanding as part of @briangrider 's refactor, there would actually not be any need to expose that method anymore specifically, but I would still like it to be there in some capacity, and am OK with the breaking name change from getInnerHTML -> getHTML.


In that situation, yes, the greeting component will need to set serializable: true when attaching the shadow.

I see, in that case if you (the user of WCC) are using getHTML (through the DOM Shim), then that is when the serializable setting would come into play, got it. Yeah, reading the MDN docs (again 😅 ), it seems that flag is only intended for usage with getHTML and not really for anything else.


OK, let me start leaving comments into each PR with the items we've covered here and let me know where I can help out. I'll spin up a Greenwood branch now to start aggregating / patching in all these changes as well go.

I think maybe one thing I can do is add an example of a shadowRoot.innerHTML example to compliment our example(s) using document.createElement('template') just so we can make sure all behaviors / expectations are covered in the docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0.16.0 breaking documentation Improvements or additions to documentation enhancement Improvement to existing functionality
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants