From 0aa00d7b47b458853f771c78189784fb605eb45e Mon Sep 17 00:00:00 2001 From: Steve Orvell Date: Sat, 10 Oct 2026 15:34:01 -0700 Subject: [PATCH 01/11] Review fixes: new X() registry, importNode options, attachInternals, registry checks, clearer getRegistry --- .../src/scoped-custom-element-registry.ts | 217 +++++++++++------- .../test/behavior.test.js | 63 +++++ 2 files changed, 197 insertions(+), 83 deletions(-) diff --git a/packages/scoped-custom-element-registry/src/scoped-custom-element-registry.ts b/packages/scoped-custom-element-registry/src/scoped-custom-element-registry.ts index 76b9f88c..d885f7d4 100644 --- a/packages/scoped-custom-element-registry/src/scoped-custom-element-registry.ts +++ b/packages/scoped-custom-element-registry/src/scoped-custom-element-registry.ts @@ -105,10 +105,13 @@ const detectNativeSupport = () => { force || !hasCustomElementRegistry || !hasNullCustomElementRegistry, }; }; -Object.assign( - polyfillWindow['CustomElementRegistryPolyfill'], - detectNativeSupport() -); +// Note, a second load would run detection against the polyfill itself. +if (!polyfillWindow['CustomElementRegistryPolyfill']['loaded']) { + Object.assign( + polyfillWindow['CustomElementRegistryPolyfill'], + detectNativeSupport() + ); +} // Note, `??=` so a second load does not capture the shimmed registry. polyfillWindow['CustomElementRegistryPolyfill']['nativeRegistry'] ??= @@ -396,6 +399,16 @@ type ParametersOf< // Set only while constructing the global registry so it is not scoped. let isCreatingGlobalRegistry = false; + // Every registry the polyfill created, so a registry from elsewhere (e.g. a + // window without the polyfill, or a user's object) is never used as one. + const polyfillRegistries = shared( + 'polyfillRegistries', + () => new WeakSet() + ); + const isPolyfillRegistry = ( + value: unknown + ): value is ShimmedCustomElementsRegistry => + polyfillRegistries.has(value as object); // https://html.spec.whatwg.org/#valid-custom-element-name const reservedCustomElementNames = new Set([ @@ -414,10 +427,15 @@ type ParametersOf< !reservedCustomElementNames.has(name); // Whether an HTML element named `name` would be an HTMLUnknownElement. + const nativeCreateElementNS = Document.prototype.createElementNS as ( + namespace: string, + name: string + ) => Element; const isUnknownElementName = (name: string) => { try { return ( - document.createElementNS( + nativeCreateElementNS.call( + document, 'http://www.w3.org/1999/xhtml', name ) instanceof HTMLUnknownElement @@ -562,13 +580,19 @@ type ParametersOf< } } + constructor() { + polyfillRegistries.add(this); + } + // Note, defining can upgrade existing elements, so it's an entry point. define( tagName: string, elementClass: CustomElementConstructor, options?: ElementDefinitionOptions ) { - withDeferredUpgrades(() => this._define(tagName, elementClass, options)); + withDeferredUpgrades(() => + this._define(String(tagName), elementClass, options) + ); } _define( @@ -739,7 +763,7 @@ type ParametersOf< } get(tagName: string) { - const definition = this._definitionsByTag.get(tagName); + const definition = this._definitionsByTag.get(String(tagName)); return definition?.elementClass; } @@ -752,7 +776,13 @@ type ParametersOf< return this._definitionsByTag.get(tagName); } - ['whenDefined'](tagName: string) { + ['whenDefined'](name: string) { + let tagName: string; + try { + tagName = String(name); + } catch (e) { + return Promise.reject(e); + } if (!isValidCustomElementName(tagName)) { return Promise.reject( new DOMException( @@ -920,6 +950,8 @@ type ParametersOf< ); Object.setPrototypeOf(instance, this.constructor.prototype); definitionForElement.set(instance!, definition); + // Note, as natively, a constructed element has the global registry. + registryForNode.set(instance, globalCustomElementRegistry); // Note, the browser didn't construct a stand-in, so this element is // counted here if it may be left out of its form (see `isFormControl`). if (definition.standInFormAssociated && !definition['formAssociated']) { @@ -966,7 +998,13 @@ type ParametersOf< Object.setPrototypeOf(instance, HTMLElement.prototype); if (nativeAttachInternals) { const internals = nativeAttachInternals.call(instance); - statesOf(internals)?.add(UNDEFINED_STATE); + // Note, browsers that required a `--` prefix (e.g. Chrome before + // 125) throw for this name; the state is then simply unavailable. + try { + statesOf(internals)?.add(UNDEFINED_STATE); + } catch { + // squelch + } internalsForElement.set(instance, internals); } // Note, the element is only queued. Whatever caused its construction @@ -1545,6 +1583,12 @@ type ParametersOf< doc: Document, method: string ) => { + // Note, as WebIDL converts the argument. + if (registry != null && !isPolyfillRegistry(registry)) { + throw new TypeError( + `Failed to execute '${method}': customElementRegistry is not a CustomElementRegistry` + ); + } if ( registry && !(registry as ShimmedCustomElementsRegistry)._isScoped && @@ -1650,70 +1694,78 @@ type ParametersOf< }); }; - // Registry for a node not tracked in `registryForNode`. - const getDefaultRegistry = ( - node: Node - ): ShimmedCustomElementsRegistry | null => { - // Note, native registry state is deliberately not consulted, so the - // polyfill behaves the same whether or not the browser has support. - const ownerDoc = (node.nodeType === Node.DOCUMENT_NODE - ? node - : node.ownerDocument) as Document; - return (ownerDoc?.defaultView?.customElements || - null) as ShimmedCustomElementsRegistry | null; + // The global registry of a document's window, or null if it has none (or + // the window doesn't run the polyfill). + // Note, native registry state is deliberately not consulted, so the + // polyfill behaves the same whether or not the browser has support. + const getWindowRegistry = (doc: Document) => { + const registry = doc.defaultView?.customElements; + return isPolyfillRegistry(registry) ? registry : null; }; - // The registry of a node. The polyfill records the registry of every node - // it creates, so a node without a record was made by a parser, or is in a - // copied shadow root the polyfill couldn't reach. Its registry is worked out - // from where it is, and recorded, so later moves don't change it: - // - an element with a null registry attribute: null; - // - a shadow root: the registry kept for it if it's such a copy (see - // `pairUnreachableCopyRoots`); otherwise it's declarative, so null if its - // host has the polyfill's declarative attribute, else the document's; - // - anything else: its parent's. - // Note, a node with no parent isn't recorded: the parser constructs a custom - // element before inserting it. + // The registry of a node. + // + // Every API the polyfill patches records the registry of each node it + // creates, when it creates it. So a node without a record was made by a + // parser (the main parser, or a parsing API before its wrapper records the + // new nodes), and its registry follows the parser's rules, worked out from + // where it is (see `getParsedRegistry`). It's then recorded, so later + // moves or attribute changes don't change it. const getRegistry = (node: Node): ShimmedCustomElementsRegistry | null => { const recorded = registryForNode.get(node); if (recorded !== undefined) { return recorded; } if (node.nodeType === Node.DOCUMENT_NODE) { - return getDefaultRegistry(node); - } - let registry: ShimmedCustomElementsRegistry | null; - if (isShadowRoot(node)) { - const {host} = node; - shadowRootForHost.set(host, node); - registry = clonedShadowRootRegistries.has(host) - ? clonedShadowRootRegistries.get(host)! - : host.hasAttribute(DSD_HOST_ATTRIBUTE) - ? null - : getRegistry(host.ownerDocument); - } else if ((node as Element).matches?.(NULL_REGISTRY_SELECTOR)) { - registry = null; - } else { - const parent = node.parentNode; - if ( - parent === null || + return getWindowRegistry(node as Document); + } + const parent = node.parentNode; + const isRoot = isShadowRoot(node); + // Note, a parsed node is constructed before it's inserted, so one without + // a parent isn't recorded yet. One in a plain fragment (e.g. template + // contents) gets its window's registry, which for template contents is + // null. + if ( + !isRoot && + (parent === null || (parent.nodeType === Node.DOCUMENT_FRAGMENT_NODE && - !isShadowRoot(parent)) - ) { - return getDefaultRegistry(node); - } - registry = getRegistry(parent); - // Note, working out the parent's registry can pair a copied shadow root, - // which records this node too. - const pairedRegistry = registryForNode.get(node); - if (pairedRegistry !== undefined) { - return pairedRegistry; - } + !isShadowRoot(parent))) + ) { + return getWindowRegistry(node.ownerDocument!); } + const registry = isRoot + ? getShadowRootRegistry(node as ShadowRoot) + : getParsedRegistry(node, parent!); registryForNode.set(node, registry); return registry; }; + // The parser's rules for a node: null if it's an element with a null + // registry attribute, else its parent's registry. + // Note, the attribute only matters while parsing; it's checked here since + // a node without a record was made by a parser (see `getRegistry`). + const getParsedRegistry = (node: Node, parent: Node) => + (node as Element).matches?.(NULL_REGISTRY_SELECTOR) + ? null + : getRegistry(parent); + + // A shadow root without a record is either declarative (the parser + // attached it) or a cloned closed root the polyfill couldn't reach when + // cloning (see `pairUnreachableCopyRoots`): + // - a cloned root: the registry kept for it; + // - a declarative root whose host has the polyfill's attribute: null; + // - another declarative root: its document's registry. + const getShadowRootRegistry = (root: ShadowRoot) => { + const {host} = root; + shadowRootForHost.set(host, root); + if (clonedShadowRootRegistries.has(host)) { + return clonedShadowRootRegistries.get(host)!; + } + return host.hasAttribute(DSD_HOST_ATTRIBUTE) + ? null + : getRegistry(host.ownerDocument); + }; + // Records the registries of `node` and every element under it, including in // open shadow roots (see `getRegistry`), so that moving them later doesn't // change them. @@ -1845,14 +1897,18 @@ type ParametersOf< node: T, options?: boolean | ImportNodeOptions ): T { - // Note, as natively, the default is a shallow copy. - const deep = - typeof options === 'boolean' - ? options - : options !== undefined && !options['selfOnly']; - const optionsRegistry = ((options ?? {}) as ImportNodeOptions)[ - 'customElementRegistry' - ]; + // Note, as WebIDL converts `(boolean or ImportNodeOptions)`: omitted + // is a shallow copy, null or an object is a dictionary (deep unless + // `selfOnly`), and anything else is a boolean. + const isDictionary = + options === null || + typeof options === 'object' || + typeof options === 'function'; + const dictionary = (isDictionary + ? options ?? {} + : {}) as ImportNodeOptions; + const deep = isDictionary ? !dictionary['selfOnly'] : Boolean(options); + const optionsRegistry = dictionary['customElementRegistry']; // Note, as natively, the option isn't nullable. if (optionsRegistry === null) { throw new TypeError( @@ -2490,25 +2546,28 @@ type ParametersOf< // Note, a stand-in has already attached internals (see `UNDEFINED_STATE`), // so they are returned to the user, emulating native: internals can be - // attached once, and not if the element's class disables them. + // attached once, only to a custom element (one being or already customized, + // not failed), and not if the element's class disables them. + const internalsToHostMap = new WeakMap(); if (nativeAttachInternals) { HTMLElement.prototype['attachInternals'] = function (this: HTMLElement) { // Note, the browser only knows the stand-in class, so the user's class // is checked here, however the element was constructed. + const definition = definitionForElement.get(this); if ( - definitionForElement.get(this)?.disableInternals || - internalsAttachedByUser.has(this) + definition?.disableInternals || + internalsAttachedByUser.has(this) || + (standInElements.has(this) && (!definition || isFailed(this))) ) { throw new DOMException( - `Failed to execute 'attachInternals' on 'HTMLElement': ElementInternals for the specified element was already attached or is disabled.`, + `Failed to execute 'attachInternals' on 'HTMLElement': ElementInternals for the specified element was already attached, is disabled, or the element isn't a custom element.`, 'NotSupportedError' ); } - const internals = internalsForElement.get(this); - if (internals === undefined) { - return nativeAttachInternals.call(this); - } + const internals = + internalsForElement.get(this) ?? nativeAttachInternals.call(this); internalsAttachedByUser.add(this); + internalsToHostMap.set(internals, this); return internals; }; } @@ -2517,14 +2576,6 @@ type ParametersOf< !!window['ElementInternals'] && !!window['ElementInternals'].prototype['setFormValue'] ) { - const internalsToHostMap = new WeakMap(); - const attachInternals = HTMLElement.prototype['attachInternals']; - HTMLElement.prototype['attachInternals'] = function (...args) { - const internals = attachInternals.call(this, ...args); - internalsToHostMap.set(internals, this); - return internals; - }; - // Note, the browser only knows the stand-in, so the form members of an // element's internals are checked here: as natively, they only work for // an element customized with a form-associated class that didn't fail diff --git a/packages/scoped-custom-element-registry/test/behavior.test.js b/packages/scoped-custom-element-registry/test/behavior.test.js index 97da86c0..a1210b4a 100644 --- a/packages/scoped-custom-element-registry/test/behavior.test.js +++ b/packages/scoped-custom-element-registry/test/behavior.test.js @@ -157,6 +157,59 @@ describe('importNode', () => { expect(document.importNode(el).childNodes.length).to.equal(0); expect(document.importNode(el, {}).childNodes.length).to.equal(1); }); + + // Note, as WebIDL converts `(boolean or ImportNodeOptions)`. + it('treats null as empty options and other values as a boolean', () => { + const el = document.createElement('div'); + el.innerHTML = ''; + expect(document.importNode(el, null).childNodes.length).to.equal(1); + expect(document.importNode(el, 0).childNodes.length).to.equal(0); + expect(document.importNode(el, 1).childNodes.length).to.equal(1); + }); +}); + +describe('Registry arguments', () => { + it('a value that is not a registry throws a TypeError', () => { + const notARegistry = {}; + expect(() => + document.createElement('div', {customElementRegistry: notARegistry}) + ).to.throw(TypeError); + expect(() => + document + .createElement('div') + .attachShadow({mode: 'open', customElementRegistry: notARegistry}) + ).to.throw(TypeError); + expect(() => + document.importNode(document.createElement('div'), { + customElementRegistry: notARegistry, + }) + ).to.throw(TypeError); + }); + + it('names are converted to strings', async () => { + const registry = new CustomElementRegistry(); + const tagName = getTestTagName(); + const Logged = class extends HTMLElement {}; + const name = {toString: () => tagName}; + registry.define(name, Logged); + expect(registry.get(name)).to.equal(Logged); + expect(await registry.whenDefined(name)).to.equal(Logged); + }); +}); + +describe('Constructing with new', () => { + it('an element constructed with new keeps the global registry when moved into a scoped root', () => { + const tagName = getTestTagName(); + class X extends HTMLElement {} + customElements.define(tagName, X); + const registry = new CustomElementRegistry(); + const root = document + .createElement('div') + .attachShadow({mode: 'open', customElementRegistry: registry}); + const x = new X(); + root.append(x); + expect(x.customElementRegistry).to.equal(customElements); + }); }); describe('Registry API', () => { @@ -1530,6 +1583,16 @@ describe('Not yet customized: :state(polyfill-undefined)', () => { document.createElement(tagName); expect(error?.name).to.equal('NotSupportedError'); }); + + // Note, as natively, internals attach only to a custom element. + it("attachInternals throws for an element its registry doesn't define", () => { + const registry = new CustomElementRegistry(); + const tagName = getTestTagName(); + registry.define(tagName, class extends HTMLElement {}); + // Note, the global registry doesn't define the tag. + const el = document.createElement(tagName); + expect(() => el.attachInternals()).to.throw(DOMException); + }); }); describe('Every constructing API upgrades before it returns', () => { From 9feef67f2c6e0b47a15f456190127cad1b19fd64 Mon Sep 17 00:00:00 2001 From: Steve Orvell Date: Sat, 10 Oct 2026 15:37:22 -0700 Subject: [PATCH 02/11] Review fixes: restore registries when an insertion throws, more interception points, attribute callback order, one tree walk --- .../src/scoped-custom-element-registry.ts | 301 +++++++++++------- .../test/behavior.test.js | 42 +++ 2 files changed, 223 insertions(+), 120 deletions(-) diff --git a/packages/scoped-custom-element-registry/src/scoped-custom-element-registry.ts b/packages/scoped-custom-element-registry/src/scoped-custom-element-registry.ts index d885f7d4..32b519c2 100644 --- a/packages/scoped-custom-element-registry/src/scoped-custom-element-registry.ts +++ b/packages/scoped-custom-element-registry/src/scoped-custom-element-registry.ts @@ -356,34 +356,47 @@ type ParametersOf< const isTemplate = (node: unknown): node is HTMLTemplateElement => (node as Element | null)?.localName === 'template' && (node as Element).namespaceURI === 'http://www.w3.org/1999/xhtml'; - - // Gives `node` and every element under it with a null registry `registry` - // (see `initialize`). - const initializeSubtree = ( + // Calls `fn` for `node` and each element under it, in tree order, and if + // `shadowRoots`, also for the shadow roots the polyfill knows and what's in + // them. + const forEachInTree = ( node: Node, - registry: ShimmedCustomElementsRegistry + fn: (node: Node) => void, + shadowRoots = false ) => { - if (getRegistry(node) === null) { - registryForNode.set(node, registry); + fn(node); + const shadowRoot = shadowRoots ? getShadowRoot(node) : undefined; + if (shadowRoot) { + forEachInTree(shadowRoot, fn, true); } for (const child of childrenOf(node)) { - initializeSubtree(child, registry); + forEachInTree(child, fn, shadowRoots); } }; + // Gives `node` and every element under it with a null registry `registry` + // (see `initialize`). + const initializeSubtree = ( + node: Node, + registry: ShimmedCustomElementsRegistry + ) => + forEachInTree(node, (n) => { + if (getRegistry(n) === null) { + registryForNode.set(n, registry); + } + }); + // Upgrades each element in `node`'s subtree whose registry is `registry`, // or has it wait for a definition. const upgradeSubtree = ( node: Node, registry: ShimmedCustomElementsRegistry - ) => { - if (node.nodeType === Node.ELEMENT_NODE && getRegistry(node) === registry) { - registry._upgradeOrWait(node as HTMLElement); - } - for (const child of childrenOf(node)) { - upgradeSubtree(child, registry); - } - }; + ) => + forEachInTree(node, (n) => { + if (n.nodeType === Node.ELEMENT_NODE && getRegistry(n) === registry) { + registry._upgradeOrWait(n as HTMLElement); + } + }); class AsyncInfo { readonly promise: Promise; @@ -741,25 +754,22 @@ type ParametersOf< upgrade(root: Node) { withDeferredUpgrades(() => { // Note, as natively, elements the browser hasn't constructed are - // constructed (queuing them) so they can be customized. + // constructed (queuing them) so they can be customized. Shadow roots + // are upgraded natively one by one too, since WebKit's upgrade doesn't + // reach into them (https://bugs.webkit.org/show_bug.cgi?id=326259). nativeUpgrade.call(nativeRegistry, root); - this._upgrade(root); - }); - } - - _upgrade(root: Node) { - const registry = (root as Element)['customElementRegistry']; - if (registry === this && root.nodeType === Node.ELEMENT_NODE) { - (registry as ShimmedCustomElementsRegistry)._upgradeOrWait( - root as HTMLElement + forEachInTree( + root, + (node) => { + if (isShadowRoot(node)) { + nativeUpgrade.call(nativeRegistry, node); + } else if (node.nodeType === Node.ELEMENT_NODE) { + this._upgradeOrWait(node as HTMLElement); + } + }, + true ); - } - const shadowRoot = getShadowRoot(root); - if (shadowRoot) { - nativeUpgrade.call(nativeRegistry, shadowRoot); - this._upgrade(shadowRoot); - } - root.childNodes.forEach((n) => this._upgrade(n)); + }); } get(tagName: string) { @@ -1208,7 +1218,7 @@ type ParametersOf< const observingDefinition = (element: Element, name: string) => { const definition = definitionForElement.get(element as HTMLElement); return definition?.attributeChangedCallback && - definition.observedAttributes.has(name.toLowerCase()) && + definition.observedAttributes.has(String(name).toLowerCase()) && !isFailed(element as HTMLElement) ? definition : undefined; @@ -1239,9 +1249,10 @@ type ParametersOf< invokeCallback( definition.attributeChangedCallback, this as HTMLElement, - name.toLowerCase(), + String(name).toLowerCase(), oldValue, - newValue + newValue, + null ); } return result; @@ -1455,36 +1466,27 @@ type ParametersOf< const setRegistryForSubtree = ( node: Node, registry: ShimmedCustomElementsRegistry | null - ) => { - registryForNode.set(node, registry); - const {children} = node as Element; - if (children?.length) { - Array.from(children).forEach((child) => - setRegistryForSubtree(child, registry) - ); - } - }; + ) => forEachInTree(node, (n) => registryForNode.set(n, registry)); - // Approximate observedAttributes from the user class, since the stand-in element had none + // As natively on upgrade, calls back for each observed attribute the element + // has, in attribute order (the stand-in observes none, see + // `patchAttributeMethod`). const invokeInitialAttributeCallbacks = ( instance: CustomHTMLElement & HTMLElement, definition: CustomElementDefinition ) => { - if (!definition.attributeChangedCallback) { - return; - } - definition.observedAttributes.forEach((attr: string) => { - if (!instance.hasAttribute(attr)) { - return; + for (const attr of Array.from(instance.attributes)) { + if (definition.observedAttributes.has(attr.localName)) { + invokeCallback( + definition.attributeChangedCallback, + instance, + attr.localName, + null, + attr.value, + attr.namespaceURI + ); } - invokeCallback( - definition.attributeChangedCallback, - instance, - attr, - null, - instance.getAttribute(attr) - ); - }); + } }; // Makes a class that extends the native HTMLElement (e.g. one defined before @@ -1767,16 +1769,10 @@ type ParametersOf< }; // Records the registries of `node` and every element under it, including in - // open shadow roots (see `getRegistry`), so that moving them later doesn't + // shadow roots (see `getRegistry`), so that moving them later doesn't // change them. - const recordRegistries = (node: Node) => { - getRegistry(node); - const {shadowRoot} = node as Element; - if (shadowRoot) { - recordRegistries(shadowRoot); - } - Array.from((node as ParentNode).children ?? []).forEach(recordRegistries); - }; + const recordRegistries = (node: Node) => + forEachInTree(node, getRegistry, true); const customElementRegistryDescriptor = { get(this: Node) { @@ -1936,9 +1932,11 @@ type ParametersOf< value(this: Document, node: T): T { // Note, no upgrade here: as native does, adopted elements upgrade // when they are connected. - setRegistryForAdoptedNodes(this, node); - const adopted = adoptNode.call(this, node); - return adopted as T; + return withAdoption( + this, + [node], + () => adoptNode.call(this, node) as T + ); }, enumerable: true, configurable: true, @@ -1961,18 +1959,21 @@ type ParametersOf< // its parent's. const effectiveGlobal = (registry: ShimmedCustomElementsRegistry | null) => registry?._isScoped ? null : registry; + // Each change is noted in `previous`, so it can be undone. const adoptRegistries = ( node: Node, documentRegistry: ShimmedCustomElementsRegistry | null, + previous: Array<[Node, ShimmedCustomElementsRegistry | null | undefined]>, isAdoptedNode = true ) => { const current = getRegistry(node); if (current === null || !current._isScoped) { + let registry: ShimmedCustomElementsRegistry | null | undefined; if (isShadowRoot(node)) { const keepsNull = current === null && node.host.hasAttribute(DSD_HOST_ATTRIBUTE); if (!keepsNull) { - registryForNode.set(node, effectiveGlobal(documentRegistry)); + registry = effectiveGlobal(documentRegistry); } } else if (node.nodeType === Node.ELEMENT_NODE) { const parent = node.parentNode; @@ -1982,37 +1983,61 @@ type ParametersOf< parent === null || (parent.nodeType === Node.DOCUMENT_FRAGMENT_NODE && !isShadowRoot(parent)); - registryForNode.set( - node, - effectiveGlobal(fromDocument ? documentRegistry : getRegistry(parent)) + registry = effectiveGlobal( + fromDocument ? documentRegistry : getRegistry(parent) ); } + if (registry !== undefined) { + previous.push([node, registryForNode.get(node)]); + registryForNode.set(node, registry); + } } const shadowRoot = getShadowRoot(node); if (shadowRoot) { - adoptRegistries(shadowRoot, documentRegistry, false); + adoptRegistries(shadowRoot, documentRegistry, previous, false); } for (const child of childrenOf(node)) { - adoptRegistries(child, documentRegistry, false); + adoptRegistries(child, documentRegistry, previous, false); } }; + // Runs `fn`, which inserts or adopts `nodes` into `doc`, having first given + // those from other documents their adopted registries (see + // `adoptRegistries`). If `fn` throws, the registries are restored. // Note, as natively, adopted elements upgrade when connected: stand-ins // via their `connectedCallback`, others when the browser constructs them. - const setRegistryForAdoptedNodes = (doc: Document, ...args: Array) => - (args as unknown[]).forEach((arg) => { + const withAdoption = ( + doc: Document, + nodes: Array, + fn: () => T + ): T => { + const previous: Array< + [Node, ShimmedCustomElementsRegistry | null | undefined] + > = []; + for (const node of nodes) { + // Note, a document can't be adopted (native throws). if ( - typeof (arg as Node | null)?.nodeType !== 'number' || - (arg as Node).ownerDocument === doc + typeof (node as Node | null)?.nodeType === 'number' && + (node as Node).nodeType !== Node.DOCUMENT_NODE && + (node as Node).ownerDocument !== doc ) { - return; + adoptRegistries(node as Node, getRegistry(doc), previous); } - adoptRegistries( - arg as Node, - doc['customElementRegistry'] as ShimmedCustomElementsRegistry | null - ); - }); - + } + try { + return fn(); + } catch (e) { + for (let i = previous.length - 1; i >= 0; i--) { + const [node, registry] = previous[i]; + if (registry === undefined) { + registryForNode.delete(node); + } else { + registryForNode.set(node, registry); + } + } + throw e; + } + }; // A node without a record gets its registry from where it is (see // `getRegistry`), so before a node is moved or removed, its registry is // recorded. Recording the node is enough, since its descendants' registries @@ -2056,11 +2081,11 @@ type ParametersOf< if (removed) { recordRegistriesBeforeMove(removed(this)); } - setRegistryForAdoptedNodes( + return withAdoption( (this.ownerDocument ?? this) as Document, - ...args + args, + () => native.apply(this, args) ); - return native.apply(this, args); }); }; }; @@ -2081,19 +2106,20 @@ type ParametersOf< } }; - const installScopedSetHTMLUnsafe = (ctor: Function) => { - const native = ctor.prototype['setHTMLUnsafe']; + // Parsing methods that replace the children, e.g. `setHTMLUnsafe`. + const installScopedParsingMethod = (ctor: Function, method: string) => { + const native = ctor.prototype[method]; if (native === undefined) { return; } - ctor.prototype['setHTMLUnsafe'] = function ( + ctor.prototype[method] = function ( this: Element | ShadowRoot, ...args: Array ) { withDeferredUpgrades(() => { recordRegistriesBeforeMove(childrenOf(this)); native.apply(this, args); - Array.from(this.children).forEach(recordRegistries); + childrenOf(this).forEach(recordRegistries); }); }; }; @@ -2130,8 +2156,10 @@ type ParametersOf< recordRegistriesBetween(parent, start, end); }); }; - installScopedSetHTMLUnsafe(Element); - installScopedSetHTMLUnsafe(ShadowRoot); + for (const ctor of [Element, ShadowRoot]) { + installScopedParsingMethod(ctor, 'setHTMLUnsafe'); + installScopedParsingMethod(ctor, 'setHTML'); + } // Clones a tree, preserving each node's registry. // @@ -2302,10 +2330,25 @@ type ParametersOf< registryForNode.set(clone, registry); } } + const sources = elementsOf(node, deep); const copies = elementsOf(clone, deep); - elementsOf(node, deep).forEach((source, i) => - pairClonedElement(source, copies[i], fallbackRegistry) - ); + if (sources.length === copies.length) { + sources.forEach((source, i) => + pairClonedElement(source, copies[i], fallbackRegistry) + ); + } else { + // Note, the copy can't be paired by index (e.g. a customized built-in's + // constructor changed the source during the native clone), so it gets + // the source's registry throughout. + setRegistryForSubtree( + clone, + getCloneRegistry( + node.nodeType === Node.ELEMENT_NODE ? getRegistry(node) : null, + clone, + fallbackRegistry + ) + ); + } pairUnreachableCopyRoots(); return clone; }; @@ -2347,11 +2390,11 @@ type ParametersOf< const container = this.startContainer; return withDeferredUpgrades(() => { recordRegistriesBeforeMove([node]); - setRegistryForAdoptedNodes( + withAdoption( (container.ownerDocument ?? container) as Document, - node + [node], + () => nativeInsertNode.call(this, node) ); - nativeInsertNode.call(this, node); }); }; const nativeSurroundContents = Range.prototype.surroundContents; @@ -2361,11 +2404,11 @@ type ParametersOf< // Note, the range's contents replace `newParent`'s children. recordRegistriesBeforeMove([newParent, ...childrenOf(newParent)]); recordRegistriesBeforeMove(elementsInRange(this)); - setRegistryForAdoptedNodes( + withAdoption( (container.ownerDocument ?? container) as Document, - newParent + [newParent], + () => nativeSurroundContents.call(this, newParent) ); - nativeSurroundContents.call(this, newParent); }); }; // Note, the new nodes are parsed in the context of the range's start, so @@ -2421,6 +2464,12 @@ type ParametersOf< installScopedMethod(DocumentFragment, 'append'); installScopedMethod(DocumentFragment, 'prepend'); installScopedMethod(DocumentFragment, 'replaceChildren', childrenOf); + installScopedMethod(Document, 'append'); + installScopedMethod(Document, 'prepend'); + installScopedMethod(Document, 'replaceChildren', childrenOf); + installScopedMethod(CharacterData, 'before'); + installScopedMethod(CharacterData, 'after'); + installScopedMethod(CharacterData, 'replaceWith'); // Methods that move or remove nodes without constructing any, which only // need to record the registries of those nodes first. @@ -2443,20 +2492,32 @@ type ParametersOf< recordBefore(Element, 'moveBefore', (_, args) => args); recordBefore(Document, 'moveBefore', (_, args) => args); recordBefore(DocumentFragment, 'moveBefore', (_, args) => args); - recordBefore(Element, 'setHTML', childrenOf); - recordBefore(ShadowRoot, 'setHTML', childrenOf); recordBefore(Range, 'deleteContents', elementsInRange); - const textContentDescriptor = Object.getOwnPropertyDescriptor( - Node.prototype, - 'textContent' - )!; - Object.defineProperty(Node.prototype, 'textContent', { - ...textContentDescriptor, - set(this: Node, value: string | null) { - recordRegistriesBeforeMove(childrenOf(this)); - textContentDescriptor.set!.call(this, value); - }, - }); + // Setters that replace nodes with text, which only need to record the + // registries of the nodes they remove. + const recordBeforeSetter = ( + ctor: Function, + name: string, + nodesOf: (target: Node) => Iterable + ) => { + const descriptor = Object.getOwnPropertyDescriptor(ctor.prototype, name); + if (!descriptor?.set) { + return; + } + Object.defineProperty(ctor.prototype, name, { + ...descriptor, + set(this: Node, value: unknown) { + recordRegistriesBeforeMove(nodesOf(this)); + descriptor.set!.call(this, value); + }, + }); + }; + // Note, most `textContent` targets have no element children. + const elementChildrenOf = (node: Node) => + (node as ParentNode).firstElementChild ? childrenOf(node) : []; + recordBeforeSetter(Node, 'textContent', elementChildrenOf); + recordBeforeSetter(HTMLElement, 'innerText', elementChildrenOf); + recordBeforeSetter(HTMLElement, 'outerText', (element) => [element]); // Install scoped innerHTML on Element & ShadowRoot const installScopedSetter = (ctor: Function, name: string) => { diff --git a/packages/scoped-custom-element-registry/test/behavior.test.js b/packages/scoped-custom-element-registry/test/behavior.test.js index a1210b4a..106415a7 100644 --- a/packages/scoped-custom-element-registry/test/behavior.test.js +++ b/packages/scoped-custom-element-registry/test/behavior.test.js @@ -621,6 +621,27 @@ describe('initialize and lifecycle', () => { }); describe('attributeChangedCallback', () => { + it('on upgrade, calls back in attribute order, matching names exactly', () => { + const tagName = getTestTagName(); + const calls = []; + const container = document.createElement('div'); + container.innerHTML = `<${tagName} b="1" foobar="2" a="3">`; + customElements.define( + tagName, + class extends HTMLElement { + static observedAttributes = ['a', 'b', 'fooBar']; + attributeChangedCallback(name, oldValue, newValue, namespace) { + calls.push([name, oldValue, newValue, namespace]); + } + } + ); + customElements.upgrade(container); + expect(calls).to.deep.equal([ + ['b', null, '1', null], + ['a', null, '3', null], + ]); + }); + // Returns a class whose attributeChangedCallback logs `name`. const logAttributes = (log) => class extends HTMLElement { @@ -1822,6 +1843,27 @@ describe('Upgrade order', () => { }); describe('Adoption', () => { + it('inserting next to a text node adopts the element', () => { + for (const method of ['before', 'after']) { + const el = inertDocument().createElement('div'); + const text = document.body.appendChild(document.createTextNode('')); + text[method](el); + expect(el.customElementRegistry, method).to.equal(customElements); + el.remove(); + text.remove(); + } + }); + + it("an insertion that throws doesn't change the node's registry", () => { + const el = document.createElement('div'); + const otherDocument = document.implementation.createHTMLDocument(); + expect(() => + otherDocument.body.insertBefore(el, document.createElement('i')) + ).to.throw(DOMException); + expect(el.ownerDocument).to.equal(document); + expect(el.customElementRegistry).to.equal(customElements); + }); + it('a null element adopted from an inert document gets the document registry', () => { const el = inertDocument().createElement('div'); expect(el.customElementRegistry).to.be.null; From a299e428f5bf2fd110abc38178e10376a0896f74 Mon Sep 17 00:00:00 2001 From: Steve Orvell Date: Sat, 10 Oct 2026 15:39:15 -0700 Subject: [PATCH 03/11] Review cleanup: remove no-op code, share element creation, skip empty flushes --- .../src/scoped-custom-element-registry.ts | 200 +++++++++--------- 1 file changed, 97 insertions(+), 103 deletions(-) diff --git a/packages/scoped-custom-element-registry/src/scoped-custom-element-registry.ts b/packages/scoped-custom-element-registry/src/scoped-custom-element-registry.ts index 32b519c2..a239b2a8 100644 --- a/packages/scoped-custom-element-registry/src/scoped-custom-element-registry.ts +++ b/packages/scoped-custom-element-registry/src/scoped-custom-element-registry.ts @@ -320,10 +320,6 @@ type ParametersOf< 'definitionForElement', () => new WeakMap() ); - const waitingRegistryForElement = shared( - 'waitingRegistryForElement', - () => new WeakMap() - ); const globalDefinitionForConstructor = new WeakMap< CustomElementConstructor, CustomElementDefinition @@ -441,8 +437,10 @@ type ParametersOf< // Whether an HTML element named `name` would be an HTMLUnknownElement. const nativeCreateElementNS = Document.prototype.createElementNS as ( - namespace: string, - name: string + this: Document, + namespace: string | null, + name: string, + options?: ElementCreationOptions ) => Element; const isUnknownElementName = (name: string) => { try { @@ -839,8 +837,7 @@ type ParametersOf< } definition ??= this._getDefinition(element.localName); if (definition !== undefined) { - waitingRegistryForElement.delete(element); - customize(element, definition!, true); + customize(element, definition!); } else if (element.localName.includes('-')) { // Note, so once defined, it's upgraded. constructedDirectly.delete(element); @@ -848,7 +845,6 @@ type ParametersOf< // connected/disconnected callbacks manage queuing for upgrade, so // only queue when the element is a stand-in and connected; otherwise // the element upgrades via native define. - waitingRegistryForElement.set(element, this); if (element.isConnected && standInElements.has(element)) { this._waitForDefinition(element); } @@ -1076,8 +1072,10 @@ type ParametersOf< definition.disconnectedCallback.apply(this, args); } } else { - // Note, so a disconnected element isn't kept alive. - waitingRegistryForElement.get(this)?._stopWaitingForDefinition(this); + // Note, so a disconnected element isn't kept alive. Its registry + // hasn't changed since it started waiting: only a null registry can + // change while connected (by `initialize`), and null never waits. + getRegistry(this)?._stopWaitingForDefinition(this); } } @@ -1366,6 +1364,9 @@ type ParametersOf< // Note, elements added to the queue while it's flushing (e.g. constructed by // an unpatched API in a user's constructor) are customized in this flush. const flushUpgrades = (queue: Set) => { + if (queue.size === 0) { + return; + } flushingQueues.add(queue); try { queue.forEach((instance) => { @@ -1432,9 +1433,6 @@ type ParametersOf< const observeParsing = () => { const parsed = parserQueue; const observer = new MutationObserver(() => { - if (flushingQueues.has(parsed)) { - return; - } const inserted = new Set( Array.from(parsed).filter((element) => element.parentNode !== null) ); @@ -1511,8 +1509,7 @@ type ParametersOf< // Helper to upgrade an instance with a CE definition using "constructor call trick" const customize = ( instance: HTMLElement, - definition: CustomElementDefinition, - isUpgrade = false + definition: CustomElementDefinition ) => { // prevent double customization if (definitionForElement.get(instance) === definition) { @@ -1573,7 +1570,9 @@ type ParametersOf< if (definition.attributeChangedCallback) { invokeInitialAttributeCallbacks(instance, definition); } - if (isUpgrade && instance.isConnected) { + // Note, the stand-in's own connected callback ran before it was + // customized, so the user's runs now. + if (instance.isConnected) { invokeCallback(definition.connectedCallback, instance); } }; @@ -1659,41 +1658,39 @@ type ParametersOf< (registry as ShimmedCustomElementsRegistry | undefined)?._associate?.( this.ownerDocument ); - return withDeferredUpgrades(() => { - const shadowRoot = nativeAttachShadow.call( - this, - nativeInit, - ...(args as []) - ) as ShadowRootWithSettableCustomElementRegistry; - // Note, for a host with the polyfill's declarative attribute, native - // returns its existing declarative root, emptied. - if (this.hasAttribute(DSD_HOST_ATTRIBUTE)) { - getRegistry(shadowRoot); - return shadowRoot; - } - shadowRootForHost.set(this, shadowRoot); - // Note, as native does, the root uses the document's registry by - // default. Recording it marks the root as attached by the polyfill (see - // `getRegistry`). - registryForNode.set( - shadowRoot, - (registry !== undefined - ? registry - : this.ownerDocument[ - 'customElementRegistry' - ]) as ShimmedCustomElementsRegistry | null - ); - if (registry !== undefined) { - // for back compat, set both `registry` and `customElements` - (shadowRoot as ShadowRootInitWithSettableCustomElements)[ - 'registry' - ] = registry; - (shadowRoot as ShadowRootInitWithSettableCustomElements)[ - 'customElements' - ] = registry; - } + const shadowRoot = nativeAttachShadow.call( + this, + nativeInit, + ...(args as []) + ) as ShadowRootWithSettableCustomElementRegistry; + // Note, for a host with the polyfill's declarative attribute, native + // returns its existing declarative root, emptied. + if (this.hasAttribute(DSD_HOST_ATTRIBUTE)) { + getRegistry(shadowRoot); return shadowRoot; - }); + } + shadowRootForHost.set(this, shadowRoot); + // Note, as native does, the root uses the document's registry by + // default. Recording it marks the root as attached by the polyfill (see + // `getRegistry`). + registryForNode.set( + shadowRoot, + (registry !== undefined + ? registry + : this.ownerDocument[ + 'customElementRegistry' + ]) as ShimmedCustomElementsRegistry | null + ); + if (registry !== undefined) { + // for back compat, set both `registry` and `customElements` + (shadowRoot as ShadowRootInitWithSettableCustomElements)[ + 'registry' + ] = registry; + (shadowRoot as ShadowRootInitWithSettableCustomElements)[ + 'customElements' + ] = registry; + } + return shadowRoot; }; // The global registry of a document's window, or null if it has none (or @@ -1782,12 +1779,18 @@ type ParametersOf< configurable: true, }; - const { - createElement, - createElementNS, - importNode, - adoptNode, - } = Document.prototype; + const {importNode, adoptNode} = Document.prototype; + const createElement = Document.prototype.createElement as ( + this: Document, + tagName: string, + options?: ElementCreationOptions + ) => Element; + const createElementNS = nativeCreateElementNS as ( + this: Document, + namespace: string | null, + tagName: string, + options?: ElementCreationOptions + ) => Element; // https://dom.spec.whatwg.org/#flatten-element-creation-options // Returns the registry to use and the options to pass to the native method. @@ -1820,6 +1823,28 @@ type ParametersOf< return [optionsRegistry, nativeOptions]; }; + // Creates an element with the registry from `options`, constructing it + // directly (see `constructedDirectly`). + const createElementWithRegistry = ( + doc: Document, + options: string | ElementCreationOptions | undefined, + method: string, + create: (options: ElementCreationOptions | undefined) => Element + ) => { + const [registry, nativeOptions] = flattenElementCreationOptions( + doc, + options, + method + ); + return withDeferredUpgrades(() => { + const element = constructDirectly(() => + create(nativeOptions as ElementCreationOptions | undefined) + ); + registryForNode.set(element, registry as ShimmedCustomElementsRegistry); + return element; + }); + }; + Object.defineProperty( Element.prototype, 'customElementRegistry', @@ -1829,59 +1854,31 @@ type ParametersOf< 'customElementRegistry': customElementRegistryDescriptor, // https://dom.spec.whatwg.org/#dom-document-createelement 'createElement': { - value( + value( this: Document, - tagName: K, + tagName: string, options?: string | ElementCreationOptions - ): HTMLElementTagNameMap[K] { - const [ - customElementRegistry, - nativeOptions, - ] = flattenElementCreationOptions(this, options, 'createElement'); - return withDeferredUpgrades(() => { - const el = constructDirectly(() => - createElement.call( - this, - tagName, - nativeOptions as ElementCreationOptions - ) - ) as HTMLElementTagNameMap[K]; - registryForNode.set( - el, - customElementRegistry as ShimmedCustomElementsRegistry - ); - return el; - }); + ) { + return createElementWithRegistry(this, options, 'createElement', (o) => + createElement.call(this, tagName, o) + ); }, enumerable: true, configurable: true, }, 'createElementNS': { - value( + value( this: Document, namespace: string | null, - tagName: K, + tagName: string, options?: string | ElementCreationOptions - ): HTMLElementTagNameMap[K] { - const [ - customElementRegistry, - nativeOptions, - ] = flattenElementCreationOptions(this, options, 'createElementNS'); - return withDeferredUpgrades(() => { - const el = constructDirectly(() => - createElementNS.call( - this, - namespace, - tagName, - nativeOptions as ElementCreationOptions - ) - ) as HTMLElementTagNameMap[K]; - registryForNode.set( - el, - customElementRegistry as ShimmedCustomElementsRegistry - ); - return el; - }); + ) { + return createElementWithRegistry( + this, + options, + 'createElementNS', + (o) => createElementNS.call(this, namespace, tagName, o) + ); }, enumerable: true, configurable: true, @@ -2213,9 +2210,6 @@ type ParametersOf< fallbackRegistry ) ); - if (copyElement.localName.includes('-')) { - currentQueue().add(copyElement as HTMLElement); - } // Note, as natively, the fallback registry doesn't apply to a template's // contents. A shallow clone doesn't copy them, so there's nothing to pair. if (isTemplate(sourceElement)) { From 9628e19cc19f847a6e2c9250148d32dec0bd6f0b Mon Sep 17 00:00:00 2001 From: Steve Orvell Date: Sat, 10 Oct 2026 16:03:54 -0700 Subject: [PATCH 04/11] Review fixes: customize once, form callbacks on upgrade, define re-entry, adoption rollback, shared parsing wrapper --- .../src/scoped-custom-element-registry.ts | 97 ++++++++++++++----- .../test/behavior.test.js | 72 ++++++++++++++ 2 files changed, 143 insertions(+), 26 deletions(-) diff --git a/packages/scoped-custom-element-registry/src/scoped-custom-element-registry.ts b/packages/scoped-custom-element-registry/src/scoped-custom-element-registry.ts index a239b2a8..54292867 100644 --- a/packages/scoped-custom-element-registry/src/scoped-custom-element-registry.ts +++ b/packages/scoped-custom-element-registry/src/scoped-custom-element-registry.ts @@ -309,6 +309,12 @@ type ParametersOf< 'internalsAttachedByUser', () => new WeakSet() ); + const nativeInternalsFormGetter = window['ElementInternals'] + ? Object.getOwnPropertyDescriptor( + window['ElementInternals'].prototype, + 'form' + )?.get + : undefined; const statesOf = (internals: ElementInternals | undefined) => (internals as {['states']?: Set} | undefined)?.['states']; const nativeDefine = window.customElements.define; @@ -365,7 +371,12 @@ type ParametersOf< if (shadowRoot) { forEachInTree(shadowRoot, fn, true); } - for (const child of childrenOf(node)) { + // Note, `fn` doesn't change the tree. + for ( + let child = (node as ParentNode).firstElementChild; + child; + child = child.nextElementSibling + ) { forEachInTree(child, fn, shadowRoots); } }; @@ -642,9 +653,18 @@ type ParametersOf< `the name "${tagName}" is used by a customized built-in element` ); } - const definition = this._whileDefining(() => + let definition = this._whileDefining(() => readDefinition(tagName, elementClass, standInClass) ); + // Note, reading the class runs user code, which can define the tag in + // another registry, so the stand-in is looked up again. + if (!standInClass && nativeGet.call(nativeRegistry, tagName)) { + standInClass = nativeGet.call(nativeRegistry, tagName); + definition = { + ...definition, + standInFormAssociated: standInClass!['formAssociated'], + }; + } // Note, native define constructs the stand-ins of existing elements, // which are customized when this entry point returns. if (!standInClass) { @@ -841,10 +861,11 @@ type ParametersOf< } else if (element.localName.includes('-')) { // Note, so once defined, it's upgraded. constructedDirectly.delete(element); - // Note, only custom names can be defined. The stand-in's - // connected/disconnected callbacks manage queuing for upgrade, so - // only queue when the element is a stand-in and connected; otherwise - // the element upgrades via native define. + // Note, only custom names can be defined. A connected stand-in waits + // for a definition here (its disconnected callback stops waiting, and + // its connected callback waits again). Any other element is upgraded + // once the browser constructs it, by native define or on its next + // connection. if (element.isConnected && standInElements.has(element)) { this._waitForDefinition(element); } @@ -1062,7 +1083,7 @@ type ParametersOf< disconnectedCallback( this: HTMLElement, - ...args: ParametersOf + ...args: ParametersOf ) { const definition = definitionForElement.get(this); if (definition) { @@ -1287,7 +1308,8 @@ type ParametersOf< (isParsing() || entryQueues.length > 0); const isUpgradeQueued = (element: HTMLElement) => parserQueue.has(element) || entryQueues.some((queue) => queue.has(element)); - // Elements the browser constructed as stand-ins. Only these are customized: + // Elements the browser constructed as stand-ins. Only these are customized + // (besides elements constructed with `new`, see `HTMLElement` above): // customizing any other element would be undone when the browser later // constructs it, which resets its prototype. const standInElements = shared( @@ -1511,8 +1533,9 @@ type ParametersOf< instance: HTMLElement, definition: CustomElementDefinition ) => { - // prevent double customization - if (definitionForElement.get(instance) === definition) { + // Note, as natively, an element is customized at most once, and never + // after its upgrade failed. + if (definitionForElement.has(instance) || isFailed(instance)) { return; } // Note, an element the browser hasn't constructed is left to it; it's @@ -1575,6 +1598,19 @@ type ParametersOf< if (instance.isConnected) { invokeCallback(definition.connectedCallback, instance); } + // Note, as natively on upgrade, a form-associated element is told its + // form and whether it's disabled. The stand-in's own form callbacks ran + // before it was customized (see `getFormAssociatedDefinition`). + if (definition['formAssociated']) { + const internals = internalsForElement.get(instance); + const form = internals && nativeInternalsFormGetter?.call(internals); + if (form) { + invokeCallback(definition['formAssociatedCallback'], instance, form); + } + if (instance.matches(':disabled')) { + invokeCallback(definition['formDisabledCallback'], instance, true); + } + } }; // https://dom.spec.whatwg.org/#dom-element-attachshadow etc.: a non-scoped @@ -1698,6 +1734,10 @@ type ParametersOf< // Note, native registry state is deliberately not consulted, so the // polyfill behaves the same whether or not the browser has support. const getWindowRegistry = (doc: Document) => { + // Note, `window.customElements` is writable, so this window's isn't read. + if (doc === document) { + return globalCustomElementRegistry; + } const registry = doc.defaultView?.customElements; return isPolyfillRegistry(registry) ? registry : null; }; @@ -2026,6 +2066,11 @@ type ParametersOf< } catch (e) { for (let i = previous.length - 1; i >= 0; i--) { const [node, registry] = previous[i]; + // Note, a native method can adopt and then throw, e.g. `append` of a + // node from another document along with an invalid node. + if (node.ownerDocument === doc) { + continue; + } if (registry === undefined) { registryForNode.delete(node); } else { @@ -2103,21 +2148,22 @@ type ParametersOf< } }; + // Runs `parse`, which replaces `target`'s children with parsed ones, + // recording the registries of the old children before and the new after. + const replaceChildrenByParsing = (target: Node, parse: () => void) => + withDeferredUpgrades(() => { + recordRegistriesBeforeMove(childrenOf(target)); + parse(); + childrenOf(target).forEach(recordRegistries); + }); // Parsing methods that replace the children, e.g. `setHTMLUnsafe`. const installScopedParsingMethod = (ctor: Function, method: string) => { const native = ctor.prototype[method]; if (native === undefined) { return; } - ctor.prototype[method] = function ( - this: Element | ShadowRoot, - ...args: Array - ) { - withDeferredUpgrades(() => { - recordRegistriesBeforeMove(childrenOf(this)); - native.apply(this, args); - childrenOf(this).forEach(recordRegistries); - }); + ctor.prototype[method] = function (this: Node, ...args: Array) { + replaceChildrenByParsing(this, () => native.apply(this, args)); }; }; @@ -2515,15 +2561,14 @@ type ParametersOf< // Install scoped innerHTML on Element & ShadowRoot const installScopedSetter = (ctor: Function, name: string) => { - const descriptor = Object.getOwnPropertyDescriptor(ctor.prototype, name)!; + const descriptor = Object.getOwnPropertyDescriptor(ctor.prototype, name); + if (!descriptor?.set) { + return; + } Object.defineProperty(ctor.prototype, name, { ...descriptor, - set(value) { - withDeferredUpgrades(() => { - recordRegistriesBeforeMove(childrenOf(this)); - descriptor.set!.call(this, value); - Array.from((this as ParentNode).children).forEach(recordRegistries); - }); + set(this: Node, value: unknown) { + replaceChildrenByParsing(this, () => descriptor.set!.call(this, value)); }, }); }; diff --git a/packages/scoped-custom-element-registry/test/behavior.test.js b/packages/scoped-custom-element-registry/test/behavior.test.js index 106415a7..e7681537 100644 --- a/packages/scoped-custom-element-registry/test/behavior.test.js +++ b/packages/scoped-custom-element-registry/test/behavior.test.js @@ -197,6 +197,53 @@ describe('Registry arguments', () => { }); }); +describe('Customized once', () => { + it("an element adopted into a null registry and initialized isn't customized again", () => { + const tagName = getTestTagName(); + const constructed = []; + customElements.define( + tagName, + class extends HTMLElement { + constructor() { + super(); + constructed.push('global'); + } + } + ); + const registry = new CustomElementRegistry(); + registry.define( + tagName, + class extends HTMLElement { + constructor() { + super(); + constructed.push('scoped'); + } + } + ); + const el = document.createElement(tagName); + document.implementation.createHTMLDocument().adoptNode(el); + registry.initialize(el); + document.body.append(el); + el.remove(); + expect(constructed).to.deep.equal(['global']); + }); + + it('defining a tag while another registry reads its class works', () => { + const tagName = getTestTagName(); + const other = new CustomElementRegistry(); + class Outer extends HTMLElement { + static get observedAttributes() { + other.define(tagName, class extends HTMLElement {}); + return []; + } + attributeChangedCallback() {} + } + const registry = new CustomElementRegistry(); + expect(() => registry.define(tagName, Outer)).not.to.throw(); + expect(registry.get(tagName)).to.equal(Outer); + }); +}); + describe('Constructing with new', () => { it('an element constructed with new keeps the global registry when moved into a scoped root', () => { const tagName = getTestTagName(); @@ -2408,6 +2455,31 @@ describe('Constructors', () => { }); describe('Forms', () => { + // Note, as natively on upgrade. + it('upgrading tells a form-associated element its form and that it is disabled', () => { + const tagName = getTestTagName(); + const calls = []; + const form = document.body.appendChild(document.createElement('form')); + form.innerHTML = `
<${tagName}>
`; + customElements.define( + tagName, + class extends HTMLElement { + static formAssociated = true; + formAssociatedCallback(owner) { + calls.push(['form', owner === form]); + } + formDisabledCallback(disabled) { + calls.push(['disabled', disabled]); + } + } + ); + form.remove(); + expect(calls).to.deep.equal([ + ['form', true], + ['disabled', true], + ]); + }); + it("an ordinary form's elements behave natively", () => { const form = document.createElement('form'); form.innerHTML = ``; From 1ec8614cb36f197669bd846972a1b0f4f023b58e Mon Sep 17 00:00:00 2001 From: Steve Orvell Date: Sat, 10 Oct 2026 16:16:32 -0700 Subject: [PATCH 05/11] Review fixes: prototype set by super(), form callbacks only for form-associated stand-ins, waiting elements and adoption, record parentless nodes before moves --- .../src/scoped-custom-element-registry.ts | 96 +++++++++++-------- .../test/behavior.test.js | 63 ++++++++++++ 2 files changed, 120 insertions(+), 39 deletions(-) diff --git a/packages/scoped-custom-element-registry/src/scoped-custom-element-registry.ts b/packages/scoped-custom-element-registry/src/scoped-custom-element-registry.ts index 54292867..d34c0219 100644 --- a/packages/scoped-custom-element-registry/src/scoped-custom-element-registry.ts +++ b/packages/scoped-custom-element-registry/src/scoped-custom-element-registry.ts @@ -371,7 +371,8 @@ type ParametersOf< if (shadowRoot) { forEachInTree(shadowRoot, fn, true); } - // Note, `fn` doesn't change the tree. + // Note, `fn` doesn't change the tree: callers only record registries, or + // queue upgrades, which run when their entry point returns. for ( let child = (node as ParentNode).firstElementChild; child; @@ -511,8 +512,7 @@ type ParametersOf< // https://html.spec.whatwg.org/#dom-customelementregistry-define const readDefinition = ( tagName: string, - elementClass: CustomElementConstructor, - standInClass: CustomElementConstructor | undefined + elementClass: CustomElementConstructor ): CustomElementDefinition => { const {prototype} = elementClass; if ( @@ -555,15 +555,6 @@ type ParametersOf< disableShadow: disabledFeatures.includes('shadow'), disableInternals: disabledFeatures.includes('internals'), 'formAssociated': formAssociated, - // Note, the browser only knows the stand-in, so whether the tag can be - // form-associated is fixed by its first definition, or reserved in - // `CustomElementRegistryPolyfill.formAssociated`. - standInFormAssociated: - standInClass?.formAssociated ?? - (formAssociated || - polyfillWindow['CustomElementRegistryPolyfill']['formAssociated'].has( - tagName - )), }; }; @@ -647,24 +638,31 @@ type ParametersOf< this._defineCustomizedBuiltIn(tagName, elementClass, options); return; } - let standInClass = nativeGet.call(nativeRegistry, tagName); - if (standInClass && !standInClasses.has(standInClass)) { - throw defineError( - `the name "${tagName}" is used by a customized built-in element` - ); - } - let definition = this._whileDefining(() => - readDefinition(tagName, elementClass, standInClass) + const checkStandIn = () => { + const standInClass = nativeGet.call(nativeRegistry, tagName); + if (standInClass && !standInClasses.has(standInClass)) { + throw defineError( + `the name "${tagName}" is already defined natively (e.g. as a customized built-in element)` + ); + } + return standInClass; + }; + checkStandIn(); + const definition = this._whileDefining(() => + readDefinition(tagName, elementClass) ); - // Note, reading the class runs user code, which can define the tag in - // another registry, so the stand-in is looked up again. - if (!standInClass && nativeGet.call(nativeRegistry, tagName)) { - standInClass = nativeGet.call(nativeRegistry, tagName); - definition = { - ...definition, - standInFormAssociated: standInClass!['formAssociated'], - }; - } + // Note, reading the class runs user code, which can define the tag + // elsewhere, so the stand-in is looked up after it. The browser only + // knows the stand-in, so whether the tag can be form-associated is fixed + // by its first definition, or reserved in + // `CustomElementRegistryPolyfill.formAssociated`. + let standInClass = checkStandIn(); + definition.standInFormAssociated = standInClass + ? Boolean(standInClass['formAssociated']) + : definition['formAssociated'] || + polyfillWindow['CustomElementRegistryPolyfill']['formAssociated'].has( + tagName + ); // Note, native define constructs the stand-ins of existing elements, // which are customized when this entry point returns. if (!standInClass) { @@ -727,7 +725,7 @@ type ParametersOf< // their constructors and callbacks. Native define reads the class // again: if that fails, the definition is removed. const definition = this._whileDefining(() => - readDefinition(tagName, elementClass, undefined) + readDefinition(tagName, elementClass) ); this._definitionsByTag.set(tagName, definition); this._definitionsByClass.set(elementClass, definition); @@ -957,6 +955,9 @@ type ParametersOf< current.construction = isUpgrade ? {instance: undefined, elementClass, isUpgrade} : undefined; + // Note, as natively, the prototype is set by `super()`, so a + // constructor that throws before it leaves the element as it was. + Object.setPrototypeOf(instance, Object.getPrototypeOf(this)); return instance; } // Construction case: we need to construct the StandInElement and return @@ -1094,8 +1095,10 @@ type ParametersOf< } } else { // Note, so a disconnected element isn't kept alive. Its registry - // hasn't changed since it started waiting: only a null registry can - // change while connected (by `initialize`), and null never waits. + // hasn't changed since it started waiting: `initialize` only changes + // a null registry, which never waits, and adoption, which changes + // registries before the move, stops it waiting first (see + // `adoptRegistries`). getRegistry(this)?._stopWaitingForDefinition(this); } } @@ -1548,7 +1551,6 @@ type ParametersOf< currentQueue().add(instance); return; } - Object.setPrototypeOf(instance, definition.elementClass.prototype); definitionForElement.set(instance, definition); const {elementClass} = definition; const previous = current.construction; @@ -1601,7 +1603,7 @@ type ParametersOf< // Note, as natively on upgrade, a form-associated element is told its // form and whether it's disabled. The stand-in's own form callbacks ran // before it was customized (see `getFormAssociatedDefinition`). - if (definition['formAssociated']) { + if (definition['formAssociated'] && definition.standInFormAssociated) { const internals = internalsForElement.get(instance); const form = internals && nativeInternalsFormGetter?.call(internals); if (form) { @@ -1729,8 +1731,9 @@ type ParametersOf< return shadowRoot; }; - // The global registry of a document's window, or null if it has none (or - // the window doesn't run the polyfill). + // The global registry of a document's window: this window's for its + // document, otherwise null (a document without a window, or another + // window's, whose registries this copy of the polyfill doesn't know). // Note, native registry state is deliberately not consulted, so the // polyfill behaves the same whether or not the browser has support. const getWindowRegistry = (doc: Document) => { @@ -1969,6 +1972,7 @@ type ParametersOf< value(this: Document, node: T): T { // Note, no upgrade here: as native does, adopted elements upgrade // when they are connected. + recordRegistriesBeforeMove([node]); return withAdoption( this, [node], @@ -2023,6 +2027,9 @@ type ParametersOf< registry = effectiveGlobal( fromDocument ? documentRegistry : getRegistry(parent) ); + // Note, the native move disconnects it after this, by which time its + // registry has changed. + current?._stopWaitingForDefinition(node as HTMLElement); } if (registry !== undefined) { previous.push([node, registryForNode.get(node)]); @@ -2089,8 +2096,15 @@ type ParametersOf< // for methods that only move or remove. const recordRegistriesBeforeMove = (nodes: Iterable) => { for (const node of nodes) { - if ((node as Node | null)?.nodeType === Node.ELEMENT_NODE) { - getRegistry(node as Node); + const type = (node as Node | null)?.nodeType; + if (type === Node.ELEMENT_NODE) { + // Note, `getRegistry` doesn't record a node without a parent (or in a + // plain fragment), so it's recorded here. + if (!registryForNode.has(node as Node)) { + registryForNode.set(node as Node, getRegistry(node as Node)); + } + } else if (type === Node.DOCUMENT_FRAGMENT_NODE) { + recordRegistriesBeforeMove(childrenOf(node as Node)); } } }; @@ -2269,7 +2283,11 @@ type ParametersOf< } } const sourceRoot = getShadowRoot(sourceElement); - if (!sourceRoot) { + // Note, a root that isn't clonable isn't copied. + if ( + !sourceRoot || + (sourceRoot as {['clonable']?: boolean})['clonable'] === false + ) { return; } // Note, a closed copy root is reached via the copy host's internals, or diff --git a/packages/scoped-custom-element-registry/test/behavior.test.js b/packages/scoped-custom-element-registry/test/behavior.test.js index e7681537..ba554af8 100644 --- a/packages/scoped-custom-element-registry/test/behavior.test.js +++ b/packages/scoped-custom-element-registry/test/behavior.test.js @@ -244,6 +244,46 @@ describe('Customized once', () => { }); }); +describe('Prototype on a failed upgrade', () => { + it('a constructor that throws before super() leaves the element an HTMLElement', () => { + const tagName = getTestTagName(); + const container = document.body.appendChild(document.createElement('div')); + container.innerHTML = `<${tagName}>`; + class Failing extends HTMLElement { + constructor() { + if (container) { + throw new Error('before super'); + } + super(); + } + } + captureReportedErrors(() => customElements.define(tagName, Failing)); + expect(Object.getPrototypeOf(container.firstChild)).to.equal( + HTMLElement.prototype + ); + container.remove(); + }); +}); + +describe('Registries kept when moved', () => { + it('a node parsed into a fragment keeps the document registry when moved into a scoped root', () => { + const tagName = getTestTagName(); + const registry = new CustomElementRegistry(); + const fragment = document.createDocumentFragment(); + fragment.append(document.createElement('div')); + fragment.firstChild.insertAdjacentHTML( + 'afterend', + `<${tagName}>` + ); + const parsed = fragment.lastChild; + document + .createElement('div') + .attachShadow({mode: 'open', customElementRegistry: registry}) + .append(fragment); + expect(parsed.customElementRegistry).to.equal(customElements); + }); +}); + describe('Constructing with new', () => { it('an element constructed with new keeps the global registry when moved into a scoped root', () => { const tagName = getTestTagName(); @@ -2455,6 +2495,29 @@ describe('Constructors', () => { }); describe('Forms', () => { + // Note, the tag's first definition fixes that its stand-in isn't + // form-associated (see the README). + it("upgrading to a form-associated class doesn't throw when the tag's stand-in isn't form-associated", () => { + const tagName = getTestTagName(); + customElements.define(tagName, class extends HTMLElement {}); + const registry = new CustomElementRegistry(); + registry.define( + tagName, + class extends HTMLElement { + static formAssociated = true; + } + ); + const form = document.body.appendChild(document.createElement('form')); + const root = form + .appendChild(document.createElement('div')) + .attachShadow({mode: 'open', customElementRegistry: registry}); + const errors = captureReportedErrors(() => { + root.innerHTML = `<${tagName}>`; + }); + form.remove(); + expect(errors).to.deep.equal([]); + }); + // Note, as natively on upgrade. it('upgrading tells a form-associated element its form and that it is disabled', () => { const tagName = getTestTagName(); From f333b56e7cf8488bfb38194cb543d428977d87b7 Mon Sep 17 00:00:00 2001 From: Steve Orvell Date: Sun, 11 Oct 2026 05:43:06 -0700 Subject: [PATCH 06/11] Review cleanup: read registries directly, drop failed definitions, unpaired fragment copies, comments Internal code calls getRegistry rather than the public getter. A failed upgrade removes the element's definition, as natively, so isFailed checks go and a failed element can attach a shadow root. A fragment or document copy that can't be paired keeps its elements' positional registries instead of null. Removes the adoptedCallback registry change (patched APIs already do it), an unused interface augmentation and other small redundancies. Fixes stale comments. --- .../src/scoped-custom-element-registry.ts | 208 +++++++----------- .../test/behavior.test.js | 50 +++++ 2 files changed, 128 insertions(+), 130 deletions(-) diff --git a/packages/scoped-custom-element-registry/src/scoped-custom-element-registry.ts b/packages/scoped-custom-element-registry/src/scoped-custom-element-registry.ts index d34c0219..4bd2dabb 100644 --- a/packages/scoped-custom-element-registry/src/scoped-custom-element-registry.ts +++ b/packages/scoped-custom-element-registry/src/scoped-custom-element-registry.ts @@ -144,11 +144,6 @@ interface CustomHTMLElement { ): void; } -interface CustomElementRegistry { - _getDefinition(tagName: string): CustomElementDefinition | undefined; - initialize: (node: Node) => void; -} - interface CustomElementDefinition { elementClass: CustomElementConstructor; tagName: string; @@ -169,13 +164,11 @@ interface CustomElementDefinition { observedAttributes: Set; disableShadow?: boolean; disableInternals?: boolean; - // Whether the tag's stand-in is form-associated (see `readDefinition`). + // Whether the tag's stand-in is form-associated (see `define`). standInFormAssociated?: boolean; - /** - * The class that's registered on the global custom element registry for this - * element definition. Only present if this definition is registered on the - * global registry, though all definitions do have a standin. - */ + // The tag's stand-in class, which `new` constructs. Only set for a + // definition in the global registry, the only one whose class `new` can + // construct. standInClass?: CustomElementConstructor; } @@ -666,7 +659,7 @@ type ParametersOf< // Note, native define constructs the stand-ins of existing elements, // which are customized when this entry point returns. if (!standInClass) { - defineStandInEverywhere(tagName, definition.standInFormAssociated!); + defineStandInEverywhere(tagName, definition.standInFormAssociated); standInClass = nativeGet.call(nativeRegistry, tagName); } this._definitionsByTag.set(tagName, definition); @@ -764,9 +757,8 @@ type ParametersOf< } } - // Note, this does *not* initialize the tree but just provokes upgrade - // and since the element may already have been natively upgraded, - // this must be done manually. + // Upgrades the elements in the tree that use this registry; as natively, + // others are left as they are. upgrade(root: Node) { withDeferredUpgrades(() => { // Note, as natively, elements the browser hasn't constructed are @@ -848,16 +840,15 @@ type ParametersOf< // Upgrades the element if this registry defines it, otherwise has it wait // for a definition. _upgradeOrWait(element: HTMLElement, definition?: CustomElementDefinition) { - const registry = element['customElementRegistry']; - const canUpgrade = registry === this; - if (!canUpgrade) { + if (getRegistry(element) !== this) { return; } definition ??= this._getDefinition(element.localName); if (definition !== undefined) { - customize(element, definition!); + customize(element, definition); } else if (element.localName.includes('-')) { - // Note, so once defined, it's upgraded. + // Note, so once defined, it's customized as an upgrade (see + // `constructedDirectly`). constructedDirectly.delete(element); // Note, only custom names can be defined. A connected stand-in waits // for a definition here (its disconnected callback stops waiting, and @@ -879,7 +870,7 @@ type ParametersOf< if ( !this._isScoped && (node.nodeType === Node.DOCUMENT_NODE || - node.ownerDocument?.['customElementRegistry'] !== this) + getRegistry(node.ownerDocument!) !== this) ) { throw new DOMException( `Failed to execute 'initialize' on 'CustomElementRegistry': a global registry can only initialize nodes in its document`, @@ -935,10 +926,9 @@ type ParametersOf< })); // User extends this HTMLElement, which returns the CE being upgraded window.HTMLElement = (function HTMLElement(this: HTMLElement) { - // Upgrading case: the StandInElement constructor was run by the browser's - // native custom elements and we're in the process of running the - // "constructor-call trick" on the natively constructed instance, so just - // return that here + // Customizing: the browser constructed the stand-in, and the user's + // constructor is being called on it (the "constructor-call trick"), so + // it's returned here. // Note, `this` has the constructed class's prototype. (The build compiles // to ES5, which can't express `new.target`.) if ( @@ -960,9 +950,8 @@ type ParametersOf< Object.setPrototypeOf(instance, Object.getPrototypeOf(this)); return instance; } - // Construction case: we need to construct the StandInElement and return - // it; note the current spec proposal only allows new'ing the constructor - // of elements registered with the global registry + // Constructing with `new`: a stand-in is constructed and returned. As + // natively, only a class defined in the global registry can be. const definition = globalDefinitionForConstructor.get( this.constructor as CustomElementConstructor ); @@ -981,10 +970,9 @@ type ParametersOf< // Note, as natively, a constructed element has the global registry. registryForNode.set(instance, globalCustomElementRegistry); // Note, the browser didn't construct a stand-in, so this element is - // counted here if it may be left out of its form (see `isFormControl`). + // noted here if it may be left out of its form (see `isFormControl`). if (definition.standInFormAssociated && !definition['formAssociated']) { - excludableFormElements.add(instance); - current.excludableFormElementCount++; + addExcludableFormElement(instance); } return instance; } as unknown) as typeof HTMLElement; @@ -1007,12 +995,15 @@ type ParametersOf< // Creates the stand-in class the browser knows for a tag, which delegates to // the definition of each element's own registry. + // Note, its callbacks call the user's directly, since the browser reports + // their errors. Callbacks the polyfill calls itself go through + // `invokeCallback`, which reports them. const standInClasses = new WeakSet(); const createStandInClass = ( formAssociated: boolean ): CustomElementConstructor => { const standInClass = (class ScopedCustomElementBase { - // Note, this can't vary by registry (see `readDefinition`). + // Note, this can't vary by registry (see `define`). static get ['formAssociated']() { return formAssociated; } @@ -1047,8 +1038,7 @@ type ParametersOf< constructedDirectly.add(instance); } if (formAssociated) { - excludableFormElements.add(instance); - current.excludableFormElementCount++; + addExcludableFormElement(instance); } currentQueue().add(instance); if (!shouldDeferUpgrade()) { @@ -1063,11 +1053,7 @@ type ParametersOf< ) { const definition = definitionForElement.get(this); if (definition) { - // Delegate out to user callback - if (!isFailed(this)) { - definition.connectedCallback && - definition.connectedCallback.apply(this, args); - } + definition.connectedCallback?.apply(this, args); } else { // Note, a newly constructed element is still queued, and is left to // the flush. Otherwise, it upgrades via its current registry (which @@ -1076,9 +1062,7 @@ type ParametersOf< if (isUpgradeQueued(this)) { return; } - (this[ - 'customElementRegistry' - ] as ShimmedCustomElementsRegistry | null)?._upgradeOrWait(this); + getRegistry(this)?._upgradeOrWait(this); } } @@ -1088,11 +1072,7 @@ type ParametersOf< ) { const definition = definitionForElement.get(this); if (definition) { - // Delegate out to user callback - if (!isFailed(this)) { - definition.disconnectedCallback && - definition.disconnectedCallback.apply(this, args); - } + definition.disconnectedCallback?.apply(this, args); } else { // Note, so a disconnected element isn't kept alive. Its registry // hasn't changed since it started waiting: `initialize` only changes @@ -1110,7 +1090,7 @@ type ParametersOf< // element is connected, so it needs nothing. ['connectedMoveCallback'](this: HTMLElement) { const definition = definitionForElement.get(this); - if (!definition || isFailed(this)) { + if (!definition) { return; } if (definition['connectedMoveCallback']) { @@ -1125,24 +1105,9 @@ type ParametersOf< this: HTMLElement, ...args: ParametersOf ) { - // Note, if this has a null or global registry, it gets the registry - // of the document into which it's adopted (see `adoptRegistries`). As - // native does, it upgrades when connected, not here. - const current = this[ - 'customElementRegistry' - ] as ShimmedCustomElementsRegistry | null; - if (current === null || !current._isScoped) { - registryForNode.set( - this, - this.ownerDocument[ - 'customElementRegistry' - ] as ShimmedCustomElementsRegistry | null - ); - } - const definition = definitionForElement.get(this); - if (!isFailed(this)) { - definition?.adoptedCallback?.apply(this, args); - } + // Note, its registry was already changed by the patched API that + // adopted it (see `adoptRegistries`). + definitionForElement.get(this)?.adoptedCallback?.apply(this, args); } // Form-associated custom elements lifecycle methods @@ -1235,13 +1200,11 @@ type ParametersOf< // The browser only knows the stand-in class, whose observed attributes can't // vary by registry, so `attributeChangedCallback` is approximated by // patching `setAttribute`, `removeAttribute` and `toggleAttribute` once, - // calling the callback of the element's own definition. Note, an element - // which failed to upgrade gets no further reactions. + // calling the callback of the element's own definition. const observingDefinition = (element: Element, name: string) => { const definition = definitionForElement.get(element as HTMLElement); return definition?.attributeChangedCallback && - definition.observedAttributes.has(String(name).toLowerCase()) && - !isFailed(element as HTMLElement) + definition.observedAttributes.has(String(name).toLowerCase()) ? definition : undefined; }; @@ -1320,7 +1283,8 @@ type ParametersOf< () => new WeakSet() ); - // Elements which failed to upgrade get no further reactions. + // Elements which failed to upgrade, so are never customized again. Their + // definition is removed (see `customize`), so they get no reactions. const failedElements = shared( 'failedElements', () => new WeakSet() @@ -1341,15 +1305,12 @@ type ParametersOf< current.directConstructions--; } }; - const isFailed = (element: HTMLElement) => failedElements.has(element); // The definition of an element customized with a form-associated class. As // natively, only such an element gets form callbacks and is a form control. const getFormAssociatedDefinition = (element: Element) => { const definition = definitionForElement.get(element as HTMLElement); - return definition?.['formAssociated'] && !isFailed(element as HTMLElement) - ? definition - : undefined; + return definition?.['formAssociated'] ? definition : undefined; }; // Stand-ins that may be left out of their form's collections (see @@ -1361,6 +1322,10 @@ type ParametersOf< 'excludableFormElements', () => new WeakSet() ); + const addExcludableFormElement = (element: HTMLElement) => { + excludableFormElements.add(element); + current.excludableFormElementCount++; + }; // Note, as natively, an error from a constructor or callback is reported // rather than thrown, so the remaining reactions still run. Only a failed @@ -1397,9 +1362,7 @@ type ParametersOf< queue.forEach((instance) => { queue.delete(instance); try { - (instance[ - 'customElementRegistry' - ] as ShimmedCustomElementsRegistry | null)?._upgradeOrWait(instance); + getRegistry(instance)?._upgradeOrWait(instance); } catch (e) { reportReactionError(e); } @@ -1538,7 +1501,7 @@ type ParametersOf< ) => { // Note, as natively, an element is customized at most once, and never // after its upgrade failed. - if (definitionForElement.has(instance) || isFailed(instance)) { + if (definitionForElement.has(instance) || failedElements.has(instance)) { return; } // Note, an element the browser hasn't constructed is left to it; it's @@ -1578,8 +1541,10 @@ type ParametersOf< ); } } catch (e) { - // Note, an element whose constructor throws gets no further reactions, - // and, as natively, isn't defined. + // Note, as natively, an element whose constructor throws has no + // definition, so gets no further reactions, and is never customized + // again. + definitionForElement.delete(instance); failedElements.add(instance); throw e; } finally { @@ -1631,7 +1596,7 @@ type ParametersOf< if ( registry && !(registry as ShimmedCustomElementsRegistry)._isScoped && - registry !== doc['customElementRegistry'] + registry !== getRegistry(doc) ) { throw new DOMException( `Failed to execute '${method}': a global registry must be the document's registry`, @@ -1648,6 +1613,12 @@ type ParametersOf< // The shadow root exposed to a host's internals, without recording its // registry. + const internalsShadowRootDescriptor = window['ElementInternals'] + ? Object.getOwnPropertyDescriptor( + window['ElementInternals'].prototype, + 'shadowRoot' + ) + : undefined; const getShadowRootFromInternals = (host: Element) => { const internals = internalsForElement.get(host as HTMLElement); return internals @@ -1713,20 +1684,14 @@ type ParametersOf< // `getRegistry`). registryForNode.set( shadowRoot, - (registry !== undefined - ? registry - : this.ownerDocument[ - 'customElementRegistry' - ]) as ShimmedCustomElementsRegistry | null + registry !== undefined + ? (registry as ShimmedCustomElementsRegistry | null) + : getRegistry(this.ownerDocument) ); if (registry !== undefined) { - // for back compat, set both `registry` and `customElements` - (shadowRoot as ShadowRootInitWithSettableCustomElements)[ - 'registry' - ] = registry; - (shadowRoot as ShadowRootInitWithSettableCustomElements)[ - 'customElements' - ] = registry; + // Note, for back compat, `registry` and `customElements` are set too. + shadowRoot['registry'] = registry; + shadowRoot['customElements'] = registry; } return shadowRoot; }; @@ -1828,12 +1793,6 @@ type ParametersOf< tagName: string, options?: ElementCreationOptions ) => Element; - const createElementNS = nativeCreateElementNS as ( - this: Document, - namespace: string | null, - tagName: string, - options?: ElementCreationOptions - ) => Element; // https://dom.spec.whatwg.org/#flatten-element-creation-options // Returns the registry to use and the options to pass to the native method. @@ -1846,14 +1805,14 @@ type ParametersOf< string | ElementCreationOptions | undefined ] => { if (typeof options !== 'object' || options === null) { - return [doc['customElementRegistry'], options]; + return [getRegistry(doc), options]; } const { 'customElementRegistry': optionsRegistry, ...nativeOptions } = options; if (optionsRegistry === undefined) { - return [doc['customElementRegistry'], nativeOptions]; + return [getRegistry(doc), nativeOptions]; } if (nativeOptions['is'] !== undefined) { throw new DOMException( @@ -1920,7 +1879,7 @@ type ParametersOf< this, options, 'createElementNS', - (o) => createElementNS.call(this, namespace, tagName, o) + (o) => nativeCreateElementNS.call(this, namespace, tagName, o) ); }, enumerable: true, @@ -1954,8 +1913,7 @@ type ParametersOf< validateRegistry(optionsRegistry, this, 'importNode'); // Note, the provided registry is used only as a fallback to set when // the imported node's registry is null. - const fallbackRegistry = - optionsRegistry ?? this['customElementRegistry']; + const fallbackRegistry = optionsRegistry ?? getRegistry(this); return withDeferredUpgrades(() => cloneWithRegistries( node, @@ -2265,7 +2223,7 @@ type ParametersOf< registryForNode.set( copyElement, getCloneRegistry( - sourceElement['customElementRegistry'], + getRegistry(sourceElement), copyElement, fallbackRegistry ) @@ -2310,10 +2268,7 @@ type ParametersOf< sourceRoot: ShadowRoot, copyRoot: ShadowRoot ) => { - const registry = getCloneRegistry( - sourceRoot['customElementRegistry'], - copyRoot - ); + const registry = getCloneRegistry(getRegistry(sourceRoot), copyRoot); const sources = elementsOf(sourceRoot, true); const copies = elementsOf(copyRoot, true); if (sources.length !== copies.length) { @@ -2366,7 +2321,7 @@ type ParametersOf< for (const [host, source] of unreachableCopyRoots) { clonedShadowRootRegistries.set( host, - getCloneRegistry(source['customElementRegistry'], host) + getCloneRegistry(getRegistry(source), host) ); } unreachableCopyRoots.clear(); @@ -2394,17 +2349,15 @@ type ParametersOf< sources.forEach((source, i) => pairClonedElement(source, copies[i], fallbackRegistry) ); - } else { + } else if (node.nodeType === Node.ELEMENT_NODE) { // Note, the copy can't be paired by index (e.g. a customized built-in's // constructor changed the source during the native clone), so it gets - // the source's registry throughout. + // the source's registry throughout. A copied fragment or document is + // left unrecorded, so its elements get their registries from where + // they are (see `getRegistry`). setRegistryForSubtree( clone, - getCloneRegistry( - node.nodeType === Node.ELEMENT_NODE ? getRegistry(node) : null, - clone, - fallbackRegistry - ) + getCloneRegistry(getRegistry(node), clone, fallbackRegistry) ); } pairUnreachableCopyRoots(); @@ -2614,13 +2567,8 @@ type ParametersOf< }); // Note, a custom element host reaches its closed declarative root, with its - // content, via `attachInternals().shadowRoot`. - const internalsShadowRootDescriptor = window['ElementInternals'] - ? Object.getOwnPropertyDescriptor( - window['ElementInternals'].prototype, - 'shadowRoot' - ) - : undefined; + // content, via `attachInternals().shadowRoot`, so the root's registry is + // recorded there. if (internalsShadowRootDescriptor?.get) { Object.defineProperty(window['ElementInternals'].prototype, 'shadowRoot', { ...internalsShadowRootDescriptor, @@ -2675,7 +2623,7 @@ type ParametersOf< if ( definition?.disableInternals || internalsAttachedByUser.has(this) || - (standInElements.has(this) && (!definition || isFailed(this))) + (standInElements.has(this) && !definition) ) { throw new DOMException( `Failed to execute 'attachInternals' on 'HTMLElement': ElementInternals for the specified element was already attached, is disabled, or the element isn't a custom element.`, @@ -2691,8 +2639,8 @@ type ParametersOf< } if ( - !!window['ElementInternals'] && - !!window['ElementInternals'].prototype['setFormValue'] + window['ElementInternals'] && + window['ElementInternals'].prototype['setFormValue'] ) { // Note, the browser only knows the stand-in, so the form members of an // element's internals are checked here: as natively, they only work for @@ -2738,12 +2686,12 @@ type ParametersOf< ]) { const descriptor = Object.getOwnPropertyDescriptor(proto, getter); if (descriptor?.get) { - const nativeGet = descriptor.get; + const nativeGetter = descriptor.get; Object.defineProperty(proto, getter, { ...descriptor, get(this: ElementInternals) { checkFormAssociated(this, getter); - return nativeGet.call(this); + return nativeGetter.call(this); }, }); } diff --git a/packages/scoped-custom-element-registry/test/behavior.test.js b/packages/scoped-custom-element-registry/test/behavior.test.js index ba554af8..78de96c4 100644 --- a/packages/scoped-custom-element-registry/test/behavior.test.js +++ b/packages/scoped-custom-element-registry/test/behavior.test.js @@ -265,6 +265,28 @@ describe('Prototype on a failed upgrade', () => { }); }); +describe('Definition after a failed upgrade', () => { + it('an element whose constructor threw can attach a shadow root, though its class disables them', () => { + const tagName = getTestTagName(); + const container = document.body.appendChild(document.createElement('div')); + container.innerHTML = `<${tagName}>`; + class Failing extends HTMLElement { + static get disabledFeatures() { + return ['shadow']; + } + constructor() { + super(); + throw new Error('constructor'); + } + } + captureReportedErrors(() => customElements.define(tagName, Failing)); + expect(() => + container.firstChild.attachShadow({mode: 'open'}) + ).not.to.throw(); + container.remove(); + }); +}); + describe('Registries kept when moved', () => { it('a node parsed into a fragment keeps the document registry when moved into a scoped root', () => { const tagName = getTestTagName(); @@ -834,6 +856,34 @@ describe('Cloning', () => { expect(template.cloneNode(false).content.childNodes.length).to.equal(0); }); + itWithCustomizedBuiltIns( + 'a fragment copy that can\u2019t be paired keeps the global registry', + () => { + const tagName = getTestTagName(); + let source; + class Button extends HTMLButtonElement { + constructor() { + super(); + // Note, adds an element to the source after the native clone, so + // the copy can't be paired by index. + source?.append(document.createElement('div')); + } + } + customElements.define(tagName, Button, {extends: 'button'}); + const fragment = document.createDocumentFragment(); + fragment.append( + document.createElement('button', {is: tagName}), + document.createElement('div') + ); + source = fragment; + const copy = fragment.cloneNode(true); + source = undefined; + expect(fragment.children.length).to.equal(3); + expect(copy.children.length).to.equal(2); + expect(copy.children[1].customElementRegistry).to.equal(customElements); + } + ); + it('keeps each node\u2019s registry in a mixed tree', () => { const registry = new CustomElementRegistry(); const {tagName, Logged} = defineLogged(registry); From 803294ce1542f08441bb30652e58d4be308d08c4 Mon Sep 17 00:00:00 2001 From: Steve Orvell Date: Sun, 11 Oct 2026 05:57:14 -0700 Subject: [PATCH 07/11] Review cleanup: remove a redundant registry check and an unreachable fallback --- .../src/scoped-custom-element-registry.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/scoped-custom-element-registry/src/scoped-custom-element-registry.ts b/packages/scoped-custom-element-registry/src/scoped-custom-element-registry.ts index 4bd2dabb..c3f5e45f 100644 --- a/packages/scoped-custom-element-registry/src/scoped-custom-element-registry.ts +++ b/packages/scoped-custom-element-registry/src/scoped-custom-element-registry.ts @@ -388,13 +388,13 @@ type ParametersOf< }); // Upgrades each element in `node`'s subtree whose registry is `registry`, - // or has it wait for a definition. + // or has it wait for a definition (see `_upgradeOrWait`). const upgradeSubtree = ( node: Node, registry: ShimmedCustomElementsRegistry ) => forEachInTree(node, (n) => { - if (n.nodeType === Node.ELEMENT_NODE && getRegistry(n) === registry) { + if (n.nodeType === Node.ELEMENT_NODE) { registry._upgradeOrWait(n as HTMLElement); } }); @@ -2149,7 +2149,8 @@ type ParametersOf< ) { const where = String(position).toLowerCase(); const isOutside = where === 'beforebegin' || where === 'afterend'; - const parent = (isOutside ? this.parentNode : this) ?? this; + // Note, without a parent, native throws for an outside position. + const parent = isOutside ? this.parentNode! : this; const start = where === 'beforebegin' ? this.previousSibling From 87733ecfda4924afe0db96e9d12361313f6628c9 Mon Sep 17 00:00:00 2001 From: Steve Orvell Date: Fri, 23 Oct 2026 07:09:53 -0700 Subject: [PATCH 08/11] WPT: update known failures; the review fixes pass three more tests --- .../test/wpt/baseline/chromium.json | 5 ----- .../test/wpt/baseline/firefox.json | 5 ----- .../test/wpt/baseline/webkit.json | 5 ----- 3 files changed, 15 deletions(-) diff --git a/packages/scoped-custom-element-registry/test/wpt/baseline/chromium.json b/packages/scoped-custom-element-registry/test/wpt/baseline/chromium.json index 1d0c4e9c..ed1e12ef 100644 --- a/packages/scoped-custom-element-registry/test/wpt/baseline/chromium.json +++ b/packages/scoped-custom-element-registry/test/wpt/baseline/chromium.json @@ -1,13 +1,8 @@ { - "custom-elements/registries/Document-customElementRegistry.html": [ - "customElementRegistry on a document of a disconnected iframe should return contentWindow.customElements" - ], "custom-elements/registries/Element-customElementRegistry-exceptions.html": [ "customElementRegistry on a failed custom element created by parser should return the specified custom regsitry" ], "custom-elements/registries/define.html": [ - "Element names: defining an element named null should throw a SyntaxError", - "Element names: defining an element named undefined should throw a SyntaxError", "If constructor.prototype throws, should rethrow" ], "custom-elements/registries/initial-about-blank.window.html": [ diff --git a/packages/scoped-custom-element-registry/test/wpt/baseline/firefox.json b/packages/scoped-custom-element-registry/test/wpt/baseline/firefox.json index 5b3c5588..fbc9c7d4 100644 --- a/packages/scoped-custom-element-registry/test/wpt/baseline/firefox.json +++ b/packages/scoped-custom-element-registry/test/wpt/baseline/firefox.json @@ -1,13 +1,8 @@ { - "custom-elements/registries/Document-customElementRegistry.html": [ - "customElementRegistry on a document of a disconnected iframe should return contentWindow.customElements" - ], "custom-elements/registries/Element-customElementRegistry-exceptions.html": [ "customElementRegistry on a failed custom element created by parser should return the specified custom regsitry" ], "custom-elements/registries/define.html": [ - "Element names: defining an element named null should throw a SyntaxError", - "Element names: defining an element named undefined should throw a SyntaxError", "If constructor.prototype throws, should rethrow" ], "custom-elements/registries/initial-about-blank.window.html": [ diff --git a/packages/scoped-custom-element-registry/test/wpt/baseline/webkit.json b/packages/scoped-custom-element-registry/test/wpt/baseline/webkit.json index 1d0c4e9c..ed1e12ef 100644 --- a/packages/scoped-custom-element-registry/test/wpt/baseline/webkit.json +++ b/packages/scoped-custom-element-registry/test/wpt/baseline/webkit.json @@ -1,13 +1,8 @@ { - "custom-elements/registries/Document-customElementRegistry.html": [ - "customElementRegistry on a document of a disconnected iframe should return contentWindow.customElements" - ], "custom-elements/registries/Element-customElementRegistry-exceptions.html": [ "customElementRegistry on a failed custom element created by parser should return the specified custom regsitry" ], "custom-elements/registries/define.html": [ - "Element names: defining an element named null should throw a SyntaxError", - "Element names: defining an element named undefined should throw a SyntaxError", "If constructor.prototype throws, should rethrow" ], "custom-elements/registries/initial-about-blank.window.html": [ From 33597a6f2e2c3d4e0e4a252f45648308d6a74d0c Mon Sep 17 00:00:00 2001 From: Steve Orvell Date: Fri, 23 Oct 2026 07:10:13 -0700 Subject: [PATCH 09/11] Docs: changelog for the review fixes; adoption is undone if the move throws --- .../scoped-custom-element-registry/CHANGELOG.md | 16 ++++++++++++++++ .../scoped-custom-element-registry/DESIGN.md | 3 ++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/packages/scoped-custom-element-registry/CHANGELOG.md b/packages/scoped-custom-element-registry/CHANGELOG.md index f49604ca..2fd95c96 100644 --- a/packages/scoped-custom-element-registry/CHANGELOG.md +++ b/packages/scoped-custom-element-registry/CHANGELOG.md @@ -25,6 +25,22 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 null registry - `HTMLElement`, and classes extending it, inherit the statics of `Element` and `Node` (e.g. `ELEMENT_NODE`) +- `new X()` gives the element the global registry +- `importNode` reads its options as WebIDL does: null or an object is a + dictionary, anything else a boolean +- A registry argument that isn't a `CustomElementRegistry` throws a + `TypeError` +- `define`, `get` and `whenDefined` convert the name to a string +- An element is customized at most once; as natively, a failed upgrade clears + its definition, and a constructor that throws before `super()` leaves it an + `HTMLElement` +- `attachInternals` throws for an element that isn't customized +- Form callbacks run on upgrade, and only for form-associated elements +- Initial `attributeChangedCallback`s run in attribute order +- An insertion that throws restores the registries it changed +- `Document` `append`/`prepend`/`replaceChildren`, `CharacterData` + `before`/`after`/`replaceWith`, `textContent`, `innerText`, `outerText` and + `setHTML` keep registries ### Changed diff --git a/packages/scoped-custom-element-registry/DESIGN.md b/packages/scoped-custom-element-registry/DESIGN.md index d3382189..71f8e092 100644 --- a/packages/scoped-custom-element-registry/DESIGN.md +++ b/packages/scoped-custom-element-registry/DESIGN.md @@ -89,7 +89,8 @@ registry. Moving a node into another document changes null and global registries as the spec says, to an _effective global registry_: the new document's global registry, or null if the document's registry is scoped. Scoped registries are -kept. Patched insertion methods apply this before the native move. +kept. Patched insertion methods apply this before the native move, and undo +it if the move throws. The polyfill's state is shared by same-origin windows, so nodes moved between them work as natively. From afc2be8c8c00f023f07205224ba7c62e5bf7e629 Mon Sep 17 00:00:00 2001 From: Steve Orvell Date: Fri, 23 Oct 2026 07:14:26 -0700 Subject: [PATCH 10/11] Review cleanup after the rebase: share internalsToHostMap across windows, adoptRegistries uses forEachInTree, clearer names and comments --- .../src/scoped-custom-element-registry.ts | 100 +++++++++--------- 1 file changed, 52 insertions(+), 48 deletions(-) diff --git a/packages/scoped-custom-element-registry/src/scoped-custom-element-registry.ts b/packages/scoped-custom-element-registry/src/scoped-custom-element-registry.ts index c3f5e45f..c5fe335e 100644 --- a/packages/scoped-custom-element-registry/src/scoped-custom-element-registry.ts +++ b/packages/scoped-custom-element-registry/src/scoped-custom-element-registry.ts @@ -1697,8 +1697,8 @@ type ParametersOf< }; // The global registry of a document's window: this window's for its - // document, otherwise null (a document without a window, or another - // window's, whose registries this copy of the polyfill doesn't know). + // document (even once the window is gone), otherwise its window's if that + // runs the polyfill, else null (e.g. a document without a window). // Note, native registry state is deliberately not consulted, so the // polyfill behaves the same whether or not the browser has support. const getWindowRegistry = (doc: Document) => { @@ -1787,8 +1787,11 @@ type ParametersOf< configurable: true, }; - const {importNode, adoptNode} = Document.prototype; - const createElement = Document.prototype.createElement as ( + const { + importNode: nativeImportNode, + adoptNode: nativeAdoptNode, + } = Document.prototype; + const nativeCreateElement = Document.prototype.createElement as ( this: Document, tagName: string, options?: ElementCreationOptions @@ -1862,7 +1865,7 @@ type ParametersOf< options?: string | ElementCreationOptions ) { return createElementWithRegistry(this, options, 'createElement', (o) => - createElement.call(this, tagName, o) + nativeCreateElement.call(this, tagName, o) ); }, enumerable: true, @@ -1918,7 +1921,7 @@ type ParametersOf< cloneWithRegistries( node, deep, - (n, nodeDeep) => importNode.call(this, n, nodeDeep), + (n, nodeDeep) => nativeImportNode.call(this, n, nodeDeep), fallbackRegistry ) ) as T; @@ -1934,7 +1937,7 @@ type ParametersOf< return withAdoption( this, [node], - () => adoptNode.call(this, node) as T + () => nativeAdoptNode.call(this, node) as T ); }, enumerable: true, @@ -1960,48 +1963,46 @@ type ParametersOf< registry?._isScoped ? null : registry; // Each change is noted in `previous`, so it can be undone. const adoptRegistries = ( - node: Node, + adopted: Node, documentRegistry: ShimmedCustomElementsRegistry | null, - previous: Array<[Node, ShimmedCustomElementsRegistry | null | undefined]>, - isAdoptedNode = true - ) => { - const current = getRegistry(node); - if (current === null || !current._isScoped) { - let registry: ShimmedCustomElementsRegistry | null | undefined; - if (isShadowRoot(node)) { - const keepsNull = - current === null && node.host.hasAttribute(DSD_HOST_ATTRIBUTE); - if (!keepsNull) { - registry = effectiveGlobal(documentRegistry); + previous: Array<[Node, ShimmedCustomElementsRegistry | null | undefined]> + ) => + forEachInTree( + adopted, + (node) => { + const oldRegistry = getRegistry(node); + if (oldRegistry !== null && oldRegistry._isScoped) { + return; } - } else if (node.nodeType === Node.ELEMENT_NODE) { - const parent = node.parentNode; - const fromDocument = - current !== null || - isAdoptedNode || - parent === null || - (parent.nodeType === Node.DOCUMENT_FRAGMENT_NODE && - !isShadowRoot(parent)); - registry = effectiveGlobal( - fromDocument ? documentRegistry : getRegistry(parent) - ); - // Note, the native move disconnects it after this, by which time its - // registry has changed. - current?._stopWaitingForDefinition(node as HTMLElement); - } - if (registry !== undefined) { - previous.push([node, registryForNode.get(node)]); - registryForNode.set(node, registry); - } - } - const shadowRoot = getShadowRoot(node); - if (shadowRoot) { - adoptRegistries(shadowRoot, documentRegistry, previous, false); - } - for (const child of childrenOf(node)) { - adoptRegistries(child, documentRegistry, previous, false); - } - }; + let registry: ShimmedCustomElementsRegistry | null | undefined; + if (isShadowRoot(node)) { + const keepsNull = + oldRegistry === null && node.host.hasAttribute(DSD_HOST_ATTRIBUTE); + if (!keepsNull) { + registry = effectiveGlobal(documentRegistry); + } + } else if (node.nodeType === Node.ELEMENT_NODE) { + const parent = node.parentNode; + const fromDocument = + oldRegistry !== null || + node === adopted || + parent === null || + (parent.nodeType === Node.DOCUMENT_FRAGMENT_NODE && + !isShadowRoot(parent)); + registry = effectiveGlobal( + fromDocument ? documentRegistry : getRegistry(parent) + ); + // Note, the native move disconnects it after this, by which time + // its registry has changed. + oldRegistry?._stopWaitingForDefinition(node as HTMLElement); + } + if (registry !== undefined) { + previous.push([node, registryForNode.get(node)]); + registryForNode.set(node, registry); + } + }, + true + ); // Runs `fn`, which inserts or adopts `nodes` into `doc`, having first given // those from other documents their adopted registries (see @@ -2615,7 +2616,10 @@ type ParametersOf< // so they are returned to the user, emulating native: internals can be // attached once, only to a custom element (one being or already customized, // not failed), and not if the element's class disables them. - const internalsToHostMap = new WeakMap(); + const internalsToHostMap = shared( + 'internalsToHostMap', + () => new WeakMap() + ); if (nativeAttachInternals) { HTMLElement.prototype['attachInternals'] = function (this: HTMLElement) { // Note, the browser only knows the stand-in class, so the user's class From 254007fc655627e53f71026a87e0712548cb28f3 Mon Sep 17 00:00:00 2001 From: Steve Orvell Date: Fri, 23 Oct 2026 07:49:04 -0700 Subject: [PATCH 11/11] Skip the failed-upgrade attachShadow test natively: Chromium keeps the definition --- .../scoped-custom-element-registry/README.md | 5 ++- .../test/behavior.test.js | 41 +++++++++++-------- 2 files changed, 27 insertions(+), 19 deletions(-) diff --git a/packages/scoped-custom-element-registry/README.md b/packages/scoped-custom-element-registry/README.md index 24a19d8b..7da9126b 100644 --- a/packages/scoped-custom-element-registry/README.md +++ b/packages/scoped-custom-element-registry/README.md @@ -134,8 +134,9 @@ Tests of these are skipped when the browser's native support is used. one; - a copy of an element with a global registry, in a document with a scoped registry, gets that scoped registry instead of null; - - `ElementInternals.setFormValue` works for an element whose constructor - failed, which isn't form-associated; + - an element whose constructor failed keeps its definition, so + `ElementInternals.setFormValue` still works for it, and it can't attach a + shadow root if its class disables them; - adopting a null-registry element gives it the document's registry, instead of its new parent's (as an effective global registry); - adopting a host gives its shadow root a null registry but leaves the diff --git a/packages/scoped-custom-element-registry/test/behavior.test.js b/packages/scoped-custom-element-registry/test/behavior.test.js index 78de96c4..e6c0b257 100644 --- a/packages/scoped-custom-element-registry/test/behavior.test.js +++ b/packages/scoped-custom-element-registry/test/behavior.test.js @@ -266,25 +266,32 @@ describe('Prototype on a failed upgrade', () => { }); describe('Definition after a failed upgrade', () => { - it('an element whose constructor threw can attach a shadow root, though its class disables them', () => { - const tagName = getTestTagName(); - const container = document.body.appendChild(document.createElement('div')); - container.innerHTML = `<${tagName}>`; - class Failing extends HTMLElement { - static get disabledFeatures() { - return ['shadow']; - } - constructor() { - super(); - throw new Error('constructor'); + // Note, Chromium keeps the definition of an element whose constructor + // failed (see README). + itWithPolyfill( + 'an element whose constructor threw can attach a shadow root, though its class disables them', + () => { + const tagName = getTestTagName(); + const container = document.body.appendChild( + document.createElement('div') + ); + container.innerHTML = `<${tagName}>`; + class Failing extends HTMLElement { + static get disabledFeatures() { + return ['shadow']; + } + constructor() { + super(); + throw new Error('constructor'); + } } + captureReportedErrors(() => customElements.define(tagName, Failing)); + expect(() => + container.firstChild.attachShadow({mode: 'open'}) + ).not.to.throw(); + container.remove(); } - captureReportedErrors(() => customElements.define(tagName, Failing)); - expect(() => - container.firstChild.attachShadow({mode: 'open'}) - ).not.to.throw(); - container.remove(); - }); + ); }); describe('Registries kept when moved', () => {