enhance-dev / enhance-ssr

Server side render for custom elements.
140 stars 9 forks source link

Template with a default slot and other elements can cause extra elements to render #5

Closed ryanbethel closed 2 years ago

ryanbethel commented 2 years ago

Because Enhance copies all the elements inside a template into the markup for SSR if there is a slot and other elements in a template the extra elements not in the slot end up being rendered twice.

For example if you render <my-input><span>in the slot</span></my-input> with the following template:

module.exports = function MyInputTemplate(state = {}, html) {
  return html`
    <h1>Title</h1>
    <slot>Default Text</slot>
    <script type="module">
      class MyInput extends HTMLElement {
        constructor() {
          super()
          const template = document.getElementById('my-input-template')
          this.attachShadow({ mode: 'open' }).appendChild(
            template.content.cloneNode(true),
          )
        }
      }

      customElements.define('my-input', MyInput)
    </script>
  `
}

You will get the following markup sent to the browser:

<my-input>
  <h1>Title</h1>
  <span>text in the slot</span>
</my-input>

So that without javascript it will render immediatly as you expect. But then when the custom element takes over it upgrades my-input> but now it has two children instead of the original one so both of them get slotted. The rendered output ends up looking like the following:

Title

Title

text in the slot


This is not really a bug in my mind because it is a direct result of how Enhance does SSR rendering and it can't be easily avoided without adding indirection. But it can cause some unexpected behavior that needs to be clearly documented. I will link this to a discussion of how I think it could be fixed or documented (https://github.com/enhance-dev/enhance/discussions/14)

kristoferjoseph commented 2 years ago

This is a hard one since it requires the accompanying Web Component to be initiated in the browser to see. I will add tests for this case and we can figure out how we want to handle it. Feels like we will want to give guidance on how to use unnamed slots or just name all slots.

kristoferjoseph commented 2 years ago

I've captured what it would take to handle these cases while respecting user intent instead of what we currently do which is respect the user authored template. https://github.com/enhance-dev/enhance/discussions/14#discussioncomment-2100049

kristoferjoseph commented 2 years ago

OK this is fixed in v2.1.0 Per the spec there is no default content rendered in an unnamed slot. For default content one must use a named slot.