11ty / is-land

A new performance-focused way to add interactive client-side components to your web site.
https://is-land.11ty.dev/
MIT License
528 stars 17 forks source link

Declarative Shadow DOM support #13

Closed e111077 closed 1 year ago

e111077 commented 1 year ago

When using is-land and an SSR'd web component, is-land causes it to flicker because forceFallback will remove the SSR'd component from the page and then append it once hydrated.

The workaround i've gotten here was simply to subclass is-land, and override forceFallback to not do anything.

I can't really tell what forceFallback is for, would appreciate any context here.

Repro

site/index.html

<script type="module" src="/js/lit-hydrate-support.js"></script>
<is-land on:idle import="/js/my-counter.js">
  <my-counter></my-counter>
</is-land>

11ty.cjs

const litPlugin = require('@lit-labs/eleventy-plugin-lit');

module.exports = function (eleventyConfig) {
  // copy folders to the 11ty output folder
  eleventyConfig
    .addPassthroughCopy({ [`lib/`]: 'js/' })

  // add the lit-ssr plugin
  eleventyConfig.addPlugin(litPlugin, {
    mode: 'worker',
    componentModules: [`./lib/my-counter.js`],
  });

  return {
    htmlTemplateEngine: 'njk',
    dir: {
      input: 'site',
      output: '_prod',
    },
  };
};

lib/my-counter.js

import {LitElement, html} from 'lit';
export class MyCounter extends LitElement {
  static properties = {counter: {type: Number}};
  constructor() {
    this.count = 0;
  }
  render() {
    return html`
      <button @click=${() => this.count++}>Increment</button>
      <div>Count: ${this.count}</div>
    `;
  }
};
customElements.define('my-counter', MyCounter);

lib/lit-hydrate-suopport.js

import 'lit/experimental-hydrate-support.js';
zachleat commented 1 year ago

just as a tip I believe you can noop the API via window.Island.fallback[":not(:defined)"] = function() {}; to avoid monkeypatching!

At what point is my-counter.js defined in the component registry?

The purpose of forceFallback is to avoid defining components that have already been defined in the registry.

You can see this in play on the is-land demos here: https://github.com/11ty/is-land/blob/01e55ae6b7f5635d3c0fd16825e37239d87cc542/_includes/layout.html#L33-L38

I hear what you’re saying though—if you can guarantee that the component won’t be defined ahead of time, it would be faster to bypass this step!

e111077 commented 1 year ago

Ah sorry I forgot to include the CE.define call in my issue.

But it's SSRd on the page and defined at the async import initiated by the is-land. The patch of :not(:defined) components means that the SSRd component is removed from the page and then reattached once the definition loads causing a flicker before hydration

zachleat commented 1 year ago

Ah, right on—now it makes sense. Yeah I agree that we should make a better change here.

One question though: what happens when there are two island instances?

<is-land on:interaction import="/js/my-counter.js">
  <my-counter>Fallback content</my-counter>
</is-land>
<is-land on:interaction import="/js/my-counter.js">
  <my-counter>Fallback content</my-counter>
</is-land>

Rather, more accurately, I want the second is-land > my-counter to not initialize when the first is-land is initialized (and I still want Fallback content to show pre-init). Does that make sense? This is the problem forceFallback solves.

e111077 commented 1 year ago

That's an interesting use case! Perhaps scoped element registries in the future can be the real answer here. It's sort of great that the CE registry can upgrade all the elements at once and I'd argue that doing less magic here might be the way here because it's the interaction model of custom elements. Additionally, one might consider SSR'd declarative shadow DOM content as fallback content.

But i can see that if startup is particularly painful it can cause some perf issues here. Perhaps this can be a flag on is-land? <is-land global-ce-hydration> though it'd be not particularly great to have to do something special for CEs.

... Or perhaps you can simply change the tagname of the element rather than replacing the node? In this case you can keep the hydrated shadow DOM on the page but prevent hydration across the page?

zachleat commented 1 year ago

@e111077 is there an easy way to rename an element? That would be ideal! .tagName and .nodeName seem to be readonly and I don’t see any DOM method for renaming

e111077 commented 1 year ago

Oh uh no. Idk what I was thinking tbh. There's outerhtml but that's a bad answer.

The answer here in the future would be finagling with scoped element registries. Otherwise you can use a pivot constructor maybe like the polyfills for scoped element registries do, but that might cause issues down the line and also be much more complexity than we might be asking for.

zachleat commented 1 year ago

Well, I spent a lot of time experimenting on this one and went down a lot of unproductive roads 😅

Primarily I played with a global registry of external elements that required an element rename (only after another island using that element was hydrated) but the performance was pretty bad.

I think unfortunately I’m going to settle on an ssr attribute that opts-out of the rename behavior. It’s the lightest and speediest way forward here for now. Open to better names there if you have opinions!

zachleat commented 1 year ago

This will require authors to take extra care when using lit with this ssr attribute though (specifically if two custom elements in different islands share the same name), per https://github.com/11ty/is-land/issues/13#issuecomment-1311037005

zachleat commented 1 year ago

Maybe global would be a better attribute name? hmm

e111077 commented 1 year ago

Bummer, but yeah the flag makes sense here I think until we get something better in the platform. Thanks for the consideration!

e111077 commented 1 year ago

<is-land global on:idle import="..."> makes it sound like it'll do something to upgrade all the islands which is true for CE, but not necessarily react, vue, etc, right?

Maybe something like:

<is-land global-content on:idle import="..."> or <is-land global-ce-hydrate on:idle import="...">? Or something that hints that it's dependent on the content?

...or maybe I'm just reading into this too much. Regardless global is a better name that ssr IMO

zachleat commented 1 year ago

I took a step back and I think I settled on a much better solution here.

Rather than try to workaround the issue by avoiding the rename, I think we can solve both problems here by simply cloning the shadow root into the placeholder element. I went ahead and included the Declarative Shadow DOM polyfill too so that the fallback content matches in Safari and Firefox.

Check out this test case https://is-land.11ty.dev/issues/13/

This alleviates the need to use a new attribute (no ssr or global) and no more worrying about global registration/island leakage.

Does this work better for your example?

e111077 commented 1 year ago

Looks pretty great!

Including the DSD polyfill might become dead weight after some time once it's native across browser or esp if someone is including it already because they're using CEs + DSD.

It shouldn't break anything though AFAICT

zachleat commented 1 year ago

I did want to highlight this callout to defer-hydration from @dgp1130 https://techhub.social/@develwithoutacause/109332565955255126

https://github.com/webcomponents-cg/community-protocols/pull/15

…which will likely provide a better way forward and allow us to move away from renaming elements altogether! 🚀

e111077 commented 1 year ago

Bringing up this issue again because i'm running into an interesting use case where rename is causing issues where skipping fallback might still help:

I have 2 components, menu and button. I want menu to hydrate based solely on the user interacting with button, but button is not SSRd but already defined. e.g.:

<is-land on:interaction="pointerenter,pointerdown,focusin" import="menu.js">
  <md-icon-button>menu</md-icon-button>
  <md-menu>
    ...
  </md-menu>
</is-land>

This is resulting in:

<is-land on:interaction="">
  <is-land--is-land--is-land--is-land--md-icon-button>
    palette
  </is-land--is-land--is-land--is-land--md-icon-button>
  <md-menu>
    ...
  </md-menu>
</is-land>

This means that md-icon-button cannot upgrade because the tag names are different despite wanting only to use it as the trigger for hydration of menu.

Additionally overriding the static properties won't work here. e.g.

import {Island} from '@11ty/island';

Island.prefix = '';

In this example, the first line will trigger the upgrade of all the <is-land> elements on the page before the prefix can be set