Skip to content

fix: review follow-ups for the recent reactive fixes (observer/react) - #257

Merged
ximing merged 3 commits into
masterfrom
fix/review-followups-reactive
Aug 31, 2026
Merged

fix: review follow-ups for the recent reactive fixes (observer/react)#257
ximing merged 3 commits into
masterfrom
fix/review-followups-reactive

Conversation

@ximing

@ximing ximing commented Aug 31, 2026

Copy link
Copy Markdown
Owner

背景

针对最近一批响应式修复(#228/#229/#233/#234 等)整体 review 后的跟进修复。Review 中每条候选问题都经过实证验证;本 PR 修复其中确认的 6 项正确性问题、1 项性能回归,并清理 3 项死代码,全部配回归测试(每个测试都验证过在未修复代码上失败)。

observer(db2ab84)

react

死 reaction 重建(00da83e)

审计修正:React 19 下首渲染挂起的组件会被整体重挂载(fiber/hook 状态不保留),函数组件由 subscribe cleanup/recreate 兜底自愈;真正可复现的是类组件路径,且更严重——view 类组件在普通 StrictMode 下就永久失去响应式:模拟卸载调用 componentWillUnmount 置空 _reactiveRender,重挂载只重放 componentDidMount、不触发 render,无任何重建。

  • 抽出 _createReactiveRender(),在 componentDidMount(重建 + forceUpdate 重收集依赖)与 render(兜底)检测 reaction 死亡(null 或 unobserved)并重建
  • useObserver 增加 unobserved 检查作纵深防御

bindServices 隐藏态容器(48879ee)

审计修正:React 19 中 Suspense 重新隐藏不拆除 useEffect(只拆 layoutEffect),<Activity> 隐藏才会。committed + microtask 的销毁只能取消 StrictMode 同帧重挂载;Activity/Offscreen 的 hide→reveal 跨 commit,microtask 在中间销毁容器,而 useRef 保留的 ADM 让 createADM 不再执行,reveal 后子组件解析到 null 容器。

  • render 时检测容器为空则原地重建(Activity reveal 会重渲染子树),并重新挂上 GC 兜底;代价(隐藏期丢失 service 状态)已在注释说明
  • queueMicrotask 运行时探测 + Promise 降级(旧 RN JSC/Hermes,与包内 FinalizationRegistry/WeakRef 降级同一批环境)
  • 删除销毁后 no-op 的 FinalizationRegistry 重注册、ADM 死字段 timmer

测试

  • observer:1266 passed(新增 6 个回归测试)
  • react:219 passed(新增 6 个回归测试)
  • service:237 passed;react 包 tsc --noEmit 无错误

🤖 Generated with Claude Code

ximing and others added 3 commits September 1, 2026 00:22
- batch: guard the Error.cause assignment so a frozen/non-extensible
  callback error no longer throws a TypeError from finally that would
  replace the in-flight exception (the #212 masking all over again).
- batch: when the flush error cannot be attached (callback threw a
  non-Error, the Error already has a cause, or the object is frozen),
  console.warn instead of silently dropping the reaction's stack.
- observe: reviving an unobserved reaction now resets everRan, so a
  failing first run after revival takes the firstRun auto-detach path
  instead of restoring an empty snapshot and becoming a live reaction
  with zero dependencies (interaction regression between #215 and #233).
- runAsReaction: drop the per-rerun cleaners.slice() (releaseReaction
  never mutates the old array in place, so the reference is a valid
  snapshot) and the unreachable else-release branch.

Co-Authored-By: Claude Code <noreply@anthropic.com>
view-wrapped class components silently lost reactivity after any
componentWillUnmount that was not a real unmount — StrictMode's
simulated remount and Suspense/Offscreen hide both null the
_reactiveRender reaction, and nothing ever recreated it. Recreate in
componentDidMount (the remount path replays cDM without re-rendering)
and in render as a fallback; also treat an unobserved (first-run
auto-detached) reaction as dead.

useObserver gains the same unobserved-reaction guard as defense in
depth — empirically React 19 remounts mount-suspended fibers (no hook
state to preserve) and committed re-suspensions are healed by the
subscribe cleanup/recreate path, but a non-null dead reaction must
never be reused for tracking.

Co-Authored-By: Claude Code <noreply@anthropic.com>
The committed-flag + queueMicrotask destroy only cancels StrictMode's
same-flush re-setup. <Activity>/Offscreen hide→reveal spans separate
commits, so the microtask destroys the container while the tree is
merely hidden and the useRef-preserved ADM never recreates it — after
reveal, children resolve against a null container. Rebuild the
container in place during render (Activity reveal re-renders the tree)
and re-arm the FinalizationRegistry backstop for the rebuilt container.
Trade-off, noted in code: service state is lost while hidden.

Also:
- guard queueMicrotask with a Promise.resolve().then fallback for old
  RN JSC/Hermes environments (same targets the FinalizationRegistry and
  WeakRef fallbacks exist for),
- drop the post-destroy registry re-register (provable no-op),
- drop the dead `timmer` ADM field.

Co-Authored-By: Claude Code <noreply@anthropic.com>
@ximing
ximing merged commit 5182f79 into master Aug 31, 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.

1 participant