reactjs / reactjs/react.dev

Effect cleanup potential issue in React 17 for mutable value inside effect

Đang mở
#4,012 0 bình luận 2 reaction 0 người được giao Xem trên GitHub

Chưa có ai nhận issue này.

Ngôn ngữ chính
JavaScript
Star
11.8k
Fork
7.9k
Merge trung bình
1 ngày 11 giờ
Pull request đã merge (30 ngày)
11

Mô tả

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.current is 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.current go null between unmount and cleanup call in React 17?
  • Who is setting the value null? Is it react which can set ref.current to 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.

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Hướng nghiên cứu

Bắt đầu với phần changelog của React 17 được liên kết trong issue và so sánh phần giải thích đó với ví dụ useMountEffect được báo cáo. Làm rõ ai có thể đặt ref.current thành null, cleanup chạy khi nào và tại sao timing đã thay đổi trong React 17. Công việc được xem là hoàn tất khi tài liệu trả lời các câu hỏi đã liệt kê mà không để hành vi của mutable refs trở nên mơ hồ.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Đánh giá

Công nghệ
javascript, react
Lĩnh vực
documentation
Loại issue
Tài liệu
Độ khó
3/5
Thời gian dự kiến
1-2 ngày
Mức độ hoạt động
Đình trệ
Độ rõ ràng
Khá rõ ràng
Mức phù hợp với người mới
35/100

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.