-
Notifications
You must be signed in to change notification settings - Fork 7
Description
Background
React 18 StrictMode in dev intentionally double-invokes mount effects (mount -> cleanup -> mount) to surface side effects.
Historically we kept <React.StrictMode> disabled in src/main.tsx because the xterm.js integration was not safe under double-mount and would break rendering / lifecycle.
During the pane-architecture merge, <React.StrictMode> was re-enabled in src/main.tsx even though the underlying StrictMode-safety work was not complete, which appears to be responsible for a bunch of runtime-only regressions (xterm renderer issues / double init/cleanup).
StrictMode was reverted again in 98057fc and we added test/unit/client/main-entry.test.ts as a guard.
Goal
Actually make the client StrictMode-safe (at least: TerminalView/xterm + WS terminal lifecycle) and then re-enable <React.StrictMode> in src/main.tsx.
Repro (today)
- Temporarily wrap the root render in
<React.StrictMode>insrc/main.tsx. - Run
npm run dev. - Create/open terminal panes and interact.
- Observe xterm rendering issues / lifecycle weirdness that does not appear when StrictMode is off.
Exact symptoms can vary, but the root cause is the same: effects run twice in dev and some side effects are not fully idempotent and/or not fully cleaned up.
What Needs To Happen
1) Audit StrictMode-sensitive side effects
Focus on:
src/components/TerminalView.tsx(xterm + addons init, DOM listeners, resize observers, etc.)src/lib/ws-client.ts(message handlers, subscriptions)src/components/panes/PaneContainer.tsxandsrc/store/paneCleanupListeners.ts(terminal detach/kill decisions)
React StrictMode will run effects twice. After a full mount -> unmount -> mount cycle, we must be back in a correct state.
2) Make the xterm integration idempotent + cleanup-complete
- Ensure a mount always creates a fresh xterm instance and a cleanup always disposes it (and all addons) without leaving stray listeners/observers.
- Ensure we never end up with multiple xterm instances attached to the same DOM node.
- Ensure any event listeners registered on
window/documentare removed in cleanup.
3) Make backend terminal lifecycle safe under StrictMode remounts
StrictMode remounts must not create duplicate backend terminals, nor accidentally kill/detach terminals.
- Verify
createRequestIdis stable across remounts (should live in pane content state). - Verify the client does not send irreversible cleanup on unmount. Killing/detaching should be driven by pane removal from Redux state (close tab/pane), not component unmount.
- Ensure reconnect / attach logic is resilient to fast detach+attach sequences.
4) Add tests that prove StrictMode-safety
Suggested tests:
- A unit/integration test that renders the relevant pane/TerminalView under
<React.StrictMode>and asserts:- no duplicate
terminal.createfor a single pane/createRequestId (or duplicates are reused/attached) - no
terminal.create.cancel { kill: true }is emitted during StrictMode remount - no console errors (we already fail tests on unexpected
console.error)
- no duplicate
- Mock
xtermto assert cleanup disposes instances/addons and re-init works on remount.
Once StrictMode is re-enabled, update/remove test/unit/client/main-entry.test.ts (currently asserts StrictMode is not used).
5) Re-enable StrictMode in the entrypoint
- Put
<React.StrictMode>back insrc/main.tsx. - Keep the new tests in CI to prevent regressions.
Acceptance Criteria
- With
<React.StrictMode>enabled insrc/main.tsxin dev, basic terminal usage works (no renderer issues). - No duplicate backend terminals are created for a single pane due to dev double-mount.
- Test coverage exists for StrictMode behavior;
npm testpasses.