Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

【BUG】usePersistFn不能兼容react devtool #728

Closed
xianjie-li opened this issue Nov 13, 2020 · 22 comments · Fixed by #739
Closed

【BUG】usePersistFn不能兼容react devtool #728

xianjie-li opened this issue Nov 13, 2020 · 22 comments · Fixed by #739
Assignees

Comments

@xianjie-li
Copy link

复现步骤

  1. 打开下方链接
    https://wjl37.csb.app/

  2. 点击任意按钮, 计数增加正常
    image

  3. 打开react devtool, 并选中计数器所在组件, 点击新增按钮计数不再增加


组件代码如下,也可以点击实例页面右下角开sandbox:

export default () => {
  const [count, setCount] = useState(0);

  const persistFn1 = usePersistFn(() => {
    console.log(1);
    setCount((prev) => {
      console.log(2);
      return prev + 1;
    });
  });

  const persistFn2 = usePersistFn(() => {
    console.log(3, count);
    setCount(count + 1);
  });

  return (
    <>
      <button type="button" onClick={persistFn1}>
        Add Count {count}
      </button>
      <button type="button" onClick={persistFn2}>
        Add Count {count}
      </button>
    </>
  );
};

源码进行如下修改可以修复,但是不确定是否会影响现有项目

fnRef.current = fn;

// =>
useEffect(() => {
   fnRef.current = fn;
});

// 或 =>
useMemo(() => {
    fnRef.current = fn;
});
@brickspert
Copy link
Collaborator

感谢反馈,这个 bug 可太奇怪了~

@gaowujie2004
Copy link

@brickspert 你好,请教一下这个问题,背后的原因是什么呢。

@gaowujie2004
Copy link

从代码上看,fnRef.current = useMemo(() => fn, [fn]);

在组件中这样调用:
useMemoizedFn( () => { ....... } )

那么 fn 都是新的函数引用。据我所知 useMemo(callback, [...deps]) , 对于 callback 的调用是同步的。
那么 useMemo(() => fn, [fn]); () => fn; 这个函数每一次重渲染都同步执行,实在是感觉没起多大作用哎,但就是能解决问题,哈哈。

@brickspert
Copy link
Collaborator

@brickspert 你好,请教一下这个问题,背后的原因是什么呢。

背后的原因还不清楚

@joriewong
Copy link

useLayoutEffect(() => {
    fnRef.current = fn;
});

@AnathanPham
Copy link

useLayoutEffect(() => {
    fnRef.current = fn;
});

讲道理,这个方法也不太行。
我觉得不使用并发模式的前提下,用当前版本的useMemoizedFn 很香。

useLayoutEffect是 RFC给出的例子的实现。

useEvent潜在的问题?

我的评论 reactjs/rfcs#220 (comment)
子组件 useLayoutEffect 无法获取最新的 方法。
子组件的 useLayoutEffect 发生在 父组件 之前。如果在子组件的 useLayoutEffect 中执行 useEvent的返回的方法,将会访问旧的值。
想到这里,我觉得既然我们没有使用并发模式,那么用 useMemoizedFn 也没什么毛病甚至更好。

@youjiaqi421
Copy link

@brickspert 你好,请教一下这个问题,背后的原因是什么呢。

这个产生了 什么问题吗

@youjiaqi421
Copy link

从代码上看,fnRef.current = useMemo(() => fn, [fn]);

在组件中这样调用: useMemoizedFn( () => { ....... } )

那么 fn 都是新的函数引用。据我所知 useMemo(callback, [...deps]) , 对于 callback 的调用是同步的。 那么 useMemo(() => fn, [fn]); () => fn; 这个函数每一次重渲染都同步执行,实在是感觉没起多大作用哎,但就是能解决问题,哈哈。

这个产生了什么问题吗?我复现了半天,没搞出来 , 我现在用的版本是concurrent模式

@youjiaqi421
Copy link

从代码上看,fnRef.current = useMemo(() => fn, [fn]);

在组件中这样调用: useMemoizedFn( () => { ....... } )

那么 fn 都是新的函数引用。据我所知 useMemo(callback, [...deps]) , 对于 callback 的调用是同步的。 那么 useMemo(() => fn, [fn]); () => fn; 这个函数每一次重渲染都同步执行,实在是感觉没起多大作用哎,但就是能解决问题,哈哈。
我觉得可能是 ref赋值时机搞出来的这个问题

@kongmoumou
Copy link
Contributor

kongmoumou commented Jul 30, 2022

@brickspert
查了下资料,react devtool inspect 组件时会进行 shallow render,并且替换所有 hooks 为 mock hooks(用来获取 hook 信息,参考 bvaughn 评论)。这就会导致在选中时,触发 render,并且因为在 render 中修改 ref,导致 ref.current 被替换成 devtool mock 的空函数(无法触发更新)。但是用 useMemo 包一层,mock useMemo 会始终返回组件正常 render 时的 memorized value,也就不会破坏原有的功能了 🤣

@dadazhouRenee
Copy link

那为啥 useInterval 中 fn 不需要用useMemo包呢

@suwu150
Copy link

suwu150 commented Jan 17, 2023

那为啥 useInterval 中 fn 不需要用useMemo包呢

你的会有什么问题吗,在使用时

@Flcwl
Copy link
Contributor

Flcwl commented May 14, 2023

理论应该反馈给 react 解决

@hcjlxl
Copy link

hcjlxl commented May 18, 2023

理论应该反馈给 react 解决

他们讨论过了,需要按照 hooks的规则走,就是要将这个 ref 的修改放在useEffect 里面 不能直接在render中修改

raotaohub added a commit to raotaohub/ez-modal-react that referenced this issue Jun 12, 2023
@vaynevayne
Copy link

理论应该反馈给 react 解决

他们讨论过了,需要按照 hooks的规则走,就是要将这个 ref 的修改放在useEffect 里面 不能直接在render中修改

直接在 effect 里修改也会有问题,比如这个 hook,
我使用
https://github.com/chakra-ui/chakra-ui/blob/main/packages/hooks/use-callback-ref/src/index.ts#L10
这个 hook 会造成 setValue 中的 value 不是最新的, 而使用 ahook 的 useMemoizedFn 就可以解决这个问题,

@hcjlxl
Copy link

hcjlxl commented Aug 16, 2023

直接在 effect 里修改也会有问题,比如这个 hook, 我使用 https://github.com/chakra-ui/chakra-ui/blob/main/packages/hooks/use-callback-ref/src/index.ts#L10 这个 hook 会造成 setValue 中的 value 不是最新的, 而使用 ahook 的 useMemoizedFn 就可以解决这个问题,

你使用的chakra-ui里面这个hook写了依赖数组吗,他导出的useCallback 这个hook 支持依赖数组的

@vaynevayne
Copy link

写了

@hcjlxl
Copy link

hcjlxl commented Aug 16, 2023

写了

如果去掉会不会是正常的呢

@vaynevayne
Copy link

vaynevayne commented Aug 16, 2023

charkra-ui 的这个 hook 不能去掉依赖吧

 const [config, setConfig] = useControlledImmer<ReportConfig>({
    value,
    onChange,
    defaultValue,
  })
import { useCallbackRef } from '@chakra-ui/react-use-callback-ref'
import produce, { Draft, freeze } from 'immer'
import { useState } from 'react'





// 这是我自己实现的, 参考 charka-ui 的逻辑
export function useControlledImmer(
  props:
){
  const {
    value: valueProp,
    defaultValue,
    onChange,
    shouldUpdate = (prev, next) => prev !== next,
  } = props

  const onChangeProp = useCallbackRef(onChange)
  const shouldUpdateProp = useCallbackRef(shouldUpdate)

  const [uncontrolledState, setUncontrolledState] = useState(
    freeze(
      typeof defaultValue === 'function' ? defaultValue() : defaultValue,
      true
    )
  )
  const controlled = valueProp !== undefined
  const value = controlled ? valueProp : uncontrolledState

  const setValue = useCallbackRef(
    (updater) => {
      const nextValue =
        typeof updater === 'function'
          ? produce(value, updater)
          : freeze(updater)

      if (!shouldUpdateProp(value, nextValue)) {
        return
      }

      if (!controlled) {
        setUncontrolledState(nextValue)
      }

      onChangeProp(nextValue)
    },
    [controlled, onChangeProp, value, shouldUpdateProp]
  )

  return [value, setValue]
}

@awmleer
Copy link
Collaborator

awmleer commented Nov 13, 2023

@fantasticsoul 大概看了一眼感觉好像并没有效果诶?你说的多包了一层是指多套了一个 ref 么,memoizedFn.current.fn 看起来永远会调用最新的 fnRef.current.fn,而问题的关键是 fnRef.current.fn 在 devtools 的上下文环境里被修改成了一个错误的闭包环境下的 fn(说的有点绕,但是看你截图里贴的那个 issue,应该是这个意思),因此感觉并不能解决问题

不过我并没有实际写出来验证过,有兴趣的话要不简单搞个 CodeSandbox demo 试试?

@fantasticsoul
Copy link

哈哈试了不行所以我把建议删了,还是得 useMemo,我在helux里改进了下useMemorizedFn,进化为 useStable了,可以传入多个函数集合、单个函数,或者其他任意值了

image

CodeSandbox线上示例见这里

image

@fantasticsoul
Copy link

@fantasticsoul 大概看了一眼感觉好像并没有效果诶?你说的多包了一层是指多套了一个 ref 么,memoizedFn.current.fn 看起来永远会调用最新的 fnRef.current.fn,而问题的关键是 fnRef.current.fn 在 devtools 的上下文环境里被修改成了一个错误的闭包环境下的 fn(说的有点绕,但是看你截图里贴的那个 issue,应该是这个意思),因此感觉并不能解决问题

不过我并没有实际写出来验证过,有兴趣的话要不简单搞个 CodeSandbox demo 试试?

还是useMemo最靠谱

raotaohub pushed a commit to raotaohub/nice-modal-react that referenced this issue Aug 23, 2024
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 a pull request may close this issue.