Skip to content

perf(raycast): reduce renderer hook and registry churn#555

Open
JustYannicc wants to merge 51 commits into
SuperCmdLabs:mainfrom
JustYannicc:codex/consolidate-raycast-renderer-hooks
Open

perf(raycast): reduce renderer hook and registry churn#555
JustYannicc wants to merge 51 commits into
SuperCmdLabs:mainfrom
JustYannicc:codex/consolidate-raycast-renderer-hooks

Conversation

@JustYannicc

@JustYannicc JustYannicc commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

What changed

  • Ignore stale useFetch responses and stale promise hook completions across usePromise, useCachedPromise, and useStreamJSON.
  • Abort and ignore stale useStreamJSON lifecycle work on revalidation/unmount while composing caller-provided abort signals without mutating them.
  • Skip no-op form error state updates when setValue/setError would publish the same error state.
  • Use binary search for visible grid virtual row bounds while preserving section layout options.
  • Cache Raycast list/grid registry visible signatures to avoid full dirty-snapshot walks on equivalent re-renders.
  • Cache icon tint color resolution, readable tint DOM parsing, positive local icon asset paths, and transient missing asset paths.
  • Map launcher file-result commands by path for root-search candidate assembly while preserving first duplicate-path command semantics and result ordering.
  • Align the consolidated grid registry dirty-check harness with the integration stack's section object model.

Why

These renderer hot paths are hit during extension renders, hover/selection churn, virtualized grid scrolling, icon-heavy surfaces, JSON streaming hooks, and launcher file search. The consolidation keeps stale async completions from overwriting newer state and reduces repeated DOM, filesystem, registry, and launcher lookup work without changing the Raycast API surface.

Compatibility impact

Public Raycast hook, form, grid, list, icon, and launcher APIs are unchanged. Stale async runs are ignored only when a newer run has superseded them or the hook is unmounted. useStreamJSON observes caller-provided abort signals via a composed signal and does not abort caller-owned controllers. Missing local asset paths are cached only for a short stale window so newly-created assets can still resolve shortly after appearing. Launcher duplicate file-result paths continue to use the first command, matching the old .find semantics.

How tested

  • pnpm exec node --test scripts/test-use-fetch-stale-runs.mjs scripts/test-form-runtime-error-state-noop.mjs scripts/test-grid-virtualization.mjs scripts/test-icon-tint-cache.mjs scripts/test-registry-dirty-checks.mjs scripts/test-icon-asset-cache.mjs scripts/test-abortable-promise-hooks.mjs scripts/test-launcher-file-result-command-map.mjs scripts/test-use-stream-json-lifecycle.mjs
  • pnpm exec tsc -p tsconfig.renderer.json --noEmit --pretty false
  • pnpm exec vite build
  • git diff --check fork/codex/consolidate-raycast-renderer-hooks..HEAD
  • Codex LSP focused diagnostics on touched renderer TS/TSX files: use-fetch.ts, use-promise.ts, use-cached-promise.ts, use-stream-json.ts, useLauncherCommandModel.ts, launcher-file-candidates.ts, grid-runtime-hooks.ts, grid-runtime-virtualization.ts, list-runtime-hooks.ts, icon-runtime-assets.tsx, form-runtime.tsx, and form-runtime-state.ts.
  • Codex LSP references checked for the launcher file command map/candidate helpers and list/grid visible-signature builders.

Performance evidence

  • Grid virtualization harness: 5,000 eager cells before vs 30 visible cells after; 100,000-row visible range rendered 55 rows with 51 bound reads.
  • Icon asset cache harness: 10,000 repeated positive path resolutions used 1 statSync call; 10,000 repeated missing path resolutions within the stale window used 1 statSync call.
  • Icon tint cache harness: 10,000 repeated string tint resolutions used 1 element creation; readable tint repeat used 2 element creations, 1 body append, and 2 getComputedStyle calls.
  • Registry dirty-check harness: single recreated equivalent list/grid item avoided full 5,000-item snapshot walks and built 1 visible signature; recreated equivalent full sets avoided full snapshot walks and produced 0 version updates.
  • Launcher file candidate harness: 3,000-result assembly preserved first duplicate-path command/order and measured path-map median 26.381ms vs find-backed median 46.192ms with equal checksums.
  • Promise/fetch/stream stale-run harnesses cover latest-run wins for stale data and stale errors across default, abortable, revalidation, and unmount paths.
  • useStreamJSON before evidence from perf(utils): guard useStreamJSON lifecycle #554: the lifecycle harness failed before the hook change for stale data overwrite, stale error surfacing, missing unmount cleanup, and missing signal composition; it passes after this consolidation.

Stack validation

This branch starts from codex/perf-consolidated-integration-stack and adds only the Raycast renderer hook, registry, grid, form, icon, launcher file lookup, and useStreamJSON lifecycle fixes assigned here. Renderer typecheck and production renderer build pass on this stack.

Replaces

@shobhit99

Copy link
Copy Markdown
Contributor

Reviewed the Raycast-runtime portion of this stack (the 10 commits on top of codex/perf-consolidated-integration-stack): the promise/fetch/stream hooks, list/grid registries, grid virtualization, icon asset/tint caches, form error state, and launcher file-candidate extraction. No public Raycast API surface changes; hook return shapes and isLoading/data/error/mutate/revalidate semantics are preserved. This PR's own diff carries no user-facing strings (i18n N/A).

Strengths

  • Stale-run guarding is the real win. The monotonic latestRunIdRef/runIdRef guards in usePromise, useCachedPromise, useFetch, and useStreamJSON correctly make "latest run wins." For the non-abortable path this fixes a genuine bug: previously isCurrentRun = () => !runController || ... returned true for every concurrent run, so an out-of-order-resolving stale run could clobber newer state. The added latestRunIdRef.current === runId term is essential there and redundant-but-harmless for the abortable path (each execute reassigns abortControllerRef in lock-step). Sequential pagination in useCachedPromise/useFetch is unaffected (the appending page is always the latest run).
  • useStreamJSON abort composition is well done. composeAbortSignal observes the lifecycle signal + the caller's signal and aborts only its own controller, never the caller's — matching the PR claim. It correctly distinguishes caller-abort (surfaces the error, as before) from lifecycle-abort (lifecycleController.signal.aborted / superseded runId → suppressed), removes listeners in finally, and only nulls abortControllerRef when it still owns it, so a late-resolving superseded run can't clobber the new run's ref or isLoading.
  • Registry churn reduction is correct and even fixes a gap. Per-item visible signatures replace the O(n) full-snapshot walk on every microtask. The list/grid signatures are a superset of the old snapshot fields — grid now additionally tracks content/subtitle/accessory/keywords/quickLook, which the old grid snapshot omitted — so there are no missed version bumps (no stale rows), and grid content changes are now detected. The early-returns fire only on provably no-visible-change. Critically, because the integration base assigns order via a stable orderRef (assigned once, dropped from the useLayoutEffect deps in list-runtime-renderers.tsx / grid-runtime-items.tsx), registry.set is not invoked on selection/hover re-renders, so adding orderChanged/String(order) to the signature introduces no per-keystroke churn.
  • Grid virtualization binary search (grid-runtime-virtualization.ts) is correct for a monotonic row layout: slice(firstBottom≥start, firstTop>end) reproduces the old filter's top+height >= start && top <= end boundaries exactly, including empty-range edges.
  • Icon caches key theme-dependent color/tint results by a theme signature + version (invalidated via a MutationObserver on documentElement class/style), cache theme-independent normalization/validity separately, and correctly exclude var()/env()/currentcolor/inherit/... from caching. Negative asset-path cache uses a 250 ms TTL so freshly-created assets still resolve.
  • launcher-file-candidates.ts replaces a per-result O(n) .find(item.path === result.path) with a prebuilt first-wins Map, preserving both the first-duplicate-wins semantics and result ordering; otherwise a faithful extraction. Form no-op skips return the same reference (React bails the re-render) and skip the redundant setCurrentFormErrors publish while the global stays in sync.

Concerns / potential bugs

  • [Low] use-fetch.ts — function-URL effect keying. The init effect dep changed from a constant urlString ('function' for function URLs) to url, with dedup on the resolved page-0 URL via lastInitialRequestKeyRef. This is a net improvement — it fixes a real staleness bug where a function URL embedding changing state (e.g. () => \...?q=${query}`) never refetched under the old constant key. The edge risk: a function URL that resolves *non-deterministically* (embeds Date.now()/ a random nonce) now yields a newinitialRequestKeyevery render, sofetchData(0)fires each render (each fetch togglesisLoading` → re-render → refetch), i.e. a fetch loop. This pattern is unusual/arguably already broken, but it's a behavior change worth a note.
  • [Very low] icon-runtime-assets.tsx readable-tint cache. Theme detection keys off the documentElement dark class + class/style signature. If a theme CSS var like --surface-base-rgb ever changed without a corresponding class/style mutation on documentElement (e.g. a stylesheet swap), a cached readable tint could be briefly stale. Low risk given the app toggles theme via the dark class; cosmetic only.
  • [Nit] parseCssColorToRgb removes the temp <span> inside the try after getComputedStyle; if getComputedStyle itself threw, the span would leak. Practically impossible, and still strictly better than the previous un-guarded version.

Suggestions

  • Consider whether use-fetch should snapshot typeof url === 'function' and only re-key on the resolved URL when the function identity and resolved value both change, to fully neutralize the non-deterministic-URL edge — or just document the expectation that function URLs resolve deterministically.

🟡 Approve with minor comments — behavior/API is preserved and the stale-run and abort fixes are genuine improvements; only low-severity edge cases noted.

🤖 Automated review by Claude Code (Opus 4.8).

@JustYannicc

Copy link
Copy Markdown
Collaborator Author

Addressed the review concerns in 1fd471e:

  • Hardened useFetch function URL keying so hook-owned state renders do not re-resolve a non-deterministic URL function into a fetch loop, while caller rerenders with a changed resolved URL still refetch.
  • Extended readable tint cache invalidation to observe stylesheet/head changes in addition to documentElement class/style changes.
  • Moved temporary CSS color parse element cleanup into finally so a thrown getComputedStyle cannot leak the node.
  • Added regression coverage for the non-deterministic URL case, stylesheet-driven tint invalidation, and temporary element cleanup on parser failure.

Proof run locally:

  • node scripts/test-use-fetch-stale-runs.mjs
  • node scripts/test-icon-tint-cache.mjs
  • node scripts/test-registry-dirty-checks.mjs
  • node scripts/test-abortable-promise-hooks.mjs
  • node scripts/test-use-stream-json-lifecycle.mjs
  • node scripts/test-icon-asset-cache.mjs
  • ./node_modules/.bin/tsc -p tsconfig.renderer.json --noEmit

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.

2 participants