fix: batch updateClipPath via requestAnimationFrame#9173
Merged
christian-byrne merged 1 commit intomainfrom Feb 27, 2026
Merged
fix: batch updateClipPath via requestAnimationFrame#9173christian-byrne merged 1 commit intomainfrom
christian-byrne merged 1 commit intomainfrom
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR adds performance optimization and comprehensive test coverage to the useDomClipping composable. The implementation introduces RAF-based batching to defer DOM measurements and clip-path calculations, reducing layout thrashing. A new test suite validates the coalescing behavior, style updates, and cancellation logic. Changes
Sequence DiagramsequenceDiagram
participant Caller as Component
participant Composable as useDomClipping
participant RAF as requestAnimationFrame
participant DOM as DOM API
participant Style as Element.style
Note over Caller,Style: Multiple rapid calls scenario
Caller->>Composable: updateClipPath(element1)
Composable->>RAF: cancelAnimationFrame(pendingRaf)
Composable->>RAF: requestAnimationFrame(callback)
Note right of RAF: pendingRaf = id1
Caller->>Composable: updateClipPath(element2)
Composable->>RAF: cancelAnimationFrame(id1)
Composable->>RAF: requestAnimationFrame(callback)
Note right of RAF: pendingRaf = id2, only id2 executes
RAF->>Composable: callback() fires
Composable->>DOM: getBoundingClientRect(element2)
DOM-->>Composable: rect2
Composable->>DOM: getBoundingClientRect(canvas)
DOM-->>Composable: canvasRect
Composable->>Composable: calculateClipPath(rect2, canvasRect)
Composable->>Style: update clip-path & willChange
Style-->>Composable: style updated
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
🎭 Playwright: ✅ 549 passed, 0 failed · 4 flaky📊 Browser Reports
|
🎨 Storybook: ✅ Built — View Storybook |
christian-byrne
force-pushed
the
perf/fix-clippath-raf
branch
from
cc01036 to
a10603c
Compare
⚡ Performance Report
Raw data{
"timestamp": "2026-02-26T04:15:47.016Z",
"gitSha": "6a790ce8ce5cea6d52d785af663ad2944a046573",
"branch": "perf/fix-clippath-raf",
"measurements": [
{
"name": "canvas-idle",
"durationMs": 2029.500999999982,
"styleRecalcs": 123,
"styleRecalcDurationMs": 29.813,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 493.127,
"heapDeltaBytes": -2025752
},
{
"name": "canvas-mouse-sweep",
"durationMs": 2042.8150000000187,
"styleRecalcs": 187,
"styleRecalcDurationMs": 67.989,
"layouts": 12,
"layoutDurationMs": 4.443,
"taskDurationMs": 988.272,
"heapDeltaBytes": -3505488
},
{
"name": "dom-widget-clipping",
"durationMs": 566.7169999999828,
"styleRecalcs": 42,
"styleRecalcDurationMs": 13.960000000000003,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 387.579,
"heapDeltaBytes": 7965944
}
]
} |
christian-byrne
force-pushed
the
perf/fix-clippath-raf
branch
5 times, most recently
from
59043e6 to
f6bcb02
Compare
Wrap getBoundingClientRect() calls in updateClipPath inside requestAnimationFrame() with cancellation. Multiple rapid calls within the same frame are coalesced into a single layout read. Eliminates ~1,053 forced synchronous layouts per profiling session that were interleaved with style mutations from PrimeVue and cursor updates, creating layout thrashing in Firefox. Includes unit test validating RAF coalescing and cancellation. Amp-Thread-ID: https://ampcode.com/threads/T-019c8ed0-59ad-720b-bc4f-6f52dc452844
christian-byrne
force-pushed
the
perf/fix-clippath-raf
branch
from
f6bcb02 to
f0808a5
Compare
dosubot
bot
added
the
size:L
📦 Bundle: 4.43 MB gzip 🔴 +54 BDetailsSummary
Category Glance App Entry Points — 17.9 kB (baseline 17.9 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 986 kB (baseline 986 kB) • 🔴 +147 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 72.1 kB (baseline 72.1 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Panels & Settings — 435 kB (baseline 435 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
User & Accounts — 16 kB (baseline 16 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Editors & Dialogs — 736 B (baseline 736 B) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
UI Components — 47.1 kB (baseline 47.1 kB) • ⚪ 0 BReusable component library chunks
Data & Services — 2.54 MB (baseline 2.54 MB) • ⚪ 0 BStores, services, APIs, and repositories
Utilities & Hooks — 55.5 kB (baseline 55.5 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Vendor & Third-Party — 8.84 MB (baseline 8.84 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 7.74 MB (baseline 7.74 MB) • ⚪ 0 BBundles that do not match a named category
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/composables/element/useDomClipping.ts (1)
108-125: Consider cancelling pending RAF on scope disposal.The RAF batching implementation is correct and achieves the performance goal. However, if the composable's scope is disposed (e.g., component unmounts) while a RAF is pending, the callback will still execute.
This is low-risk since Vue refs tolerate writes after unmount, but for completeness you could cancel the pending RAF when the scope is disposed using
onScopeDispose.♻️ Optional cleanup on scope disposal
+import { onScopeDispose } from 'vue' import type { CSSProperties } from 'vue' import { ref } from 'vue'let pendingRaf = 0 + + onScopeDispose(() => { + if (pendingRaf) cancelAnimationFrame(pendingRaf) + })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/composables/element/useDomClipping.ts` around lines 108 - 125, The pending requestAnimationFrame created in useDomClipping (pendingRaf/requestAnimationFrame block) isn't cancelled when the composable's scope is disposed; add an onScopeDispose handler in the same composable that checks pendingRaf and calls cancelAnimationFrame(pendingRaf) (and resets pendingRaf to 0) to prevent the RAF callback from running after unmount and touching style.value or DOM.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/composables/element/useDomClipping.ts`:
- Around line 108-125: The pending requestAnimationFrame created in
useDomClipping (pendingRaf/requestAnimationFrame block) isn't cancelled when the
composable's scope is disposed; add an onScopeDispose handler in the same
composable that checks pendingRaf and calls cancelAnimationFrame(pendingRaf)
(and resets pendingRaf to 0) to prevent the RAF callback from running after
unmount and touching style.value or DOM.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/composables/element/useDomClipping.test.tssrc/composables/element/useDomClipping.ts
christian-byrne
added a commit
that referenced
this pull request
Feb 27, 2026
## Summary Batch `getBoundingClientRect()` calls in `updateClipPath` via `requestAnimationFrame` to avoid forced synchronous layout. ## Changes - **What**: Wrap the layout-reading portion of `updateClipPath` in `requestAnimationFrame()` with cancellation. Multiple rapid calls within the same frame are coalesced into a single layout read. Eliminates ~1,053 forced synchronous layouts per profiling session. ## Review Focus - `getBoundingClientRect()` forces synchronous layout. When interleaved with style mutations (from PrimeVue `useStyle`, cursor writes, Vue VDOM patching), this creates layout thrashing — especially in Firefox where Stylo aggressively invalidates the entire style cache. - The RAF wrapper coalesces all calls within a frame into one, reading layout only once per frame. The `cancelAnimationFrame` ensures only the latest parameters are used. - `willChange: 'clip-path'` is included to hint the browser to optimize clip-path animations. ## Stack 4 of 4 in Firefox perf fix stack. Depends on #9170. <!-- Fixes #ISSUE_NUMBER --> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9173-fix-batch-updateClipPath-via-requestAnimationFrame-3116d73d3650810392f7fba7ea5ceb6f) by [Unito](https://www.unito.io)
christian-byrne
added
core/1.40
github-actions bot
pushed a commit
that referenced
this pull request
Mar 8, 2026
## Summary Batch `getBoundingClientRect()` calls in `updateClipPath` via `requestAnimationFrame` to avoid forced synchronous layout. ## Changes - **What**: Wrap the layout-reading portion of `updateClipPath` in `requestAnimationFrame()` with cancellation. Multiple rapid calls within the same frame are coalesced into a single layout read. Eliminates ~1,053 forced synchronous layouts per profiling session. ## Review Focus - `getBoundingClientRect()` forces synchronous layout. When interleaved with style mutations (from PrimeVue `useStyle`, cursor writes, Vue VDOM patching), this creates layout thrashing — especially in Firefox where Stylo aggressively invalidates the entire style cache. - The RAF wrapper coalesces all calls within a frame into one, reading layout only once per frame. The `cancelAnimationFrame` ensures only the latest parameters are used. - `willChange: 'clip-path'` is included to hint the browser to optimize clip-path animations. ## Stack 4 of 4 in Firefox perf fix stack. Depends on #9170. <!-- Fixes #ISSUE_NUMBER --> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9173-fix-batch-updateClipPath-via-requestAnimationFrame-3116d73d3650810392f7fba7ea5ceb6f) by [Unito](https://www.unito.io)
Member
|
@christian-byrne Successfully backported to #9588 |
github-actions
bot
removed
the
needs-backport
christian-byrne
added a commit
that referenced
this pull request
Mar 8, 2026
…ame (#9588) Backport of #9173 to `core/1.40` Automatically created by backport workflow. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9588-backport-core-1-40-fix-batch-updateClipPath-via-requestAnimationFrame-31d6d73d365081f7a8cffad0de3ab38b) by [Unito](https://www.unito.io) Co-authored-by: Christian Byrne <cbyrne@comfy.org>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Batch
getBoundingClientRect()calls inupdateClipPathviarequestAnimationFrameto avoid forced synchronous layout.Changes
updateClipPathinrequestAnimationFrame()with cancellation. Multiple rapid calls within the same frame are coalesced into a single layout read. Eliminates ~1,053 forced synchronous layouts per profiling session.Review Focus
getBoundingClientRect()forces synchronous layout. When interleaved with style mutations (from PrimeVueuseStyle, cursor writes, Vue VDOM patching), this creates layout thrashing — especially in Firefox where Stylo aggressively invalidates the entire style cache.cancelAnimationFrameensures only the latest parameters are used.willChange: 'clip-path'is included to hint the browser to optimize clip-path animations.Stack
4 of 4 in Firefox perf fix stack. Depends on #9170.
┆Issue is synchronized with this Notion page by Unito