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: In strict-mode, useRef gives you a different object each time component is called during the first render #22872

Closed
panta82 opened this issue Dec 6, 2021 · 5 comments
Labels
Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug

Comments

@panta82
Copy link

panta82 commented Dec 6, 2021

In strict mode, React will call render method twice, to catch mutate-during-render type of bugs. However, there is an inconsistency in the way this is done for useRef().

During the first render only, each call will give you a fresh ref object. During subsequent calls, you will be given the same object during both calls.

React version: 17.0.2

Steps To Reproduce

Here is a little app to demo this.

import React from "react";
import "./styles.css";

const Hooky = () => {
  const ref = React.useRef(null);
  const [num, setNum] = React.useState(0);

  ref.current = (ref.current || 0) + 1;

  return (
    <div>
      <p>
        Val: {num} <br />
        <button onClick={() => setNum(Math.random())}>Roll</button>
      </p>
      <p>Ref count: {ref.current}</p>
    </div>
  );
};

const App = () => {
  const [show, setShow] = React.useState(false);

  return (
    <div>
      <h2>Show or hide</h2>
      <button onClick={() => setShow(!show)}>{show ? "Hide" : "Show"}</button>
      <hr />
      {show && <Hooky />}
    </div>
  );
};

export default App;

Click "Show". You will see ref count set to 1, indicating the useRef has returned a different object both times it was called (it could also mean the component was called only once, but that isn't the case, I checked using debugger). If you force rerender by clicking the other button, the ref will increase in increments of 2, which is what you'd expect.

Link to code example: https://codesandbox.io/s/react-fiddle-forked-fkocg?file=/src/App.js

The current behavior

useRef in strict mode has one behavior during first render and other during subsequent renders.

The expected behavior

useRef operates consistently during all renders. Either always gives you the same object, or has a separate object for each "lane" and always reuses the same one.

Basically, the counter in the code sandbox example should either go: 2 4 6 8 or 1 2 3 4. Not 1 3 5 7, as it does now.

Why this matters

Practical issue where I encountered this was trying to do subscribe/unsubscribe system using refs. I want my components to consistently subscribe and subscribe callbacks, kind of like element ref works. This failed when I detected callbacks not getting called, due to a duplicate ref object provided the first time component is rendered.

@panta82 panta82 added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Dec 6, 2021
@vkurchatkin
Copy link

Can you provide an example where this matters? It seems that there is no legal way to observe this behaviour.

@panta82
Copy link
Author

panta82 commented Dec 6, 2021

@vkurchatkin Here is a quickly yanked out trigger system I use.
https://codesandbox.io/s/epic-paper-vuccl?file=/src/App.js

This has the aforementioned issues in strict mode.

Just for reference, I've also included my quick hacky mitigation I'm going with for now (TW):

let lastGeneratedRemoveResponderFn = null;

function isInStrictModeDoubleRender() {
  return !!console["log"]["__reactDisabledLog"];
}

function useTriggerResponder(trigger, responder) {
  const removeResponderRef = useRef(null);

  const isFirstRender = !removeResponderRef.current;
  if (
    isFirstRender &&
    isInStrictModeDoubleRender() &&
    lastGeneratedRemoveResponderFn
  ) {
    lastGeneratedRemoveResponderFn();
    lastGeneratedRemoveResponderFn = null;
  }

  useEffect(() => {
    return () => {
      if (removeResponderRef.current) {
        removeResponderRef.current();
      }
    };
  }, []);

  if (removeResponderRef.current) {
    removeResponderRef.current();
    removeResponderRef.current = null;
  }

  removeResponderRef.current = trigger.respond(responder);
  if (isFirstRender) {
    lastGeneratedRemoveResponderFn = removeResponderRef.current;
  }
}

@vkurchatkin
Copy link

Ok, so it seems that your code violates some React rules:

function useTriggerResponder(trigger, responder) {
  const removeResponderRef = useRef(null);
  useEffect(() => {
    return () => {
      if (removeResponderRef.current) {
        removeResponderRef.current();
      }
    };
  }, []);

  if (removeResponderRef.current) {
    removeResponderRef.current(); // <--- can't do that, side effect in render
    removeResponderRef.current = null;  // <--- can't do that, side effect in render
  }

  removeResponderRef.current = trigger.respond(responder);  // <--- can't do that, side effect in render
}

Strict mode does what it does specifically to surface violations like this in dev mode.

@irinakk
Copy link
Contributor

irinakk commented Dec 6, 2021

Here's an explanation on this: #18003 (comment)
for safely manage subscriptions, you might want to look into useSubscription / useMutableSource / useSyncExternalStore.

@panta82
Copy link
Author

panta82 commented Dec 6, 2021

@irinakk I thought it was safe because I was only touching refs (no state). After digging through react threads a bit, I've seen it mentioned that mutating refs is only safe in limited ways (lazy initialization, stuff like that).

I browsed through the 3 stages of react subscriptions, and my head is spinning :) It seems useSyncExternalStore will be the way forward with API-s like this. I'll probably keep the hacky version for now, and upgrade once the new api is stabilized and documented.

Thanks for your time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug
Projects
None yet
Development

No branches or pull requests

3 participants