Repository navigation
fix: ignore Esc stack entries from renders React discarded (StrictMode, React 18) - #49
jainkuniya wants to merge 1 commit into
Conversation
…e, React 18) Under React 18 StrictMode a mounting portal renders twice with a different useId; only the second runs effects, so the render-time push of the first was never cleared and stayed on top of the stack. Entries now count only once their effect has run, and unclaimed entries are dropped when nothing live is left. Fixes react-component#48 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
条目认领与 Escape 分发 src/useEscKeyDown.ts, tests/index.test.tsx |
栈条目新增 mounted 标记。Escape 分发只处理已挂载条目，并据此计算 top。关闭时移除当前条目；若没有剩余的已挂载条目，则清空栈。新增 StrictMode 测试，验证 top: true 和卸载后的空栈。 |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix · Severity of issue fixed: Medium
Possibly related issues
react-component/portal#47：该 PR 修改 Escape 分发使用的条目集合，但提供的摘要未说明是否处理了回调同步关闭 Portal 时数组缩短的问题。
Merge Risk: 
3facf
-
The reported Escape-stack regression is covered, and the remaining development-only accumulation does not prevent merging.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check 标题“fix: ignore Esc stack entries from renders React discarded (StrictMode, React 18)”与变更内容直接相关。变更在 src/useEscKeyDown.ts 中为 stack 条目新增 mounted 状态，Escape 事件只处理已 mounted 的条目；同时在 tests/index.test.tsx 中新增 … Linked Issues check 针对 #48，src/useEscKeyDown.ts在渲染时创建mounted: false的预备条目，并在 effect 中标记为已挂载。Escape 事件只处理已挂载条目，并基于过滤后的列表计算top。clear()在没有已挂载条目时删除未认领条目。这样可以保留嵌套顺序，并排除 React 18 StrictMode 丢弃渲染留下的条目。新增测试覆盖已打开的子 Po…Out of Scope Changes check 变更限于 #48所需的 Esc 栈状态、事件分发和回归测试。对过滤副本的迭代属于只处理已挂载条目的实现，并避免分发期间数组长度变化。现有测试未引入与#48无关的功能或公共 API 变更。Full details: Title check
-
Explanation
标题“fix: ignore Esc stack entries from renders React discarded (StrictMode, React 18)”与变更内容直接相关。变更在 src/useEscKeyDown.ts 中为 stack 条目新增 mounted 状态，Escape 事件只处理已 mounted 的条目；同时在 tests/index.test.tsx 中新增 StrictMode 回归测试。标题说明了主要修复点：忽略 React 丢弃的渲染所产生的 Esc 栈条目，并限定在 StrictMode 与 React 18 场景下，与目标中描述的问题一致。
Full details: Linked Issues check
-
Explanation
针对
#48，src/useEscKeyDown.ts在渲染时创建mounted: false的预备条目，并在 effect 中标记为已挂载。Escape 事件只处理已挂载条目，并基于过滤后的列表计算top。clear()在没有已挂载条目时删除未认领条目。这样可以保留嵌套顺序，并排除 React 18 StrictMode 丢弃渲染留下的条目。新增测试覆盖已打开的子 Portal 挂载、关闭、Escape 发送到父 Portal，以及卸载后栈为空。PR 验证信息还报告 React 18.3.1 的 Esc 测试全部通过。
-
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
兔子轻按 Escape 键，
栈中条目先等 effect。
未认领项不再挡路，
下层 Portal 重获 top。
卸载之后栈归空，
月光下兔耳轻轻摇。
Comment @coderabbitai help to get the list of available commands.
Fixes #48
Problem
useEscKeyDownpushes each open portal onto the Esc stack during render (inuseMemo), so a parent lands below its children. On React 18, StrictMode renders a mounting component twice, anduseIdreturns a different id each time; only the second render runs effects. So a portal that mounts already open leaves the first render's entry on the stack for good. This happens with antdModal+destroyOnHidden, a nestedDrawer, or aTooltipshown inside one. After that portal closes, the stale entry is the top of the stack, and the portal below it (theDrawertheModalwas opened from) never getstop: trueagain. Escape stops closing it. React 19 and production builds aren't affected.Change
mounted. The render-time push adds the entry withmounted: false, and the effect marks ittrue.onGlobalKeyDowndispatches to the mounted entries only. An entry from a discarded render never takes part, sotopis computed over the portals that are really open.clear()drops the unclaimed entries once no mounted entry is left, so they can't accumulate across opens.A side effect: the loop now indexes a filtered copy, not
stackitself. A portal that closes during dispatch can no longer shorten the array being indexed, which is the crash described in #47. I haven't reproduced #47, so this PR doesn't claim to fix it.Test
onEsc › a portal that mounts open over another and closes leaves the other top in StrictMode. A is open; B mounts already open, then closes. Escape must reach A withtop: true, and unmounting must leave the stack empty. On React 18.3.1 it fails onmaster(A only receives{ top: false }) and passes with this change.Verification
4f29120a6403b4ac08ad21914107888aad90df9d.master: 3 Esc tests fail (the new one,onEsc should treat first mounted portal as top in StrictMode,should clear stack when unmount), plusref-able › no warning for React 19 ref, which needs React 19.master: 23/24.ref-able › not support fragment. It fails onmastertoo and is unrelated.tscpasses.eslint: no new warnings; the tworeact-hooks/exhaustive-depswarnings on this hook are onmastertoo.npm run compilesucceeds.tests/index.test.tsx(fix: skip the container update when it is unchanged #46, fix: preserve non-element children when forwarding refs #43, feat: add CSP nonce support to Portal #35): no conflicts. No open PR touchessrc/useEscKeyDown.ts.Drawer→Modal(destroyOnHidden) under React 18 StrictMode, and checked in a browser: with this change, Escape closes theDraweragain after theModal, a nestedDraweror aTooltipcloses.AI assistance disclosure: Claude Code was used to trace the cause, write the fix and the test, and run the checks above. The failures and passes listed were reproduced locally.
Summary by CodeRabbit