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

Declarative Shadow DOM support #13

Closed
e111077 opened this issue Nov 8, 2022 · 15 comments
Closed

Declarative Shadow DOM support #13

e111077 opened this issue Nov 8, 2022 · 15 comments
Assignees
Labels
bug Something isn't working

Comments

@e111077
Copy link

e111077 commented Nov 8, 2022

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
Copy link
Member

zachleat commented Nov 9, 2022

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:

<!--
You can directly import dependencies (for eager loading)
or for lazy loaded islands put dependencies into `import="./lib/vanilla-web-component.js"`
Look inside this file for a commented-out example of using this programmatically in your component code
-->
<script type="module" src="/lib/vanilla-web-component.js"></script>

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!

@zachleat zachleat self-assigned this Nov 9, 2022
@zachleat zachleat added the bug Something isn't working label Nov 9, 2022
@e111077
Copy link
Author

e111077 commented Nov 9, 2022

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
Copy link
Member

zachleat commented Nov 10, 2022

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
Copy link
Author

e111077 commented Nov 11, 2022

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
Copy link
Member

zachleat commented Nov 11, 2022

@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
Copy link
Author

e111077 commented Nov 11, 2022

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
Copy link
Member

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
Copy link
Member

zachleat commented Nov 11, 2022

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 #13 (comment)

@zachleat zachleat added this to the <is-land> Next Version milestone Nov 11, 2022
@zachleat
Copy link
Member

Maybe global would be a better attribute name? hmm

@zachleat zachleat reopened this Nov 11, 2022
@e111077
Copy link
Author

e111077 commented Nov 11, 2022

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

@e111077
Copy link
Author

e111077 commented Nov 12, 2022

<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
Copy link
Member

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?

zachleat added a commit that referenced this issue Nov 12, 2022
@e111077
Copy link
Author

e111077 commented Nov 12, 2022

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
Copy link
Member

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

webcomponents-cg/community-protocols#15

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

@zachleat zachleat changed the title Declarative Shadow DOM / lit-ssr + is-land flicker Declarative Shadow DOM support Nov 14, 2022
@e111077
Copy link
Author

e111077 commented Apr 21, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants