fix(app): eliminate terminal scroll lag by removing MutationObserver and throttling scroll events#1622
fix(app): eliminate terminal scroll lag by removing MutationObserver and throttling scroll events#1622TommyLike wants to merge 1 commit into
Conversation
…and throttling scroll events The MutationObserver with subtree:true was firing React setState on every xterm DOM mutation (character output, cursor blink, scroll line rendering), causing cascading React re-renders that blocked the main thread and made the mouse wheel unresponsive on web. Changes: - Remove MutationObserver (redundant with ResizeObserver for viewport sizing) - Throttle scroll handler with requestAnimationFrame to batch setState calls - Early-return in updateViewportMetrics when metrics unchanged since last emit - Cancel pending rAF on cleanup to avoid stale updates Co-Authored-By: TommyLike <tommylikehu@gmail.com>
|
| Filename | Overview |
|---|---|
| packages/app/src/components/terminal-emulator.tsx | Removes the redundant MutationObserver, adds rAF-based scroll throttle, and deduplicates setState via lastMetricsRef — good approach, but the ref is not reset in the viewport-not-found early-return branch, creating a potential state/ref mismatch. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[DOM scroll / resize / xterm mutation] --> B{Event source}
B -->|scroll event| C[handleViewportScroll]
B -->|ResizeObserver| E[updateViewportMetrics - direct]
C --> D{scrollRafId pending?}
D -->|yes| F[drop event]
D -->|no| G[requestAnimationFrame]
G --> H[updateViewportMetrics]
E --> H
H --> I{metrics changed vs lastMetricsRef?}
I -->|no change| J[early return — no setState]
I -->|changed| K[update lastMetricsRef]
K --> L[setViewportMetrics setState]
L --> M[scrollbar visibility effect]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[DOM scroll / resize / xterm mutation] --> B{Event source}
B -->|scroll event| C[handleViewportScroll]
B -->|ResizeObserver| E[updateViewportMetrics - direct]
C --> D{scrollRafId pending?}
D -->|yes| F[drop event]
D -->|no| G[requestAnimationFrame]
G --> H[updateViewportMetrics]
E --> H
H --> I{metrics changed vs lastMetricsRef?}
I -->|no change| J[early return — no setState]
I -->|changed| K[update lastMetricsRef]
K --> L[setViewportMetrics setState]
L --> M[scrollbar visibility effect]
Comments Outside Diff (1)
-
packages/app/src/components/terminal-emulator.tsx, line 576-581 (link)lastMetricsRefis not reset in the "viewport not found" early-return path. If the effect fires once without a viewport (settingviewportMetricsstate to{0,0,0}), and then re-fires with a viewport whose real DOM values happen to match the stale ref (e.g., same terminal dimensions as a prior session),updateViewportMetricswill pass the equality check and skip thesetState— leaving the state frozen at{0,0,0}while the DOM is fully populated. The scrollbar would then stay permanently hidden.
Reviews (1): Last reviewed commit: "fix(app): eliminate terminal scroll lag ..." | Re-trigger Greptile
Summary
Fixes terminal scroll freezing and mouse wheel unresponsiveness on the web version by removing a redundant
MutationObserverthat was flooding React withsetStatecalls on every xterm DOM mutation.Closes #1621.
Root Cause
The
MutationObserverinterminal-emulator.tsxobserved the entire xterm DOM subtree (subtree: true+childList: true+attributes: true) and fired ReactsetStateon every single mutation. Since xterm.js aggressively mutates its DOM during rendering (every character, cursor blink, scroll line), this created a cascade of React reconciliation passes that blocked the main thread.The
ResizeObserver(already present) covers the legitimate use case — it watches.xterm-viewportand.xterm-scroll-areafor size changes. TheMutationObserverwas completely redundant.Changes
MutationObserver— redundant with the existingResizeObserverrequestAnimationFrame— batchessetStatecalls to at most one per frameupdateViewportMetrics— skipssetStateif metrics unchanged since last emissionFiles Modified
packages/app/src/components/terminal-emulator.tsxTesting
Co-Authored-By: TommyLike tommylikehu@gmail.com