Skip to content

fix(web): dedupe React so hook-using deps stop crashing - #1211

Merged
tiann merged 1 commit into
tiann:mainfrom
hqhq1025:fix/react-dedupe
Jul 29, 2026
Merged

fix(web): dedupe React so hook-using deps stop crashing#1211
tiann merged 1 commit into
tiann:mainfrom
hqhq1025:fix/react-dedupe

Conversation

@hqhq1025

Copy link
Copy Markdown
Collaborator

What

Two Vite dev/build config additions:

  1. resolve.dedupe: ['react', 'react-dom']
  2. optimizeDeps.include for the four workbox packages

Why (1): duplicate React silently breaks any hook-using dependency

@radix-ui/react-popover is declared in web/package.json, but it is not linked into web/node_modules — only react-dialog and react-slot are:

$ ls web/node_modules/@radix-ui/
react-dialog  react-slot

So it resolves React from the repo root instead, which is a physically different copy from the one app code imports:

web app imports react →  node_modules/.bun/react@19.2.3/node_modules/react
react-popover imports →  node_modules/react

Two React instances make every hook-using third-party component throw Invalid hook call on render. Without an error boundary React then unmounts the whole tree, so the symptom is a blank page rather than a visible error.

Re-running bun install does not change the layout, so this is not stale local state.

Impact: this was already failing 20 tests on main

main with this patch
Failing 8 files / 20 tests 0
Passing 1483 1560
Total collected 1503 1560

The collected total goes up by 57 because the affected files were dying during import — their tests were never counted at all.

cd web && npm run test
# main:       Test Files 8 failed | 173 passed (181)
#             Tests     20 failed | 1483 passed (1503)
# this patch: Test Files 181 passed (181)
#             Tests     1560 passed (1560)

I found this while building a feature that renders a Radix Popover — the component white-screened the entire page. The failing tests on main turned out to be the same root cause.

Why (2): dev service worker forces a mid-run page reload

VitePWA devOptions.enabled: true injects SW registration into every served page. The workbox imports inside sw.ts are only discovered after registration, so Vite re-optimizes deps and issues optimized dependencies changed → reloading. On a cold cache this reliably kills whichever e2e test is in flight (elements vanish mid-assertion).

Declaring the four workbox packages in optimizeDeps.include pre-bundles them at startup instead. This affects all e2e fixtures, not any one suite.

Verification

  • npm run test — 1560/1560 pass (was 1483/1503 with 20 failures)
  • npm run typecheck — clean
  • npm run build — succeeds, PWA manifest generated normally
  • npm run test:mermaid-lightbox:e2e — 15/15 pass

Note

resolve.dedupe is a safety net, not a substitute for fixing the install layout. If the real fix is to make bun link @radix-ui/react-popover into web/node_modules, I'm happy to change direction — but the dedupe is worth keeping regardless, since it makes the app resilient to this class of hoisting difference.

Radix Popover crashed with "Invalid hook call" because
@radix-ui/react-popover is not linked into web/node_modules and
resolves react from the repo root — a different instance than the
one app code imports. Two React copies make every hook-using third
party component throw on render and unmount the whole tree.

The VitePWA dev service worker also pulls its workbox imports only
after registration, so Vite re-optimizes deps and force-reloads the
page mid-run, which nondeterministically kills whichever e2e test is
in flight.

@github-actions github-actions 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.

Findings
No findings.

Summary
Review mode: initial
No diff-introduced issues found. Residual risk: this config-only change depends on Vite/Bun dependency resolution behavior, so coverage should come from web typecheck/test/build plus E2E on a cold Vite dependency cache.

Testing
Not run (automation).

HAPI Bot

@hqhq1025

Copy link
Copy Markdown
Collaborator Author

@tiann 这个是从 #1212 里拆出来的独立修复。发现过程是做 Radix Popover 相关功能时整页白屏,追下去发现 main 上那 20 个测试失败是同一个根因。

单独提出来是因为它跟引用功能没关系,而且价值独立 —— 合了之后 npm run test 就是全绿的。#1212 暂时包含了这个 commit(否则那边的 e2e 跑不起来),这边合并后那边会自动去重。

如果你觉得正确的修法应该是让 bun 把 @radix-ui/react-popover 正常链进 web/node_modules,而不是在 vite 层兜底,我也可以往那个方向改。

@tiann
tiann merged commit 87e8874 into tiann:main Jul 29, 2026
2 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.

2 participants