Skip to content

fix: coalesce scan() into a single rAF to avoid paint delay on bulk DOM mutations #344

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

Open
alexus37 wants to merge 2 commits into
base: main
Choose a base branch
from
+48 −10
Open

fix: coalesce scan() into a single rAF to avoid paint delay on bulk DOM mutations #344

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c95922a
fix: coalesce scan() into a single rAF to avoid paint delay on bulk D…
alexus37 Apr 16, 2026
0114d5e
style: fix prettier formatting
alexus37 Apr 16, 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
26 changes: 16 additions & 10 deletions src/lazy-define.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,26 +57,32 @@ const strategies: Record<string, Strategy> = {

type ElementLike = Element | Document | ShadowRoot

const timers = new WeakMap<ElementLike, number>()
const pendingElements = new Set<ElementLike>()
let scanTimer: number | null = null

function scan(element: ElementLike) {
cancelAnimationFrame(timers.get(element) || 0)
timers.set(
element,
requestAnimationFrame(() => {
pendingElements.add(element)
if (scanTimer != null) return
scanTimer = requestAnimationFrame(() => {
scanTimer = null
if (!dynamicElements.size) {
pendingElements.clear()
return
}
for (const el of pendingElements) {
for (const tagName of dynamicElements.keys()) {
const child: Element | null =
element instanceof Element && element.matches(tagName) ? element : element.querySelector(tagName)
const child: Element | null = el instanceof Element && el.matches(tagName) ? el : el.querySelector(tagName)
if (customElements.get(tagName) || child) {
const strategyName = (child?.getAttribute('data-load-on') || 'ready') as keyof typeof strategies
const strategy = strategyName in strategies ? strategies[strategyName] : strategies.ready
// eslint-disable-next-line github/no-then
for (const cb of dynamicElements.get(tagName) || []) strategy(tagName).then(cb)
dynamicElements.delete(tagName)
timers.delete(element)
}
}
})
)
}
pendingElements.clear()
Comment on lines +72 to +84
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Within the rAF callback, once dynamicElements becomes empty (after dynamicElements.delete(tagName)), the outer loop still iterates over all remaining pendingElements, doing no useful work. For large DOM mutation batches this adds avoidable work before paint. Consider short-circuiting out of the loops when dynamicElements.size === 0 (e.g., a labeled break) to keep the rAF callback bounded by the number of tagNames actually pending.

Copilot uses AI. Check for mistakes.
})
Comment on lines 63 to +85
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pendingElements.clear() runs at the end of the rAF callback, but scanTimer is set to null at the start of that callback. If scan() is invoked while this rAF callback is executing, it will schedule a new rAF and add elements into pendingElements, but those newly-added elements will be cleared by the currently-running callback, so the next rAF runs with an empty set and the scan is effectively dropped. To avoid losing scans, snapshot+clear pendingElements at the start of the rAF callback (process the snapshot), leaving any new elements added during processing for the next scheduled rAF.

Copilot uses AI. Check for mistakes.
}

let elementLoader: MutationObserver
Expand Down
32 changes: 32 additions & 0 deletions test/lazy-define.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,38 @@ describe('lazyDefine', () => {
expect(onDefine3).to.have.callCount(1)
})

it('coalesces multiple added elements into a single rAF callback', async () => {
const onDefine = spy()
lazyDefine('coalesce-test-element', onDefine)

const rafSpy = spy(window, 'requestAnimationFrame')
const callsBefore = rafSpy.callCount

await fixture(html`
<div>
<coalesce-test-element></coalesce-test-element>
<coalesce-test-element></coalesce-test-element>
<coalesce-test-element></coalesce-test-element>
<coalesce-test-element></coalesce-test-element>
<coalesce-test-element></coalesce-test-element>
<coalesce-test-element></coalesce-test-element>
<coalesce-test-element></coalesce-test-element>
<coalesce-test-element></coalesce-test-element>
<coalesce-test-element></coalesce-test-element>
<coalesce-test-element></coalesce-test-element>
</div>
`)

await animationFrame()

const rafCallsFromScan = rafSpy.callCount - callsBefore
rafSpy.restore()

// Should use at most a few rAF calls, not one per element
expect(rafCallsFromScan).to.be.lessThan(5)
expect(onDefine).to.be.callCount(1)
})
Comment on lines +75 to +101
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The requestAnimationFrame spy is only restored on the happy path. If fixture() or animationFrame() throws, rafSpy.restore() won't run and will leak the spy into later tests. Wrap the body in a try/finally (or use Sinon sandbox) to guarantee restore.

Copilot uses AI. Check for mistakes.

it('lazy loads elements in shadow roots', async () => {
const onDefine = spy()
lazyDefine('nested-shadow-element', onDefine)
Expand Down
Loading