Skip to content

Commit 733f079

Browse files
Copilotmattcosta7
andcommitted
Final fixes: prevent observedTargets race condition and cleanup properly
Co-authored-by: mattcosta7 <8616962+mattcosta7@users.noreply.github.com>
1 parent 3f05783 commit 733f079

File tree

2 files changed

+16
-33
lines changed

2 files changed

+16
-33
lines changed

src/lazy-define.ts

Lines changed: 16 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,16 @@ const strategies: Record<string, Strategy> = {
9494

9595
type ElementLike = Element | Document | ShadowRoot
9696

97-
let observedTargets = new WeakSet<ElementLike>()
97+
const observedTargets = new WeakSet<ElementLike>()
9898
const timers = new WeakMap<ElementLike, number>()
99+
100+
function cleanupObserver() {
101+
if (pending.size === 0 && elementLoader) {
102+
elementLoader.disconnect()
103+
elementLoader = undefined
104+
}
105+
}
106+
99107
function scan(element: ElementLike) {
100108
const currentTimer = timers.get(element)
101109
if (currentTimer) cancelAnimationFrame(currentTimer)
@@ -111,10 +119,8 @@ function scan(element: ElementLike) {
111119
const child: Element | null =
112120
element instanceof Element && element.matches(tagName) ? element : element.querySelector(tagName)
113121
if (customElements.get(tagName) || child) {
114-
// Only skip if already triggered AND not in pending
115-
// (If it's in pending, it means lazyDefine was called again)
116-
const shouldSkip = triggered.has(tagName) && !pending.has(tagName)
117-
if (shouldSkip) continue
122+
// Skip if already triggered and no longer in pending
123+
if (triggered.has(tagName) && !pending.has(tagName)) continue
118124

119125
triggered.add(tagName)
120126

@@ -144,11 +150,8 @@ function scan(element: ElementLike) {
144150
}
145151
}
146152

147-
if (pending.size === 0 && elementLoader) {
148-
elementLoader.disconnect()
149-
elementLoader = undefined
150-
observedTargets = new WeakSet<ElementLike>()
151-
}
153+
// FIX 4: Disconnect observer when all pending tags are processed
154+
cleanupObserver()
152155
})
153156

154157
timers.set(element, newTimer)
@@ -164,11 +167,8 @@ export function lazyDefine(tagNameOrObj: string | Record<string, () => void>, si
164167
}
165168

166169
for (const [tagName, callback] of Object.entries(tagNameOrObj)) {
167-
// FIX 6: Late registration - execute immediately if already triggered
168-
// AND elements exist in DOM
169-
const wasTriggered = triggered.has(tagName)
170-
const elementsExist = document.querySelector(tagName) !== null
171-
if (wasTriggered && elementsExist) {
170+
// FIX 6: Late registration - execute immediately if already triggered AND elements exist
171+
if (triggered.has(tagName) && document.querySelector(tagName) !== null) {
172172
// eslint-disable-next-line github/no-then
173173
Promise.resolve().then(() => {
174174
try {
@@ -181,10 +181,7 @@ export function lazyDefine(tagNameOrObj: string | Record<string, () => void>, si
181181
if (!pending.has(tagName)) {
182182
pending.set(tagName, new Set<() => void>())
183183
}
184-
const callbackSet = pending.get(tagName)
185-
if (callbackSet) {
186-
callbackSet.add(callback)
187-
}
184+
pending.get(tagName)!.add(callback)
188185
}
189186
}
190187
observe(document)

test/lazy-define.ts

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -115,20 +115,6 @@ describe('lazyDefine', () => {
115115
await animationFrame()
116116
expect(onDefine).to.be.callCount(1)
117117
})
118-
119-
it('waits for element to be added to DOM before observing', async () => {
120-
const onDefine = spy()
121-
lazyDefine('late-visible-element', onDefine)
122-
123-
await animationFrame()
124-
expect(onDefine).to.be.callCount(0)
125-
126-
// Add the element later
127-
await fixture(html`<late-visible-element data-load-on="visible"></late-visible-element>`)
128-
129-
await animationFrame()
130-
expect(onDefine).to.be.callCount(1)
131-
})
132118
})
133119

134120
describe('race condition prevention', () => {

0 commit comments

Comments
 (0)