[scoped-custom-element-registry] throws error trying to define a CE extending the native HTMLElement

[manolakis]

Description

The polyfill throws an Error indicating that there is an illegal constructor if we try to define a CE which class has been created before applying the polyfill, so it's using the native HTMLElement function instead of the patched one.

Steps to reproduce

class CustomElementClass extends HTMLElement {}

await import('scoped-custom-element-registry.min.js');

customElements.define('my-example', CustomElementClass); // throws Error

Expected behavior

Define the component.

Actual behavior

Throws Error

[guillemcordoba]

Just bumped into this as well, for now including the polyfill before defining anything should work. Thanks @manolakis

[kevinpschaaf]

After some offline discussions about this I think we feel pretty strongly that polyfills should be loaded before application code, and we shouldn't add extra code to enable lazily bring polyfills in as implementation detail. There are lots of places in polyfills we could enable this sort of just-in-time loading at the cost of a lot of complexity.

That said it seems like this could pretty trivially be handled in userspace by e.g. extending ScopedRegistryMixin to do the prototype patch, for example:

import {
  ScopedRegistryHost as OrigScopedRegistryHost,
  ElementDefinitionsMap
} from '@lit-labs/scoped-registry-mixin';

// eslint-disable-next-line @typescript-eslint/no-explicit-any
type LitElementConstructor = new (...args: any[]) => LitElement;

export function ScopedRegistryHost<SuperClass extends LitElementConstructor>(
  superclass: SuperClass
): SuperClass {
  return class LazyScopedRegistryHost extends OrigScopedRegistryHost(superclass) {
    static elementDefinitions?: ElementDefinitionsMap;
    createRenderRoot() {
      Object.values(constructor.elementDefinitions).forEach((klass) => patchHTMLElement(klass));
      return super.createRenderRoot();
    }
}

const NativeHTMLElement = Object.getPrototypeOf(HTMLDivElement);

const patchHTMLElement = (elementClass) => {
  const parentClass = Object.getPrototypeOf(elementClass);
  if (parentClass !== window.HTMLElement) {
    if (parentClass === NativeHTMLElement) {
      return Object.setPrototypeOf(elementClass, window.HTMLElement);
    }
    return patchHTMLElement(parentClass);
  }
};

[manolakis]

But wouldn't that couple the mixin to the polyfill?

The only reason to patch the component hierarchy is to make them work with that polyfill implementation applying the constructor hack that the polyfill needs to instantiate them... and the polyfill should be transparent to the mixin, which should continue working when the browser fulfills the scoped registries 🤔

[kevinpschaaf]

But wouldn't that couple the mixin to the polyfill?

The extension to the mixin above would be coupled to the polyfill yes, but the only reason this is needed is because you want to import the polyfill as a module at the point of usage; so the same file you'd import the polyfill into you'd import the polyfill-aware mixin (and when the polyfill is no longer needed to be imported, the mixin could be switched back to the standard mixin, again in the same file). I think the only thing needed would be to make sure the above code bails on the patching if the polyfill is not in use; something like NativeHTMLElement !== window.HTMLElement might be a good test for that.

[manolakis]

Makes sense. Thanks!