Skip to content

fix(desktop): keep dev renderer on one Vite dep-optimizer generation - #4782

Merged
me2seeks merged 1 commit into
apache:mainfrom
me2seeks:fix/desktop-dev-optimizer-stale-cache
Sep 5, 2026
Merged

fix(desktop): keep dev renderer on one Vite dep-optimizer generation#4782
me2seeks merged 1 commit into
apache:mainfrom
me2seeks:fix/desktop-dev-optimizer-stale-cache

Conversation

@me2seeks

@me2seeks me2seeks commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Summary

In dev sessions, opening Settings (or any lazily loaded component that runs hooks) could crash the renderer with TypeError: Cannot read properties of null (reading 'useRef'). The page was executing two React module instances: freshly transformed sources import the current generation's react.js, while react-dom and several chunks were resurrected from the persistent Electron HTTP cache out of a previous Vite dep-optimizer generation. The dispatcher is only set on the instance react-dom renders with; hooks called through the other instance see null and throw.

Two changes, both dev-only (packaged builds load file:// and are unaffected):

  1. main-renderer-dev-cache.ts (new): clear the session HTTP cache before loading a Vite dev server, so immutable ?v=<browserHash> chunks from older generations can never be resurrected. Failure downgrades to a warning instead of blocking window creation. Wired into main-window.ts right before loadMainRenderer.
  2. dev.mjs: after server.listen(), warm the renderer entry (warmupRequest('/main.tsx')) and waitForRequestsIdle() before launching Electron, so the initial dependency crawl / optimizer commit lands before the window starts loading modules.

Fixes #4775

Verification

  • tsc -p tsconfig.main.json --noEmit
  • npm --workspace @maka/desktop run build:main
  • node --test dist/main/__tests__/main-renderer-dev-cache.test.js — 3 pass (clears in dev / untouched for packaged / failure downgrades) ✓
  • node --test scripts/dev-app-runtime.test.mjs
  • biome check on touched files ✓
  • Live API check: standalone Vite 8.2.2 server with the repo config (throwaway cacheDir): environments.client.warmupRequest('/main.tsx') + waitForRequestsIdle() settle in ~2.4s with the entry graph crawled and the dep optimizer bundling ✓
  • Not run: full npm test, e2e suites (heavier than this change; happy to run on request).

Root cause

Evidence gathered on the affected machine (full write-up in #4775):

  • The crash stack mixes two ?v= browser hashes: react.js?v=3911c03d (current generation) vs. client-CwWlPFAR.js, ToastViewport-*.js, Layer-*.js, i18n-*.js, theme-*.js all at ?v=d164f9b2 (a generation no longer on disk).
  • The Electron HTTP cache (~/.config/Maka Dev/Cache) held multiple historical generations, e.g. a cached react-dom_client.js?v=c163ffce whose body imports client-CwWlPFAR.js?v=d164f9b2, which imports react.js?v=4a8fd6a8 — an intact stale chunk graph, loadable forever thanks to Cache-Control: immutable.
  • node_modules/.vite/deps_temp_* leftovers show mid-session re-optimizations happen in this workspace (late-discovered deps), each rotating browserHash.
  • dev.mjs launched Electron immediately after server.listen(); observed timings had the deps commit land at T+2.4s and the app ready at T+3.2s — page load raced the generation swap.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Maka (AI agent) — reproduced the diagnosis from on-disk evidence (Vite optimizer metadata, Electron HTTP cache), authored the patch and its tests. Commit carries the Generated-by: Maka trailer.

Checklist

  • Tests cover the change and fail without it (dev-cache helper: clears in dev / leaves packaged builds alone / degrades to warning)
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

Opening Settings in a dev session could kill the renderer with
"Cannot read properties of null (reading 'useRef')": the page executed
two optimizer generations at once — react/react-dom chunks resurrected
from the persistent Electron HTTP cache (immutable `?v=<browserHash>`
entries whose per-dep labels survive re-optimization commits) next to
freshly transformed sources importing today's react copy. Two React
instances leave the hook dispatcher null on one of them, and the first
hook call in any lazily loaded component crashes.

- main-window: clear the session HTTP cache before loading a dev server
  (main-renderer-dev-cache.ts, packaged file:// builds untouched)
- dev.mjs: warm the renderer entry and wait for the crawl to settle
  before launching Electron, so the initial optimizer commit lands
  before the window loads

Fixes apache#4775

Generated-by: Maka

@Astro-Han Astro-Han 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.

Reviewed current head 3fb841eea9e859acefb00d719c576b3db1c9864a (OPEN, MERGEABLE, hosted label/package/test green). Technical GO — no P0–P3. The remaining merge requirements are the normal review/branch-protection gates.

What was checked

A 4-file (+136/−0) change with no schema/migration: apps/desktop/scripts/dev.mjs:163-175 warms up /main.tsx before Electron start and waits for the client Vite environment to idle; main-renderer-dev-cache.ts:41-68 only clears the HTTP cache for the dev server, warning and continuing on failure; main-window.ts:313-316,526-531 invokes before the first renderer load without touching the independent embedded-browser partition. New tests cover dev-clear, packaged no-op, and clear-failure. Both Vite APIs used were verified against Vite 8.2.2's DevEnvironment. Merge-tree against latest main is clean; git diff --check clean.

What I could not judge

The local checkout's node_modules was incomplete, so the TypeScript suite, desktop build, and real Electron/Chromium smoke could not be run independently (node --check on the changed script passes); the author's stated local verification is not taken as independent evidence either way.


Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

简体中文

本条结论全部来自 @Luna-Deep-Qronos 的审查。我自己没有读这份 diff;我核的是当前 head 有没有漂移、以及 exact-head 的 CI 状态。当前 head 是 3fb841e,可合并,检查通过。技术上无阻断问题,合并仍需走正常门禁。

@me2seeks
me2seeks requested a review from Astro-Han September 5, 2026 07:32

@jackwener jackwener left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approving at exact head 3fb841ee. No findings. All three checks pass, and the change is dev-only in a way that is enforced rather than asserted.

The root cause is established from evidence, not inferred

The crash stack mixes two ?v= browser hashes, and the cache held an intact stale chunk graph — react-dom_client.js?v=c163ffce importing client-CwWlPFAR.js?v=d164f9b2 importing react.js?v=4a8fd6a8a complete generation that no longer exists on disk yet remains loadable forever because the responses are Cache-Control: immutable. Two React instances in one page, a dispatcher set only on the one react-dom renders with, and hooks reached through the other returning null.

The module header adds the piece that makes it recurring rather than a one-off: the optimizer preserves a dep's ?v= label across re-optimization commits, including ones where the bundle bytes changed — a Vite upgrade, a patch re-application, an interrupted commit. The same label can therefore address different content, which is exactly the condition an immutable cache cannot survive. The deps_temp_* leftovers showing mid-session re-optimization in this workspace close the loop.

The guards hold where it matters

  • Dev-only is enforced at the call, not documented: if (!rendererEntry.useDevServer) return false is the first statement, and packaged builds load file:// and never reach this path.
  • The blast radius is stated and correct: clearing the session HTTP cache does not touch localStorage or cookies — they are not part of it. That is the question a reviewer should ask about clearing a cache, and the module answers it in its header rather than leaving it to be discovered.
  • The failure downgrade is a reasoned trade, not a reflex: "the duplicate-React crash it guards against is recoverable by reload, but a window that never opens is not." Choosing which failure to prefer, and saying why, is the part that usually gets omitted.
  • Structural interfaces instead of importing electron keep the module loadable in plain node tests, matching main-renderer-loader.ts. That is why the three cases — clears in dev, untouched for packaged, failure downgrades — can be unit tests at all.

Wired at main-window.ts:529, immediately before loadMainRenderer, with a comment pointing back to the module for the reasoning.

On the warmup half

Warming /main.tsx and awaiting waitForRequestsIdle() after server.listen() puts the initial crawl and the optimizer commit before the window starts requesting modules — which is the ordering that makes the cleared cache stay on one generation rather than immediately racing a re-optimization.

Worth noting that this was checked against the real API rather than assumed: a standalone Vite 8.2.2 server with the repo config and a throwaway cacheDir settles in ~2.4s with the entry graph crawled and deps bundled. Verifying that a third-party call actually does what its name suggests is a different act from reading its documentation, and for a fix whose whole mechanism is ordering, it is the one that matters.

npm test and the e2e suites were not run, which is stated plainly and is proportionate for a dev-only change of this size.

This is a fix, so the merge decision remains a human's.

简体中文

3fb841ee 上批准。没有发现问题。 三项检查全部通过,而且「仅限 dev」这件事是被强制的,而不是被声明的

根因是从证据建立的,不是推断的

crash 栈混合了两个 ?v= browser hash,而缓存里存着一条完整的过期 chunk 图——react-dom_client.js?v=c163ffce 引入 client-CwWlPFAR.js?v=d164f9b2,后者再引入 react.js?v=4a8fd6a8——一个磁盘上已不存在、却因为响应带 Cache-Control: immutable 而永远可加载的 generation。 于是一个页面里有两个 React 实例,dispatcher 只设在 react-dom 用于渲染的那一个上,而经由另一个到达的 hooks 返回 null

模块头补上了「使它反复发生而非一次性事故」的那块:优化器在多次重新优化提交之间会保留某个 dep 的 ?v= 标签,包括那些 bundle 字节已经改变的提交——Vite 升级、补丁重新打上、一次被中断的提交。于是同一个标签可以指向不同的内容,而这恰恰是一个 immutable 缓存无法幸存的条件。 那些显示本 workspace 会在会话中途重新优化的 deps_temp_* 残留,把这条链闭合了。

守卫都落在要紧处

  • 「仅限 dev」是在调用处被强制的,不是写在文档里的:if (!rendererEntry.useDevServer) return false 是第一条语句,而打包构建加载 file://,根本到不了这条路径。
  • 影响半径被明确说出且正确:清除会话 HTTP 缓存不会触及 localStorage 或 cookies——它们不属于它。 这正是评审在「清缓存」面前该问的问题,而模块在自己的头部就回答了它,没有留给别人去发现。
  • 失败降级是一次有推理的取舍,不是反射动作:「它所防范的重复 React 崩溃可以通过重新加载恢复,而一个永远打不开的窗口不能。」 选择偏好哪一种失败、并说明为什么——这一步通常会被省略。
  • 用结构化接口而不是 import electron,使该模块在纯 node 测试中可加载,与 main-renderer-loader.ts 同一模式。这也正是那三条用例(dev 下清除、打包下不动、失败降级)之所以能成为单元测试的原因。

接线在 main-window.ts:529,紧邻 loadMainRenderer 之前,并附了一条指回该模块以说明理由的注释。

关于 warmup 那一半

server.listen() 之后预热 /main.tsx 并等待 waitForRequestsIdle(),把初始爬取与优化器提交放在窗口开始请求模块之前——正是这个顺序,让被清空的缓存停留在一个 generation 上,而不是立刻与一次重新优化竞速。

值得一提的是,这一点是对着真实 API 验证过的,而不是假定的:一台独立的 Vite 8.2.2 服务器,配上仓库配置与一次性 cacheDir,约 2.4 秒落定,入口图已爬取、依赖已打包。验证一个第三方调用是否真的做了它名字所暗示的事,与阅读它的文档是两回事;而对于一个整套机制就是「顺序」的修复来说,前者才是要紧的那件。

npm test 与 e2e 套件没有运行,这一点被直白地写了出来,对这个体量的 dev-only 改动而言是相称的。

这是一个 fix,合并与否仍由人决定。


Automated review notice: This comment was posted by an automated review agent operated by jackwener. It is not an independent human review and does not replace one.

@me2seeks
me2seeks merged commit c12205b into apache:main Sep 5, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/M Under 500 readable lines

Projects

None yet

3 participants