Skip to content

Commit b6f9766

Browse files
Copilotmattcosta7
andcommitted
Address final code review feedback - improve comments and test mocking
Co-authored-by: mattcosta7 <8616962+mattcosta7@users.noreply.github.com>
1 parent 733f079 commit b6f9766

File tree

2 files changed

+9
-9
lines changed

2 files changed

+9
-9
lines changed

src/lazy-define.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -112,14 +112,15 @@ function scan(element: ElementLike) {
112112
// FIX 7: Early return optimization
113113
if (pending.size === 0) return
114114

115-
// FIX 7: Create snapshot to iterate safely
115+
// FIX 7: Create snapshot to prevent modification-during-iteration issues
116+
// (concurrent scans may delete tags from pending)
116117
const tagList = Array.from(pending.keys())
117118

118119
for (const tagName of tagList) {
119120
const child: Element | null =
120121
element instanceof Element && element.matches(tagName) ? element : element.querySelector(tagName)
121122
if (customElements.get(tagName) || child) {
122-
// Skip if already triggered and no longer in pending
123+
// Skip if already processed and not re-registered
123124
if (triggered.has(tagName) && !pending.has(tagName)) continue
124125

125126
triggered.add(tagName)
@@ -167,8 +168,9 @@ export function lazyDefine(tagNameOrObj: string | Record<string, () => void>, si
167168
}
168169

169170
for (const [tagName, callback] of Object.entries(tagNameOrObj)) {
170-
// FIX 6: Late registration - execute immediately if already triggered AND elements exist
171-
if (triggered.has(tagName) && document.querySelector(tagName) !== null) {
171+
// FIX 6: Late registration - execute immediately if already triggered
172+
// Check both triggered state and element existence to avoid executing for removed elements
173+
if (triggered.has(tagName) && document.querySelector(tagName)) {
172174
// eslint-disable-next-line github/no-then
173175
Promise.resolve().then(() => {
174176
try {

test/lazy-define.ts

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import {expect, fixture, html} from '@open-wc/testing'
2-
import {spy} from 'sinon'
2+
import {spy, stub} from 'sinon'
33
import {lazyDefine, observe} from '../src/lazy-define.js'
44

55
const animationFrame = () => new Promise<unknown>(resolve => requestAnimationFrame(resolve))
@@ -161,10 +161,8 @@ describe('lazyDefine', () => {
161161
})
162162
const onDefine2 = spy()
163163

164-
// Suppress global error reporting for this test
165-
const originalReportError = globalThis.reportError
166164
const errors: unknown[] = []
167-
globalThis.reportError = (err: unknown) => errors.push(err)
165+
const reportErrorStub = stub(globalThis, 'reportError').callsFake((err: unknown) => errors.push(err))
168166

169167
try {
170168
lazyDefine('error-test-element', onDefine1)
@@ -180,7 +178,7 @@ describe('lazyDefine', () => {
180178
// Error should have been reported
181179
expect(errors.length).to.be.greaterThan(0)
182180
} finally {
183-
globalThis.reportError = originalReportError
181+
reportErrorStub.restore()
184182
}
185183
})
186184
})

0 commit comments

Comments
 (0)