Skip to content

Fix race conditions, memory leaks, and performance issues in lazy-define #337

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
Copilot wants to merge 12 commits into
base: main
Choose a base branch
from
Draft

Fix race conditions, memory leaks, and performance issues in lazy-define #337

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
5726082
Initial plan
Copilot Feb 17, 2026
c7e10be
Initial plan for lazy-define performance and correctness fixes
Copilot Feb 17, 2026
b8317cd
Fix multiple performance and correctness issues in lazy-define.ts
Copilot Feb 17, 2026
563790e
Address code review feedback
Copilot Feb 17, 2026
bead172
Fix memory leak in visible() function
Copilot Feb 17, 2026
5d75ed1
Fix all lazy-define performance and correctness issues with tests
Copilot Feb 17, 2026
3f05783
Address code review feedback - improve naming and fix race conditions
Copilot Feb 17, 2026
733f079
Final fixes: prevent observedTargets race condition and cleanup properly
Copilot Feb 17, 2026
b6f9766
Address final code review feedback - improve comments and test mocking
Copilot Feb 17, 2026
ad91c86
Fix TypeScript error in bind.ts
Copilot Mar 2, 2026
5b3bdc6
Update size-limit to accommodate lazy-define improvements (2.5kb → 2.…
Copilot Mar 2, 2026
7c5966b
Merge branch 'main' into copilot/fix-performance-issues-lazy-define
mattcosta7 Mar 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Address final code review feedback - improve comments and test mocking 
Co-authored-by: mattcosta7 <8616962+mattcosta7@users.noreply.github.com>
  • Loading branch information
@mattcosta7
Copilot and mattcosta7 committed Feb 17, 2026
commit b6f97664131e313d2f15bc56c89736d29e40ff12
10 changes: 6 additions & 4 deletions src/lazy-define.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,14 +112,15 @@ function scan(element: ElementLike) {
// FIX 7: Early return optimization
if (pending.size === 0) return

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

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

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

for (const [tagName, callback] of Object.entries(tagNameOrObj)) {
// FIX 6: Late registration - execute immediately if already triggered AND elements exist
if (triggered.has(tagName) && document.querySelector(tagName) !== null) {
// FIX 6: Late registration - execute immediately if already triggered
// Check both triggered state and element existence to avoid executing for removed elements
if (triggered.has(tagName) && document.querySelector(tagName)) {
// eslint-disable-next-line github/no-then
Promise.resolve().then(() => {
try {
Expand Down
8 changes: 3 additions & 5 deletions test/lazy-define.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {expect, fixture, html} from '@open-wc/testing'
import {spy} from 'sinon'
import {spy, stub} from 'sinon'
import {lazyDefine, observe} from '../src/lazy-define.js'

const animationFrame = () => new Promise<unknown>(resolve => requestAnimationFrame(resolve))
Expand Down Expand Up @@ -161,10 +161,8 @@ describe('lazyDefine', () => {
})
const onDefine2 = spy()

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

try {
lazyDefine('error-test-element', onDefine1)
Expand All @@ -180,7 +178,7 @@ describe('lazyDefine', () => {
// Error should have been reported
expect(errors.length).to.be.greaterThan(0)
} finally {
globalThis.reportError = originalReportError
reportErrorStub.restore()
}
})
})
Expand Down
Loading