From 463bbe8264180137f87a3e4b41047f4db0378de9 Mon Sep 17 00:00:00 2001 From: Matthew Costabile <8616962+mattcosta7@users.noreply.github.com> Date: Sun, 8 Nov 2026 09:07:20 -0500 Subject: [PATCH] Stop retaining disconnected elements in the shared date observer The module-level dateObserver holds a strong Set of every observed element. update() re-observed the element whenever its format needed ticking, regardless of connection state, so an element could be added to the Set without ever being removed again: - an element given a datetime but never connected - a cloneNode(true) of a connected element (clones are upgraded and receive attributeChangedCallback) - an element whose attribute changed and which was removed in the same task: disconnectedCallback unobserves first, then the microtask-scheduled update() re-observes it The Set then retains the element and its whole detached tree, and every timer tick keeps calling update() on it. Only observe elements that are connected, and drop any disconnected element found while ticking as defensive cleanup. Tests that relied on observing never-connected elements now attach them to the fixture. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/relative-time-element.ts | 6 +- test/relative-time.js | 115 +++++++++++++++++++++++++++++++++++ 2 files changed, 120 insertions(+), 1 deletion(-) diff --git a/src/relative-time-element.ts b/src/relative-time-element.ts index e7c9d47..acad570 100644 --- a/src/relative-time-element.ts +++ b/src/relative-time-element.ts @@ -144,6 +144,10 @@ const dateObserver = new (class { this.updating = true try { for (const timeEl of this.elements) { + if (!timeEl.isConnected) { + this.elements.delete(timeEl) + continue + } nearestDistance = Math.min(nearestDistance, getUnitFactor(timeEl)) timeEl.update() } @@ -752,7 +756,7 @@ export class RelativeTimeElement extends HTMLElement implements Intl.DateTimeFor (this.format === 'micro' && Boolean(this.#getExplicitThreshold()) && date.getTime() > now) || (displayUserPreferredAbsoluteTime && (this.#isToday(date, locale, timeZone) || this.#isCurrentYear(date, locale, timeZone))) - if (shouldObserve) { + if (shouldObserve && this.isConnected) { dateObserver.observe(this) } else { dateObserver.unobserve(this) diff --git a/test/relative-time.js b/test/relative-time.js index 186a140..6ff90d8 100644 --- a/test/relative-time.js +++ b/test/relative-time.js @@ -50,6 +50,7 @@ suite('relative-time', function () { return 0 } try { + fixture.append(time) time.setAttribute('format', 'duration') time.setAttribute('datetime', '1970-01-01T00:00:00.000Z') await Promise.resolve() @@ -114,6 +115,7 @@ suite('relative-time', function () { return 1 } try { + fixture.append(time) time.setAttribute('format', 'micro') time.setAttribute('datetime', new Date(Date.now() + 65 * 1000).toISOString()) await Promise.resolve() @@ -129,6 +131,7 @@ suite('relative-time', function () { delays.length = 0 const pastTime = document.createElement('relative-time') + fixture.append(pastTime) pastTime.setAttribute('format', 'micro') pastTime.setAttribute('threshold', 'PT1H') pastTime.setAttribute('datetime', new Date(Date.now() - 59 * 60 * 1000 - 58 * 1000).toISOString()) @@ -308,6 +311,7 @@ suite('relative-time', function () { // eslint-disable-next-line @typescript-eslint/no-invalid-this this.timeout(3000) const el = document.createElement('relative-time') + fixture.append(el) el.setAttribute('datetime', new Date(Date.now() + 25000).toISOString()) await Promise.resolve() const display = el.shadowRoot?.textContent || el.textContent @@ -3421,4 +3425,115 @@ suite('relative-time', function () { assert.equal(el.shadowRoot.textContent, 'Sat, Dec 31, 2022') }) }) + + suite('shared update timer', () => { + const originalSetAdd = Set.prototype.add + let observedSets + + setup(() => { + observedSets = new Set() + Set.prototype.add = function (value) { + if (value instanceof RelativeTimeElement) observedSets.add(this) + return originalSetAdd.call(this, value) + } + }) + + teardown(() => { + Set.prototype.add = originalSetAdd + }) + + // The observer's Set is module-private, so detect membership by recording every Set that a + // RelativeTimeElement is added to and checking it afterwards. + function isObserved(el) { + return [...observedSets].some(set => set.has(el)) + } + + const recent = () => new Date(Date.now() - 3 * 60 * 1000).toISOString() + + test('observes connected elements', async () => { + const el = document.createElement('relative-time') + fixture.append(el) + el.setAttribute('datetime', recent()) + await Promise.resolve() + assert.isTrue(isObserved(el)) + }) + + test('stops observing elements when they are disconnected', async () => { + const el = document.createElement('relative-time') + fixture.append(el) + el.setAttribute('datetime', recent()) + await Promise.resolve() + el.remove() + assert.isFalse(isObserved(el)) + }) + + test('does not observe elements that were never connected', async () => { + const el = document.createElement('relative-time') + el.setAttribute('datetime', recent()) + await Promise.resolve() + assert.equal(el.shadowRoot.textContent, '3 minutes ago') + assert.isFalse(isObserved(el)) + }) + + test('does not observe clones of connected elements', async () => { + const el = document.createElement('relative-time') + fixture.append(el) + el.setAttribute('datetime', recent()) + await Promise.resolve() + const clone = el.cloneNode(true) + await Promise.resolve() + assert.equal(clone.shadowRoot.textContent, '3 minutes ago') + assert.isFalse(isObserved(clone)) + }) + + test('does not observe elements removed in the same task as an attribute change', async () => { + const el = document.createElement('relative-time') + fixture.append(el) + el.setAttribute('datetime', recent()) + await Promise.resolve() + el.setAttribute('datetime', new Date(Date.now() - 5 * 60 * 1000).toISOString()) + el.remove() + await Promise.resolve() + assert.equal(el.shadowRoot.textContent, '5 minutes ago') + assert.isFalse(isObserved(el)) + }) + + test('timer ticks update connected elements and drop disconnected ones', async () => { + const originalSetTimeout = window.setTimeout + const connected = document.createElement('relative-time') + const detached = document.createElement('relative-time') + let tick + // Far-future "now" so the observer always schedules a fresh timer instead of reusing a pending one. + freezeTime(new Date('2100-01-01T00:00:00.000Z')) + globalThis.setTimeout = window.setTimeout = function (callback) { + tick = callback + return 0 + } + try { + fixture.append(connected) + connected.setAttribute('datetime', '2099-12-31T23:57:00.000Z') + await Promise.resolve() + assert.isFunction(tick) + + // Force a detached element into the Set as if it had been retained before removal. + const set = [...observedSets].find(candidate => candidate.has(connected)) + originalSetAdd.call(set, detached) + detached.setAttribute('datetime', '2099-12-31T23:57:00.000Z') + let detachedUpdates = 0 + const update = detached.update + detached.update = function () { + detachedUpdates += 1 + return update.call(this) + } + + Date.now = () => new dateNow('2100-01-01T00:01:00.000Z').getTime() + tick() + assert.equal(connected.shadowRoot.textContent, '4 minutes ago') + assert.equal(detachedUpdates, 0) + assert.isFalse(set.has(detached)) + } finally { + globalThis.setTimeout = window.setTimeout = originalSetTimeout + } + }) + }) })