Skip to content

fix(theme): cancel mesh-gradient timers on teardown (#5160) - #5273

Merged
M3gA-Mind merged 2 commits into
tinyhumansai:mainfrom
M3gA-Mind:fix/5160-null-classlist
Jul 31, 2026
Merged

fix(theme): cancel mesh-gradient timers on teardown (#5160)#5273
M3gA-Mind merged 2 commits into
tinyhumansai:mainfrom
M3gA-Mind:fix/5160-null-classlist

Conversation

@M3gA-Mind

@M3gA-Mind M3gA-Mind commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Gradient.disconnect() now cancels every pending async chain it owns — the waitForCssVars rAF retry loop, the animate rAF loop, the 3s deferred isLoaded class, and the scroll-end debounce — instead of leaking them past unmount.
  • Adds a destroyed latch that init() / waitForCssVars() / play() / animate() bail on, so a frame already dispatched in the current tick cannot restart the retry chain or dereference an absent mesh.
  • pause() now actually cancels the queued frame (the contract its call site in MeshGradient.tsx already documented) and stops assuming conf exists.
  • resize() ignores events arriving without a live minigl/mesh.
  • New app/src/lib/meshGradient.test.ts — 10 lifecycle regression tests, 6 of which fail against the previous implementation.
  • init() now stores its opening animate frame on animateRaf, so disconnect() can cancel it too (CodeRabbit review).

Problem

<MeshGradient /> (the Stripe-style WebGL backdrop) unmounts on every theme switch, backdrop change and window teardown. Its Gradient instance owns three async chains that outlive the canvas React gave it:

  1. the waitForCssVars rAF retry loop (up to 200 frames),
  2. the animate rAF loop,
  3. a 3-second deferred isLoaded class.

disconnect() cancelled none of them — it only removed the resize listener (its scroll-listener block is dead code, since scrollObserver is permanently undefined). So after unmount those callbacks kept firing against a canvas React had already removed, and the 3s timeout evaluated this.el.parentElement.classList on a detached node → parentElement === nullTypeError: Cannot read properties of null (reading 'classList').

That matches the Sentry signature reported in #5160: a 2-frame stack (the outer frame is browserApiErrorsIntegration's setTimeout wrapper, which services/analytics.ts enables explicitly; the inner one is the anonymous arrow), the src/lib/meshGradient culprit on the one shortId that resolved, and 5 shortIds that are the same defect across 5 release bundle hashes rather than 5 distinct bugs — 27 events / 13 users.

For completeness: no other unguarded .classList read exists in the shipped bundle. The two remaining reads in app/src are on document.documentElement (never null), and no bundled runtime dependency reads classList off a nullable node — the dom-helpers / react-transition-group path is unreachable here because react-smooth never calls into it.

PR #5171 added this.el && this.el.parentElement && guards at the crash site. That stopped the throw but left the leak in place — timers and frames still running after teardown, one guard deletion away from re-opening.

Solution

Make the lifecycle cancellable at the root rather than guarding the last dereference:

  • Every async entry point records its handle (isLoadedTimeout, cssVarsRaf, animateRaf) on the instance.
  • disconnect() latches destroyed, clears both timeouts, cancels both rAF handles, and stays idempotent.
  • init() / waitForCssVars() / play() / animate() return early when destroyed, covering a callback that was already dispatched before disconnect() ran.
  • The isLoaded timeout keeps a detached-node check as a second line of defence for a teardown we never observed, and logs why it skipped.
  • Each teardown decision logs under the grep-friendly [MeshGradient] prefix.

The existing this.el && … guards from #5171 are retained — they are now unreachable in the normal path but cost nothing.

Submission Checklist

  • Tests added or updated (happy path + at least one failure / edge case) — app/src/lib/meshGradient.test.ts: happy path (canvas still mounted at 3s), plus disconnect-first, detached-without-disconnect, retry-loop-after-disconnect, frame-after-disconnect, pause/play, resize-after-teardown and double-disconnect
  • Diff coverage ≥ 80% — the new suite drives every changed branch in meshGradient.js; the .d.ts is type-only
  • N/A: bug fix to an existing feature — no matrix row added, removed, or renamed. Coverage matrix updated
  • N/A: no matrix rows affected, so no feature IDs apply. All affected feature IDs from the matrix are listed under ## Related
  • No new external network dependencies introduced — tests are pure jsdom, no WebGL and no network
  • N/A: no release-cut surface changed; the gradient renders identically. Manual smoke checklist updated
  • Linked issue closed via Closes #NNN in the ## Related section

Impact

  • Platform: desktop (all three OSes) — the animated backdrop is desktop-only UI. No core/Rust, backend or CLI surface touched.
  • Behaviour: unchanged while mounted. The only user-visible difference is that a gradient torn down inside the 3s isLoaded window no longer marks its (already removed) wrapper as loaded.
  • Performance: strictly better — a stale Gradient no longer holds a self-rescheduling rAF loop alive after unmount, which was burning frames indefinitely on repeated theme switches. Related in spirit to the MacBook Air在运行app时,计算机持续发烫 #3524 idle-GPU work.
  • Security / migration / compatibility: none.

Related


AI Authored PR Metadata (required for Codex/Linear PRs)

Linear Issue

  • Key: N/A
  • URL: N/A

Commit & Branch

  • Branch: fix/5160-null-classlist
  • Commit SHA: 1f151cf69

Validation Run

  • pnpm --filter openhuman-app format:checkprettier --check clean on all three changed files
  • pnpm typecheck — clean (the earlier @xyflow/react failure was a stale local install of an unrelated dependency; resolved after a reinstall, and it never affected src/lib/meshGradient*)
  • Focused tests: vitest related src/lib/meshGradient.* → 5 files / 66 tests passed; meshGradient.test.ts alone → 10 passed. The new init()-frame test fails against the pre-fix lib ("expected [] to include 1")
  • N/A: no Rust touched. Rust fmt/check (if changed)
  • N/A: no Tauri shell code touched. Tauri fmt/check (if changed)

Validation Blocked

  • command: N/A — nothing is blocked. The previously-reported pnpm typecheck failure (Cannot find module '@xyflow/react') was a stale local node_modules; it is clean after a reinstall.
  • error: N/A
  • impact: N/A

Behavior Changes

  • Intended behavior change: a disconnected Gradient performs no further DOM or WebGL work.
  • User-visible effect: none while the backdrop is mounted; a Sentry error family affecting ~13 users stops firing.

Parity Contract

Duplicate / Superseded PR Handling

  • Duplicate PR(s): none
  • Canonical PR: this one
  • Resolution: N/A

Summary by CodeRabbit

  • Bug Fixes

    • Improved gradient teardown behavior when components are unmounted or canvases are detached.
    • Prevented delayed animations, styling updates, and CSS variable checks from running after disconnection.
    • Made pause, play, resize, and repeated disconnect operations safer.
  • Documentation

    • Expanded the gradient API declarations with lifecycle methods and teardown state information.
  • Tests

    • Added coverage for asynchronous cleanup, detached canvases, animation cancellation, and repeated teardown.

`Gradient` owns three async chains that outlive the canvas React gives it:
the `waitForCssVars` rAF retry loop (up to 200 frames), the `animate` rAF
loop, and the 3s deferred `isLoaded` class. `disconnect()` cancelled none of
them, so every `<MeshGradient />` unmount — a theme switch, a backdrop
change, window teardown — left callbacks queued against a canvas React had
already removed. The `isLoaded` timeout then read
`this.el.parentElement.classList` on a detached node, which is the
"Cannot read properties of null (reading 'classList')" family Sentry grouped
across five bundle hashes (27 events / 13 users). PR tinyhumansai#5171 guarded the
expression; this removes the reason it was ever reached.

Each async entry point now records its handle and bails on `destroyed`:

- `disconnect()` latches `destroyed`, clears the `isLoaded` timeout and the
  scroll-end debounce, and cancels both rAF handles. Idempotent.
- `init()` / `waitForCssVars()` / `play()` / `animate()` return early once
  disconnected, so a frame already dispatched in the current tick cannot
  restart the retry chain or dereference the absent `mesh`.
- `pause()` cancels the queued frame (which its documented contract in
  `MeshGradient.tsx` already claimed) and no longer assumes `conf` exists —
  the wrapper calls it right after `disconnect()` on gradients whose WebGL
  init bailed early.
- `resize()` ignores events that arrive without a live `minigl`/`mesh`.

Regression tests in `app/src/lib/meshGradient.test.ts` drive the lifecycle
directly (no WebGL): five of the nine fail against the previous
implementation. Teardown decisions are logged under `[MeshGradient]`.
@M3gA-Mind
M3gA-Mind requested a review from a team July 30, 2026 13:01

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

M3gA-Mind has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Gradient now tracks teardown state and pending asynchronous handles, cancels them during disconnect(), guards lifecycle callbacks after destruction, and adds tests covering detached canvases, queued frames, CSS polling, initialization, resizing, and repeated teardown.

Changes

Gradient teardown safety

Layer / File(s) Summary
Lifecycle state and public contract
app/src/lib/meshGradient.d.ts, app/src/lib/meshGradient.js
The Gradient API exposes lifecycle methods and destruction state; runtime instances track pending timeout and animation-frame handles.
Async guards and disconnect cancellation
app/src/lib/meshGradient.js
Animation, CSS-variable polling, initialization, resizing, and deferred class updates stop or skip work after destruction; disconnect() cancels pending work and removes listeners.
Teardown behavior validation
app/src/lib/meshGradient.test.ts
Tests cover mounted and detached canvases, canceled callbacks, post-disconnect lifecycle calls, absent rendering state, and idempotent disconnects.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Suggested reviewers: al629176

Poem

A rabbit watched the gradients glow,
Then safely helped their timers go.
Frames were paused, the canvas stayed bright,
No lost callback caused a fright.
“Disconnect twice? That’s perfectly right!”

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes address #5160 by canceling async work on disconnect and preventing detached callbacks from touching classList.
Out of Scope Changes check ✅ Passed No clear unrelated changes stand out; the type declaration updates support the teardown-related runtime changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and accurately summarizes the main teardown fix for mesh-gradient timers.

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@app/src/lib/meshGradient.js`:
- Around line 710-713: Store the initial requestAnimationFrame handle in
this.animateRaf within init() so disconnect() can cancel it; update
app/src/lib/meshGradient.js lines 710-713 accordingly. Add a test in
app/src/lib/meshGradient.test.ts lines 118-126 that stubs rendering setup,
initializes a live gradient, disconnects it, and asserts cancellation of the
stored frame handle.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: bab96ceb-d6ea-43bf-b4f6-eb27e68282d3

📥 Commits

Reviewing files that changed from the base of the PR and between bb83836 and 18d48d9.

📒 Files selected for processing (3)
  • app/src/lib/meshGradient.d.ts
  • app/src/lib/meshGradient.js
  • app/src/lib/meshGradient.test.ts

Comment thread app/src/lib/meshGradient.js
…ncels it

`init()` scheduled its first `animate` frame without storing the handle, so
`disconnect()` — which only cancels `this.animateRaf` — had nothing to cancel
and that opening frame outlived teardown. Every other scheduling site
(`animate`, `play`, the `!playing` branch) already assigns to `animateRaf`;
this one was the outlier, and it is the same leak class this change set exists
to close.

Assign the handle and add a regression test that stubs the WebGL setup so
`init()` reaches its `requestAnimationFrame`, then asserts `disconnect()`
cancels that specific handle. Verified failing before the fix ("expected [] to
include 1").

`initGradientColors` joins the lifecycle-internals block in `meshGradient.d.ts`
so the test can stub it — same rationale as the `init`/`initMesh`/`resize`
entries already there.

Addresses the CodeRabbit review on tinyhumansai#5273.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

M3gA-Mind has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

@M3gA-Mind
M3gA-Mind merged commit 0294a8e into tinyhumansai:main Jul 31, 2026
21 of 24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TypeError: Cannot read properties of null (reading 'classList') — multiple bundles

1 participant