diff --git a/src/components/button/button-base.ts b/src/components/button/button-base.ts index 90b61ee18..a04f1f3b6 100644 --- a/src/components/button/button-base.ts +++ b/src/components/button/button-base.ts @@ -18,7 +18,6 @@ import { cancelDeferredFocus, deferFocus } from '#internals/deferred-focus.js'; import { EventEmitterMixin } from '#internals/mixins/event-emitter.js'; import { HostAriaMixin } from '#internals/mixins/host-aria.js'; import { partMap } from '#internals/part-map.js'; -import { getElementByIdFromRoot } from '#internals/utils/dom.js'; import { runDefaultAction } from '#internals/utils/events.js'; import { bindIf } from '#internals/utils/lit.js'; @@ -67,7 +66,10 @@ export abstract class IgcButtonBaseComponent extends EventEmitterMixin this._commandfor, + (element) => { + this.commandForElement = element; + } ); private _disabled = false; @@ -171,7 +173,6 @@ export abstract class IgcButtonBaseComponent extends EventEmitterMixin { - if (this._commandfor) { - this._resolveCommandFor(); - } - }); - } - //#endregion //#region Event handlers @@ -269,18 +254,6 @@ export abstract class IgcButtonBaseComponent extends EventEmitterMixin): void { - if (this._commandfor && ids.has(this._commandfor)) { - this._resolveCommandFor(); - } - } - - /** Looks up the `commandfor` target again and renders with it. */ - private _resolveCommandFor(): void { - this._commandForElement = getElementByIdFromRoot(this, this._commandfor!); - this.requestUpdate(); - } - //#region Public API /* alternateName: focusComponent */ diff --git a/src/internals/controllers/async-consumer.spec.ts b/src/internals/controllers/async-consumer.spec.ts index e0518e6f4..17d2d96d8 100644 --- a/src/internals/controllers/async-consumer.spec.ts +++ b/src/internals/controllers/async-consumer.spec.ts @@ -8,6 +8,18 @@ const testContext = createContext(Symbol('async-consumer-test')); type Consumer = LitElement & { received: string[] }; +function defineProvider(tag: string): void { + customElements.define( + tag, + class extends LitElement { + public provider = new ContextProvider(this, { + context: testContext, + initialValue: 'value', + }); + } + ); +} + describe('Async context consumer', () => { const consumerTag = defineCE( class extends LitElement { @@ -34,15 +46,7 @@ describe('Async context consumer', () => { const consumers = [...container.querySelectorAll(consumerTag)]; await Promise.all(consumers.map(elementUpdated)); - customElements.define( - providerTag, - class extends LitElement { - public provider = new ContextProvider(this, { - context: testContext, - initialValue: 'value', - }); - } - ); + defineProvider(providerTag); for (const each of consumers) { expect(each.received).to.eql(['value']); @@ -51,4 +55,45 @@ describe('Async context consumer', () => { // An earlier spec can add the shared listener first. expect(listen.withArgs('context-provider').callCount).to.be.at.most(1); }); + + it('asks again only for the waiting consumers inside the provider', async () => { + const providerTag = 'igc-async-consumer-test-scoped-provider'; + const container = await fixture( + `
<${providerTag}><${consumerTag}><${consumerTag}>
` + ); + const [inside, outside] = container.querySelectorAll(consumerTag); + await Promise.all([elementUpdated(inside), elementUpdated(outside)]); + + const requests = sinon.spy(); + outside.addEventListener('context-request', requests); + + defineProvider(providerTag); + + expect(inside.received).to.eql(['value']); + expect(outside.received).to.be.empty; + expect(requests.called).to.be.false; + }); + + it('answers a waiting consumer slotted into a provider in a closed shadow root', async () => { + const providerTag = 'igc-async-consumer-test-closed-provider'; + const wrapperTag = defineCE( + class extends HTMLElement { + constructor() { + super(); + this.attachShadow({ + mode: 'closed', + }).innerHTML = `<${providerTag}>`; + } + } + ); + const wrapper = await fixture( + `<${wrapperTag}><${consumerTag}>` + ); + const consumer = wrapper.querySelector(consumerTag)!; + await elementUpdated(consumer); + + defineProvider(providerTag); + + expect(consumer.received).to.eql(['value']); + }); }); diff --git a/src/internals/controllers/async-consumer.ts b/src/internals/controllers/async-consumer.ts index a42473877..2eb02c6b6 100644 --- a/src/internals/controllers/async-consumer.ts +++ b/src/internals/controllers/async-consumer.ts @@ -11,7 +11,8 @@ type AsyncContextOptions> = { callback?: (value: ContextType, dispose?: () => void) => void; }; -type Retry = (context: unknown) => void; +type ProviderEvent = HTMLElementEventMap['context-provider']; +type Retry = (event: ProviderEvent) => void; /** The unanswered consumers of each document. */ const waiting = new WeakMap>(); @@ -22,13 +23,23 @@ const waiting = new WeakMap>(); * remove. */ function retryWaiting(event: Event): void { - const { context } = event as Event & { context?: unknown }; - for (const retry of waiting.get(event.currentTarget as Document) ?? []) { - retry(context); + retry(event as ProviderEvent); } } +/** + * Whether a request of `consumer` can reach `provider`. In one tree, it + * reaches only the ancestors of the consumer. Across trees, slots decide, and + * a closed shadow root hides them. + */ +function mayReach(consumer: Element, provider: Element): boolean { + return ( + consumer.getRootNode() !== provider.getRootNode() || + provider.contains(consumer) + ); +} + function createWaiting(document: Document): Set { document.addEventListener('context-provider', retryWaiting); return new Set(); @@ -59,9 +70,13 @@ class AsyncContextConsumer< return this._consumer?.value; } - private readonly _retry: Retry = (context) => { - if (context === this._options.context) { - this._consumer?.hostConnected(); + private readonly _retry: Retry = ({ context, contextTarget }) => { + if ( + this._consumer && + context === this._options.context && + mayReach(this._host, contextTarget) + ) { + this._consumer.hostConnected(); } }; diff --git a/src/internals/controllers/id-resolver.spec.ts b/src/internals/controllers/id-resolver.spec.ts index a5bca7ff8..c8114921e 100644 --- a/src/internals/controllers/id-resolver.spec.ts +++ b/src/internals/controllers/id-resolver.spec.ts @@ -13,34 +13,45 @@ import { type IdRefResolverController, } from './id-resolver.js'; +const ID = 'idref-target'; + type HostInstance = LitElement & { resolver: IdRefResolverController; - receivedIds: Set | null; - callCount: number; + watched: string | null; + received: (Element | null)[]; capturedThis: unknown; }; -type SecondHostInstance = LitElement & { - resolver: IdRefResolverController; - callCount: number; -}; +function createTarget(id = ID): HTMLDivElement { + const element = document.createElement('div'); + element.id = id; + return element; +} describe('IdRefResolverController', () => { let tag: string; let instance: HostInstance; + /** Elements that a test added to the document, removed after each test. */ + let added: Element[]; + + function add(element: T, parent: Node = document.body): T { + parent.appendChild(element); + added.push(element); + return element; + } before(() => { tag = defineCE( class extends LitElement { - public receivedIds: Set | null = null; - public callCount = 0; + public watched: string | null = ID; + public received: (Element | null)[] = []; public capturedThis: unknown = null; public readonly resolver = addIdRefResolver( this, - function (this: LitElement, ids: Set) { - (this as unknown as HostInstance).receivedIds = new Set(ids); - (this as unknown as HostInstance).callCount++; + () => this.watched, + function (this: LitElement, element: Element | null) { + (this as unknown as HostInstance).received.push(element); (this as unknown as HostInstance).capturedThis = this; this.requestUpdate(); } @@ -50,92 +61,90 @@ describe('IdRefResolverController', () => { }); beforeEach(async () => { + added = []; const tagName = unsafeStatic(tag); instance = await fixture(html`<${tagName}>`); }); - describe('resolve(id)', () => { - let target: HTMLDivElement; + afterEach(() => { + instance.resolver.unobserve(); + // Some tests move the host out of the fixture. + instance.remove(); - afterEach(() => { - target?.remove(); - }); + for (const element of added) { + element.remove(); + } + }); + describe('resolve(id)', () => { it('returns null when no element with that ID exists', () => { expect(getElementByIdFromRoot(instance, 'nonexistent')).to.be.null; }); it('resolves an element present in the document by ID', () => { - target = document.createElement('div'); - target.id = 'resolve-target'; - document.body.appendChild(target); + const target = add(createTarget()); - expect(getElementByIdFromRoot(instance, 'resolve-target')).to.equal( - target - ); + expect(getElementByIdFromRoot(instance, ID)).to.equal(target); }); it('returns null after the element is removed', () => { - target = document.createElement('div'); - target.id = 'removed-target'; - document.body.appendChild(target); + const target = add(createTarget()); - expect(getElementByIdFromRoot(instance, 'removed-target')).to.not.be.null; + expect(getElementByIdFromRoot(instance, ID)).to.not.be.null; target.remove(); - expect(getElementByIdFromRoot(instance, 'removed-target')).to.be.null; + expect(getElementByIdFromRoot(instance, ID)).to.be.null; }); }); describe('observe() / unobserve()', () => { - let added: HTMLDivElement; + it('callback never fires when observe() was not called', async () => { + add(createTarget()); + await elementUpdated(instance); - afterEach(() => { - added?.remove(); - instance.resolver.unobserve(); + expect(instance.received).to.be.empty; }); - it('callback never fires when observe() was not called', async () => { - added = document.createElement('div'); - added.id = 'no-observe'; - document.body.appendChild(added); - await elementUpdated(instance); + it('observe() reports the current element of the id', () => { + const target = add(createTarget()); + instance.resolver.observe(); - expect(instance.callCount).to.equal(0); + expect(instance.received).to.eql([target]); }); - it('observe() is idempotent — calling it twice yields one callback per mutation', async () => { + it('observe() is idempotent — calling it twice yields one callback per change', async () => { instance.resolver.observe(); instance.resolver.observe(); - added = document.createElement('div'); - added.id = 'idempotent'; - document.body.appendChild(added); + const target = add(createTarget()); await elementUpdated(instance); - expect(instance.callCount).to.equal(1); + expect(instance.received).to.eql([target]); + }); + + it('observe() checks a new id while observing', () => { + const other = add(createTarget('idref-other')); + instance.resolver.observe(); + + instance.watched = 'idref-other'; + instance.resolver.observe(); + + expect(instance.received).to.eql([other]); }); it('after unobserve(), callback stops firing', async () => { instance.resolver.observe(); - added = document.createElement('div'); - added.id = 'first-add'; - document.body.appendChild(added); + const target = add(createTarget()); await elementUpdated(instance); - expect(instance.callCount).to.equal(1); + expect(instance.received).to.eql([target]); instance.resolver.unobserve(); - - const second = document.createElement('div'); - second.id = 'second-add'; - document.body.appendChild(second); + target.remove(); await elementUpdated(instance); - expect(instance.callCount).to.equal(1); - - second.remove(); + expect(instance.received).to.eql([target]); }); it('unobserve() is safe when not observing', () => { @@ -144,161 +153,124 @@ describe('IdRefResolverController', () => { }); describe('DOM mutation detection', () => { - let container: HTMLDivElement; + beforeEach(() => instance.resolver.observe()); - beforeEach(() => { - container = document.createElement('div'); - document.body.appendChild(container); - instance.resolver.observe(); - }); - - afterEach(() => { - container.remove(); - instance.resolver.unobserve(); - }); - - it('adding element with an id fires callback with that ID', async () => { - const el = document.createElement('div'); - el.id = 'added-foo'; - container.appendChild(el); + it('reports an added element with the id', async () => { + const target = add(createTarget()); await elementUpdated(instance); - expect(instance.callCount).to.be.greaterThan(0); - expect(instance.receivedIds!.has('added-foo')).to.be.true; + expect(instance.received).to.eql([target]); }); - it('removing element with an id fires callback with that ID', async () => { - const el = document.createElement('div'); - el.id = 'removed-bar'; - container.appendChild(el); + it('reports null after the element with the id is removed', async () => { + const target = add(createTarget()); await elementUpdated(instance); - instance.callCount = 0; - - el.remove(); + target.remove(); await elementUpdated(instance); - expect(instance.callCount).to.be.greaterThan(0); - expect(instance.receivedIds!.has('removed-bar')).to.be.true; + expect(instance.received).to.eql([target, null]); }); - it('changing an element id attribute fires for both old and new IDs', async () => { - const el = document.createElement('div'); - el.id = 'old-id'; - container.appendChild(el); + it('follows changes of the id attribute', async () => { + const target = add(createTarget()); await elementUpdated(instance); - instance.callCount = 0; + target.id = 'idref-renamed'; + await elementUpdated(instance); - el.id = 'new-id'; + const other = add(createTarget('idref-other')); + other.id = ID; await elementUpdated(instance); - expect(instance.callCount).to.be.greaterThan(0); - expect(instance.receivedIds!.has('old-id')).to.be.true; - expect(instance.receivedIds!.has('new-id')).to.be.true; + expect(instance.received).to.eql([target, null, other]); }); - it('adding a subtree with nested [id] descendants fires for all nested IDs', async () => { + it('reports a nested element with the id', async () => { const parent = document.createElement('div'); - const childA = document.createElement('span'); - const childB = document.createElement('span'); - childA.id = 'nested-a'; - childB.id = 'nested-b'; - parent.appendChild(childA); - parent.appendChild(childB); - - container.appendChild(parent); + const target = parent.appendChild(createTarget()); + + add(parent); await elementUpdated(instance); - expect(instance.receivedIds!.has('nested-a')).to.be.true; - expect(instance.receivedIds!.has('nested-b')).to.be.true; + expect(instance.received).to.eql([target]); }); - it('DOM mutations with no ID-bearing nodes do not fire callback', async () => { - const el = document.createElement('div'); - container.appendChild(el); + it('ignores changes that keep the element of the id', async () => { + const target = add(createTarget()); await elementUpdated(instance); - expect(instance.callCount).to.equal(0); - }); - - it('skips text nodes and elements with an empty id', async () => { - const parent = document.createElement('div'); const empty = document.createElement('span'); empty.setAttribute('id', ''); - parent.append(empty); + add(document.createElement('div')).append( + document.createTextNode('text'), + empty + ); + add(createTarget('idref-other')); + add(createTarget()); + await elementUpdated(instance); - container.append(document.createTextNode('text'), parent); + expect(instance.received).to.eql([target]); + }); + + it('reports the next element with the id when the first leaves', async () => { + const first = add(createTarget()); + const second = add(createTarget()); + await elementUpdated(instance); + + first.remove(); await elementUpdated(instance); - expect(instance.callCount).to.equal(0); + expect(instance.received).to.eql([first, second]); }); }); describe('host lifecycle', () => { - let el: HTMLDivElement; - afterEach(() => { - el?.remove(); - instance.resolver.unobserve(); if (!instance.isConnected) { document.body.appendChild(instance); } }); - it('observe() called before hostConnected activates on connect', async () => { + it('observe() called before hostConnected reports on connect', async () => { const detached = document.createElement(tag) as HostInstance; + const target = add(createTarget()); detached.resolver.observe(); - document.body.appendChild(detached); - await elementUpdated(detached); + expect(detached.received).to.be.empty; - el = document.createElement('div'); - el.id = 'before-connect'; - document.body.appendChild(el); + add(detached); await elementUpdated(detached); - expect(detached.callCount).to.be.greaterThan(0); - expect(detached.receivedIds!.has('before-connect')).to.be.true; - - detached.remove(); + expect(detached.received).to.eql([target]); }); it('callback is suspended while host is disconnected', async () => { instance.resolver.observe(); instance.remove(); - el = document.createElement('div'); - el.id = 'during-disconnect'; - document.body.appendChild(el); + add(createTarget()); await elementUpdated(instance); - expect(instance.callCount).to.equal(0); + expect(instance.received).to.be.empty; }); - it('observation resumes after host is reconnected', async () => { + it('a reconnect reports a change made while disconnected, then observes again', async () => { instance.resolver.observe(); instance.remove(); - el = document.createElement('div'); - el.id = 'during-disconnect'; - document.body.appendChild(el); + const target = add(createTarget()); await elementUpdated(instance); - expect(instance.callCount).to.equal(0); - document.body.appendChild(instance); await elementUpdated(instance); - const el2 = document.createElement('div'); - el2.id = 'after-reconnect'; - document.body.appendChild(el2); - await elementUpdated(instance); + expect(instance.received).to.eql([target]); - expect(instance.callCount).to.be.greaterThan(0); - expect(instance.receivedIds!.has('after-reconnect')).to.be.true; + target.remove(); + await elementUpdated(instance); - el2.remove(); + expect(instance.received).to.eql([target, null]); }); it('unobserve() during disconnect does not resume on reconnect', async () => { @@ -309,79 +281,43 @@ describe('IdRefResolverController', () => { document.body.appendChild(instance); await elementUpdated(instance); - el = document.createElement('div'); - el.id = 'after-unobserve-reconnect'; - document.body.appendChild(el); + add(createTarget()); await elementUpdated(instance); - expect(instance.callCount).to.equal(0); + expect(instance.received).to.be.empty; }); }); describe('callback context', () => { - let el: HTMLDivElement; - - afterEach(() => { - el?.remove(); - instance.resolver.unobserve(); - }); - it('this inside callback is bound to the host element', async () => { instance.resolver.observe(); - el = document.createElement('div'); - el.id = 'context-check'; - document.body.appendChild(el); + add(createTarget()); await elementUpdated(instance); expect(instance.capturedThis).to.equal(instance); }); }); - describe('reference counting', () => { - let secondTag: string; - let second: SecondHostInstance; - let el: HTMLDivElement; - - before(() => { - secondTag = defineCE( - class extends LitElement { - public callCount = 0; - public readonly resolver = addIdRefResolver( - this, - function (this: LitElement) { - (this as unknown as SecondHostInstance).callCount++; - this.requestUpdate(); - } - ); - } - ); - }); + describe('shared observation', () => { + let second: HostInstance; beforeEach(async () => { - const tagName = unsafeStatic(secondTag); - second = await fixture( - html`<${tagName}>` - ); + const tagName = unsafeStatic(tag); + second = await fixture(html`<${tagName}>`); }); - afterEach(() => { - el?.remove(); - instance.resolver.unobserve(); - second.resolver.unobserve(); - }); + afterEach(() => second.resolver.unobserve()); it('two controllers both observing both receive callbacks', async () => { instance.resolver.observe(); second.resolver.observe(); - el = document.createElement('div'); - el.id = 'shared-both'; - document.body.appendChild(el); + const target = add(createTarget()); await Promise.all([elementUpdated(instance), elementUpdated(second)]); - expect(instance.callCount).to.be.greaterThan(0); - expect(second.callCount).to.be.greaterThan(0); + expect(instance.received).to.eql([target]); + expect(second.received).to.eql([target]); }); it('one controller unobserving does not prevent the other from receiving callbacks', async () => { @@ -389,13 +325,11 @@ describe('IdRefResolverController', () => { second.resolver.observe(); instance.resolver.unobserve(); - el = document.createElement('div'); - el.id = 'shared-one-unobserve'; - document.body.appendChild(el); + const target = add(createTarget()); await Promise.all([elementUpdated(instance), elementUpdated(second)]); - expect(instance.callCount).to.equal(0); - expect(second.callCount).to.be.greaterThan(0); + expect(instance.received).to.be.empty; + expect(second.received).to.eql([target]); }); it('both controllers unobserving stops all callbacks', async () => { @@ -404,160 +338,95 @@ describe('IdRefResolverController', () => { instance.resolver.unobserve(); second.resolver.unobserve(); - el = document.createElement('div'); - el.id = 'shared-both-unobserve'; - document.body.appendChild(el); + add(createTarget()); await Promise.all([elementUpdated(instance), elementUpdated(second)]); - expect(instance.callCount).to.equal(0); - expect(second.callCount).to.equal(0); + expect(instance.received).to.be.empty; + expect(second.received).to.be.empty; }); }); describe('Shadow DOM scoping', () => { - let shadowTag: string; - let shadowHost: HTMLElement; + let shadowRoot: ShadowRoot; let shadowInstance: HostInstance; - before(() => { - shadowTag = defineCE( - class extends LitElement { - public receivedIds: Set | null = null; - public callCount = 0; - public capturedThis: unknown = null; - - public readonly resolver = addIdRefResolver( - this, - function (this: LitElement, ids: Set) { - (this as unknown as HostInstance).receivedIds = new Set(ids); - (this as unknown as HostInstance).callCount++; - (this as unknown as HostInstance).capturedThis = this; - this.requestUpdate(); - } - ); - } - ); - }); - beforeEach(() => { - shadowHost = document.createElement('div'); - shadowHost.attachShadow({ mode: 'open' }); - document.body.appendChild(shadowHost); - - shadowInstance = document.createElement(shadowTag) as HostInstance; - shadowHost.shadowRoot!.appendChild(shadowInstance); + const shadowHost = add(document.createElement('div')); + shadowRoot = shadowHost.attachShadow({ mode: 'open' }); + shadowInstance = shadowRoot.appendChild( + document.createElement(tag) as HostInstance + ); }); - afterEach(() => { - shadowInstance.resolver.unobserve(); - shadowHost.remove(); - }); + afterEach(() => shadowInstance.resolver.unobserve()); - it('detects ID additions within its own shadow root', async () => { + it('detects additions within its own shadow root', async () => { shadowInstance.resolver.observe(); - await elementUpdated(shadowInstance); - const el = document.createElement('div'); - el.id = 'shadow-target'; - shadowHost.shadowRoot!.appendChild(el); + const target = shadowRoot.appendChild(createTarget()); await elementUpdated(shadowInstance); - expect(shadowInstance.callCount).to.be.greaterThan(0); - expect(shadowInstance.receivedIds!.has('shadow-target')).to.be.true; + expect(shadowInstance.received).to.eql([target]); }); - it('detects ID removals within its own shadow root', async () => { - const el = document.createElement('div'); - el.id = 'shadow-remove'; - shadowHost.shadowRoot!.appendChild(el); - + it('detects removals within its own shadow root', async () => { + const target = shadowRoot.appendChild(createTarget()); shadowInstance.resolver.observe(); - await elementUpdated(shadowInstance); - shadowInstance.callCount = 0; - el.remove(); + target.remove(); await elementUpdated(shadowInstance); - expect(shadowInstance.callCount).to.be.greaterThan(0); - expect(shadowInstance.receivedIds!.has('shadow-remove')).to.be.true; + expect(shadowInstance.received).to.eql([target, null]); }); - it('detects ID attribute changes within its own shadow root', async () => { - const el = document.createElement('div'); - el.id = 'shadow-old'; - shadowHost.shadowRoot!.appendChild(el); - + it('detects id attribute changes within its own shadow root', async () => { + const target = shadowRoot.appendChild(createTarget('idref-other')); shadowInstance.resolver.observe(); - await elementUpdated(shadowInstance); - shadowInstance.callCount = 0; - el.id = 'shadow-new'; + target.id = ID; await elementUpdated(shadowInstance); - expect(shadowInstance.callCount).to.be.greaterThan(0); - expect(shadowInstance.receivedIds!.has('shadow-old')).to.be.true; - expect(shadowInstance.receivedIds!.has('shadow-new')).to.be.true; + expect(shadowInstance.received).to.eql([target]); }); - it('does NOT react to ID changes in the document when scoped to a shadow root', async () => { + it('does NOT react to changes in the document when scoped to a shadow root', async () => { shadowInstance.resolver.observe(); - await elementUpdated(shadowInstance); - const el = document.createElement('div'); - el.id = 'document-only'; - document.body.appendChild(el); + add(createTarget()); await elementUpdated(shadowInstance); - expect(shadowInstance.callCount).to.equal(0); - el.remove(); + expect(shadowInstance.received).to.be.empty; }); - it('does NOT react to ID changes in a different shadow root', async () => { + it('does NOT react to changes in a different shadow root', async () => { shadowInstance.resolver.observe(); - await elementUpdated(shadowInstance); - - const otherHost = document.createElement('div'); - otherHost.attachShadow({ mode: 'open' }); - document.body.appendChild(otherHost); - const el = document.createElement('div'); - el.id = 'other-shadow'; - otherHost.shadowRoot!.appendChild(el); + const other = add(document.createElement('div')); + other.attachShadow({ mode: 'open' }).appendChild(createTarget()); await elementUpdated(shadowInstance); - expect(shadowInstance.callCount).to.equal(0); - otherHost.remove(); + expect(shadowInstance.received).to.be.empty; }); it('resolves elements scoped to its own shadow root', () => { - const el = document.createElement('div'); - el.id = 'resolve-shadow'; - shadowHost.shadowRoot!.appendChild(el); + const target = shadowRoot.appendChild(createTarget()); - expect(getElementByIdFromRoot(shadowInstance, 'resolve-shadow')).to.equal( - el - ); + expect(getElementByIdFromRoot(shadowInstance, ID)).to.equal(target); }); it('does NOT resolve elements from the document root', () => { - const el = document.createElement('div'); - el.id = 'doc-element'; - document.body.appendChild(el); + add(createTarget()); - expect(getElementByIdFromRoot(shadowInstance, 'doc-element')).to.be.null; - el.remove(); + expect(getElementByIdFromRoot(shadowInstance, ID)).to.be.null; }); it('document-scoped controller does NOT react to shadow root changes', async () => { instance.resolver.observe(); - const el = document.createElement('div'); - el.id = 'inside-shadow-only'; - shadowHost.shadowRoot!.appendChild(el); + shadowRoot.appendChild(createTarget()); await elementUpdated(instance); - expect(instance.callCount).to.equal(0); - instance.resolver.unobserve(); + expect(instance.received).to.be.empty; }); }); }); diff --git a/src/internals/controllers/id-resolver.ts b/src/internals/controllers/id-resolver.ts index 0f24ab996..e7c0c811f 100644 --- a/src/internals/controllers/id-resolver.ts +++ b/src/internals/controllers/id-resolver.ts @@ -1,5 +1,5 @@ import { isServer, type LitElement, type ReactiveController } from 'lit'; -import { isDocument, isElement } from '../utils/dom.js'; +import { getElementByIdFromRoot, isDocument } from '../utils/dom.js'; import { getOrInsertComputed } from '../utils/objects.js'; const ID_REF_OBSERVERS = new WeakMap(); @@ -12,40 +12,10 @@ function getObserver(root: Node): IdRefObserver { ); } -/** Adds the ids of `nodes` and of any of their descendants to `affected`. */ -function collectIds(nodes: NodeList, affected: Set): void { - for (const node of nodes) { - if (!isElement(node)) continue; - if (node.id) affected.add(node.id); - for (const child of node.querySelectorAll('[id]')) { - if (child.id) affected.add(child.id); - } - } -} - -function changedIds(mutations: MutationRecord[]): Set { - const affected = new Set(); - - for (const mutation of mutations) { - if (mutation.type === 'attributes') { - if (!isElement(mutation.target)) continue; - const oldId = mutation.oldValue; - const newId = mutation.target.id; - if (oldId) affected.add(oldId); - if (newId) affected.add(newId); - } else { - collectIds(mutation.addedNodes, affected); - collectIds(mutation.removedNodes, affected); - } - } - - return affected; -} - /** - * Tells the resolvers of a root node which ids changed. They share one - * observer. Not an `EventTarget`: Chromium scans all listeners on each add - * and remove. + * The shared observer of the resolvers of a root node. A batch of changes + * costs one `getElementById` per resolver, not a scan of the changed nodes. + * Not an `EventTarget`: Chromium scans all listeners on each add and remove. */ class IdRefObserver { private readonly _observer?: MutationObserver; @@ -56,13 +26,9 @@ class IdRefObserver { this._root = root; if (!isServer) { - this._observer = new MutationObserver((mutations) => { - const ids = changedIds(mutations); - - if (ids.size > 0) { - for (const resolver of this._resolvers) { - resolver.idsChanged(ids); - } + this._observer = new MutationObserver(() => { + for (const resolver of this._resolvers) { + resolver.check(); } }); } @@ -74,7 +40,6 @@ class IdRefObserver { const root = isDocument(this._root) ? this._root.body : this._root; this._observer?.observe(root, { attributeFilter: ['id'], - attributeOldValue: true, subtree: true, childList: true, }); @@ -91,19 +56,27 @@ class IdRefObserver { } } -/** Resolves ID references in a root node. See {@link addIdRefResolver}. */ +/** Resolves an ID reference in a root node. See {@link addIdRefResolver}. */ class IdRefResolverController implements ReactiveController { private readonly _host: LitElement; - private readonly _callback: (ids: Set) => unknown; + private readonly _id: () => string | null; + private readonly _callback: (element: Element | null) => unknown; /** Whether the consumer asked for the observation. */ private _active = false; + /** The element of the last callback. */ + private _element: Element | null = null; /** The shared observer of the current root node, non-null only while observing. */ private _shared: IdRefObserver | null = null; - constructor(host: LitElement, callback: (ids: Set) => unknown) { + constructor( + host: LitElement, + id: () => string | null, + callback: (element: Element | null) => unknown + ) { this._host = host; this._host.addController(this); + this._id = id; this._callback = callback; } @@ -112,6 +85,8 @@ class IdRefResolverController implements ReactiveController { this._shared = getObserver(this._host.getRootNode()); this._shared.add(this); } + + this.check(); } private _unobserve(): void { @@ -120,8 +95,14 @@ class IdRefResolverController implements ReactiveController { } /** @internal */ - public idsChanged(ids: Set): void { - this._callback.call(this._host, ids); + public check(): void { + const id = this._id(); + const element = id ? getElementByIdFromRoot(this._host, id) : null; + + if (element !== this._element) { + this._element = element; + this._callback.call(this._host, element); + } } /** @internal */ @@ -136,6 +117,7 @@ class IdRefResolverController implements ReactiveController { this._unobserve(); } + /** Starts the observation, or checks the id again while observing. */ public observe(): void { this._active = true; @@ -146,22 +128,27 @@ class IdRefResolverController implements ReactiveController { public unobserve(): void { this._active = false; + this._element = null; this._unobserve(); } } /** - * Adds a controller that resolves ID references in the root node of the host. + * Adds a controller that resolves the ID reference `id` in the root node of + * the host. * * @remarks - * The controller calls `callback` with the ids that changed. Call `observe` - * to start and `unobserve` to stop. A root node shares one observation. + * The controller calls `callback` with the element of the id when it differs + * from the last one: on `observe`, on a connect and after DOM changes in the + * root node. Call `observe` to start and `unobserve` to stop. A root node + * shares one observation. */ export function addIdRefResolver( host: LitElement, - callback: (ids: Set) => unknown + id: () => string | null, + callback: (element: Element | null) => unknown ): IdRefResolverController { - return new IdRefResolverController(host, callback); + return new IdRefResolverController(host, id, callback); } export type { IdRefResolverController }; diff --git a/src/internals/controllers/internals.spec.ts b/src/internals/controllers/internals.spec.ts index 2d0318ff2..ee1fe9839 100644 --- a/src/internals/controllers/internals.spec.ts +++ b/src/internals/controllers/internals.spec.ts @@ -293,6 +293,33 @@ describe('ElementInternals controller', () => { expect(host.internals.labels).to.eql([label]); }); + + describe('in a shadow root', () => { + async function renderInShadow(content: string): Promise { + const wrapper = await fixture(html`
`); + const root = wrapper.attachShadow({ mode: 'open' }); + root.innerHTML = content; + return root; + } + + it('does not create the live list for an id when the root has no label', async () => { + const root = await renderInShadow(`<${tag} id="inner">`); + const host = root.querySelector(tag)!; + const read = sinon.spy(ElementInternals.prototype, 'labels', ['get']); + + expect(host.internals.labels).to.be.null; + expect(read.get.called).to.be.false; + }); + + it('returns the label of the same root that names the host id', async () => { + const root = await renderInShadow( + `<${tag} id="inner">` + ); + const host = root.querySelector(tag)!; + + expect(host.internals.labels).to.eql([root.querySelector('label')]); + }); + }); }); }); }); diff --git a/src/internals/controllers/internals.ts b/src/internals/controllers/internals.ts index fbf14e7c0..4b02cf569 100644 --- a/src/internals/controllers/internals.ts +++ b/src/internals/controllers/internals.ts @@ -5,7 +5,20 @@ import { type ReactiveControllerHost, } from 'lit'; import { sameItems } from '../utils/arrays.js'; -import { setOrRemoveAttribute } from '../utils/dom.js'; +import { isDocument, setOrRemoveAttribute } from '../utils/dom.js'; + +/** + * Whether a `