Effect cleanup potential issue in React 17 for mutable value inside effect
还没有人认领这个 Issue。
- 主要语言
- JavaScript
- 星标
- 11.8k
- 派生
- 7.9k
- 平均合并
- 1 天 11 小时
- 30 天内合并 PR
- 11
描述
In React 17, the effect cleanup function always runs asynchronously — for example, if the component is unmounting, the cleanup runs after the screen has been updated.
In the changelog React team has highlighted a potential issue and solution for this:
Problematic Code:
useEffect(() => {
someRef.current.someSetupMethod();
return () => {
someRef.current.someCleanupMethod();
};
});
Solution suggested by the React team:
The problem is that
someRef.currentis mutable, so by the time the cleanup function runs, it may have been set to null. The solution is to capture any mutable values inside the effect.
useEffect(() => {
const instance = someRef.current;
instance.someSetupMethod();
return () => {
instance.someCleanupMethod();
};
});
While the above solution works in most cases, I have created a custom hook, where this solution is problematic:
function useMountEffect(funcForMount, funcForUnmount) {
const funcRef = useRef();
funcRef.current = { funcForMount, funcForUnmount };
useEffect(() => {
funcRef?.current?.funcForMount?.();
return () => funcRef?.current?.funcForUnmount?.();
}, []);
}
In the above example, I explicitly don't want to capture the mutable value, otherwise, I'll have to re-run the effect when funcForUnmount changes.
The reason to create the hook in this way is to have the freedom to call the function only on mount / unmount without worrying about stale closure and dependency array.
Update:
I have found one approach for useMountEffect
function useMountEffect(funcForMount, funcForUnmount) {
const isMounted = useRef();
useEffect(() => {
return () => {
isMounted.current = false;
};
}, []);
useEffect(() => {
if (!isMounted.current) {
isMounted.current = true;
funcForMount?.();
}
return () => !isMounted.current && funcForUnmount?.();
});
}
But the major questions here are:
- How can the
ref.currentgo null between unmount and cleanup call in React 17? - Who is setting the value null? Is it react which can set
ref.currentto null? Or, is it the user who can change in between? - If a user can change in between how is this possible? Why was it not an issue prior to React 17?
The reason to raise this issue for docs is that the line The problem is that someRef.current is mutable, so by the time the cleanup function runs, it may have been set to null. is unclear and raises a lot of questions in the mind.
贡献指南
从这里开始
- 先读完整个 Issue,再读项目的贡献指南。
- 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
- Fork 仓库,在一个分支上完成修改。
- 提交 Pull Request,并在描述里引用这个 Issue 编号。
调研方向
从 issue 中链接的 React 17 changelog 部分开始,并将其中的解释与报告中的 useMountEffect 示例进行比较。明确谁可以将 ref.current 设置为 null、cleanup 何时运行,以及为什么 timing 在 React 17 中发生了变化。完成的标准是文档回答所列问题,且不会让 mutable ref 的行为产生歧义。
由索引模型根据 Issue 内容生成。
评估
- 技术栈
- javascript, react
- 领域
- documentation
- Issue 类型
- 文档
- 难度
- 3/5
- 预计耗时
- 1-2 天
- 活跃度
- 停滞
- 描述清晰度
- 基本清楚
- 新手友好度
- 35/100