Chore/scoped registries improvements

.changeset/empty-students-grow.md

@@ -0,0 +1,5 @@
+---
+'@lion/ui': patch
+---
+
+no registration of same class twice w/o scoped-registries polyfill

package.json

@@ -9,6 +9,7 @@
     "custom-elements-manifest": "npm run custom-elements-manifest --workspaces --if-present && node ./scripts/create-api-tables.mjs",
     "debug": "web-test-runner --watch --config web-test-runner-chrome.config.mjs",
     "debug:firefox": "web-test-runner --watch --config web-test-runner-firefox.config.mjs",
+    "debug:no-scoped-registries-polyfill": "npm run debug -- --no-scoped-registries-polyfill",
     "debug:webkit": "web-test-runner --watch --config web-test-runner-webkit.config.mjs",
     "format": "npm run format:eslint && npm run format:prettier",
     "format:eslint": "eslint --ext .js,.html . --fix",

packages/ui/components/core/src/ScopedElementsMixin.js

@@ -89,8 +89,8 @@ const ScopedElementsMixinImplementation = superclass =>
      */
     defineScopedElement(tagName, classToBeRegistered) {
       const registeredClass = this.registry.get(tagName);
-      const isAlreadyRegistered = registeredClass && registeredClass === classToBeRegistered;
-      if (isAlreadyRegistered && !supportsScopedRegistry()) {
+      const isNewClassWithSameName = registeredClass && registeredClass !== classToBeRegistered;
+      if (!supportsScopedRegistry() && isNewClassWithSameName) {
         // eslint-disable-next-line no-console
         console.error(
           [

packages/ui/components/core/test/ScopedElementsMixin.test.js

@@ -6,9 +6,9 @@ import {
 } from '@lit-labs/testing/fixtures.js';
 import { LitElement, html } from 'lit';
 import sinon from 'sinon';
-import { browserDetection } from '../src/browserDetection.js';
 
 import { ScopedElementsMixin, supportsScopedRegistry } from '../src/ScopedElementsMixin.js';
+import { browserDetection } from '../src/browserDetection.js';
 
 const hasRealScopedRegistrySupport = supportsScopedRegistry();
 const originalShadowRootProps = {
@@ -78,6 +78,8 @@ describe('ScopedElementsMixin', () => {
 
   describe('When scoped registries are supported', () => {
     it('registers elements on local registry', async () => {
+      if (!hasRealScopedRegistrySupport) return;
+
       const ceDefineSpy = sinon.spy(customElements, 'define');
 
       const el = /** @type {ScopedElementsHost} */ (
@@ -127,5 +129,36 @@ describe('ScopedElementsMixin', () => {
       expect(ceDefineSpy.calledWith('scoped-elements-child-no-reg')).to.be.true;
       ceDefineSpy.restore();
     });
+
+    it('fails when different classes are registered under different name', async () => {
+      class ScopedElementsHostNoReg2 extends ScopedElementsMixin(LitElement) {
+        static scopedElements = { 'scoped-elements-child-no-reg': class extends HTMLElement {} };
+
+        render() {
+          return html`<scoped-elements-child-no-reg></scoped-elements-child-no-reg>`;
+        }
+      }
+      customElements.define('scoped-elements-host-no-reg-2', ScopedElementsHostNoReg2);
+
+      const errorSpy = sinon.spy(console, 'error');
+      /** @type {ScopedElementsHostNoReg2} */ (
+        await fixture(html`<scoped-elements-host-no-reg></scoped-elements-host-no-reg>`)
+      );
+      /** @type {ScopedElementsHostNoReg2} */ (
+        await fixture(html`<scoped-elements-host-no-reg-2></scoped-elements-host-no-reg-2>`)
+      );
+
+      expect(errorSpy.args[0][0]).to.equal(
+        [
+          'You are trying to re-register the "scoped-elements-child-no-reg" custom element with a different class via ScopedElementsMixin.',
+          'This is only possible with a CustomElementRegistry.',
+          'Your browser does not support this feature so you will need to load a polyfill for it.',
+          'Load "@webcomponents/scoped-custom-element-registry" before you register ANY web component to the global customElements registry.',
+          'e.g. add "<script src="/node_modules/@webcomponents/scoped-custom-element-registry/scoped-custom-element-registry.min.js"></script>" as your first script tag.',
+          'For more details you can visit https://open-wc.org/docs/development/scoped-elements/',
+        ].join('\n'),
+      );
+      errorSpy.restore();
+    });
   });
 });

packages/ui/components/core/test/SlotMixin.test.js

@@ -1,31 +1,14 @@
-import sinon from 'sinon';
 import { defineCE, expect, fixture, fixtureSync, unsafeStatic, html } from '@open-wc/testing';
-import { ScopedElementsMixin } from '@open-wc/scoped-elements/lit-element.js';
 import { SlotMixin } from '@lion/ui/core.js';
 import { LitElement } from 'lit';
+import sinon from 'sinon';
+
+import { ScopedElementsMixin, supportsScopedRegistry } from '../src/ScopedElementsMixin.js';
 
 /**
  * @typedef {import('../types/SlotMixinTypes.js').SlotHost} SlotHost
  */
 
-// @ts-ignore
-const createElementNative = ShadowRoot.prototype.createElement;
-function mockScopedRegistry() {
-  const outputObj = { createElementCallCount: 0 };
-  // @ts-expect-error wait for browser support
-  ShadowRoot.prototype.createElement = (tagName, options) => {
-    outputObj.createElementCallCount += 1;
-    // Return an element that lit can use as render target
-    return createElementNative(tagName, options);
-  };
-  return outputObj;
-}
-
-function unMockScopedRegistry() {
-  // @ts-expect-error wait for browser support
-  ShadowRoot.prototype.createElement = createElementNative;
-}
-
 describe('SlotMixin', () => {
   it('inserts provided element into light dom and sets slot', async () => {
     const tag = defineCE(
@@ -568,8 +551,11 @@ describe('SlotMixin', () => {
   });
 
   describe('Scoped Registries', () => {
-    it('supports scoped elements when polyfill loaded', async () => {
-      const outputObj = mockScopedRegistry();
+    it('supports scoped elements when scoped registries supported (or polyfill loaded)', async () => {
+      if (!supportsScopedRegistry()) return;
+
+      // @ts-expect-error
+      const createElSpy = sinon.spy(ShadowRoot.prototype, 'createElement');
 
       class ScopedEl extends LitElement {}
 
@@ -600,14 +586,14 @@ describe('SlotMixin', () => {
       const tag = unsafeStatic(tagName);
       await fixture(html`<${tag}></${tag}>`);
 
-      expect(outputObj.createElementCallCount).to.equal(1);
+      expect(createElSpy.callCount).to.equal(1);
 
-      unMockScopedRegistry();
+      createElSpy.restore();
     });
 
-    it('does not scope elements when polyfill not loaded', async () => {
-      // @ts-expect-error
-      ShadowRoot.prototype.createElement = null;
+    it('does not scope elements when scoped registries not supported (or polyfill not loaded)', async () => {
+      if (supportsScopedRegistry()) return;
+
       class ScopedEl extends LitElement {}
 
       const tagName = defineCE(
@@ -645,7 +631,6 @@ describe('SlotMixin', () => {
 
       document.body.removeChild(renderTarget);
       docSpy.restore();
-      unMockScopedRegistry();
     });
   });
 });

packages/ui/components/form-core/test-suites/ValidateMixin.suite.js

@@ -2,25 +2,25 @@ import { LitElement } from 'lit';
 import { aTimeout, defineCE, expect, fixture, html, unsafeStatic } from '@open-wc/testing';
 import {
   getFormControlMembers,
-  AlwaysInvalid,
-  AlwaysValid,
   AsyncAlwaysInvalid,
   AsyncAlwaysValid,
+  AlwaysInvalid,
+  AlwaysValid,
 } from '@lion/ui/form-core-test-helpers.js';
 import sinon from 'sinon';
 import {
+  ResultValidator,
+  ValidateMixin,
   EqualsLength,
+  Unparseable,
   MaxLength,
   MinLength,
-  Required,
-  ResultValidator,
-  Unparseable,
-  ValidateMixin,
   Validator,
+  Required,
 } from '@lion/ui/form-core.js';
 
-import '@lion/ui/define/lion-field.js';
 import '@lion/ui/define/lion-validation-feedback.js';
+import '@lion/ui/define/lion-field.js';
 
 /**
  * @typedef {import('@lion/ui/form-core.js').LionField} LionField

web-test-runner.config.mjs

@@ -1,30 +1,22 @@
-import fs from 'fs';
-import { playwrightLauncher } from '@web/test-runner-playwright';
 import { litSsrPlugin } from '@lit-labs/testing/web-test-runner-ssr-plugin.js';
+import { playwrightLauncher } from '@web/test-runner-playwright';
+import { optimisedGlob } from 'providence-analytics/utils.js';
 
-const devMode = process.argv.includes('--dev-mode');
+const config = {
+  shouldLoadPolyfill: !process.argv.includes('--no-scoped-registries-polyfill'),
+  shouldRunDevMode: process.argv.includes('--dev-mode'),
+};
 
-const packages = fs
-  .readdirSync('packages')
-  .filter(
-    dir => fs.statSync(`packages/${dir}`).isDirectory() && fs.existsSync(`packages/${dir}/test`),
-  )
-  .map(dir => ({ name: dir, path: `packages/${dir}/test` }))
-  .concat(
-    fs
-      .readdirSync('packages/ui/components')
-      .filter(
-        dir =>
-          fs.statSync(`packages/ui/components/${dir}`).isDirectory() &&
-          fs.existsSync(`packages/ui/components/${dir}/test`),
-      )
-      .map(dir => ({ name: dir, path: `packages/ui/components/${dir}/test` })),
-  );
+const groups = (
+  await optimisedGlob(['packages/*/test', 'packages/ui/components/**/test'], {
+    onlyDirectories: true,
+  })
+).map(dir => ({ name: dir.split('/').at(-2), files: `${dir}/**/*.test.js` }));
 
 /**
  * @type {import('@web/test-runner').TestRunnerConfig['testRunnerHtml']}
  */
-const testRunnerHtml = testRunnerImport =>
+const testRunnerHtmlWithPolyfill = testRunnerImport =>
   `
 <html>
   <head>
@@ -35,31 +27,21 @@ const testRunnerHtml = testRunnerImport =>
 `;
 
 export default {
-  nodeResolve: { exportConditions: [devMode && 'development'] },
+  nodeResolve: config.shouldRunDevMode ? { exportConditions: ['development'] } : true,
   coverageConfig: {
     report: true,
     reportDir: 'coverage',
-    threshold: {
-      statements: 95,
-      functions: 95,
-      branches: 95,
-      lines: 95,
-    },
+    threshold: { statements: 95, functions: 95, branches: 95, lines: 95 },
   },
   testFramework: {
-    config: {
-      timeout: '5000',
-    },
+    config: { timeout: '5000' },
   },
-  testRunnerHtml,
+  testRunnerHtml: config.shouldLoadPolyfill ? testRunnerHtmlWithPolyfill : undefined,
   browsers: [
     playwrightLauncher({ product: 'firefox', concurrency: 1 }),
     playwrightLauncher({ product: 'chromium' }),
     playwrightLauncher({ product: 'webkit' }),
   ],
-  groups: packages.map(pkg => ({
-    name: pkg.name,
-    files: `${pkg.path}/**/*.test.js`,
-  })),
+  groups,
   plugins: [litSsrPlugin()],
 };