facebook / react

The library for web and native user interfaces.
https://react.dev
MIT License
228.54k stars 46.78k forks source link

Bug: `useId` generates a new different ID on second render of Strict Mode before un-mount #27103

Closed ernestostifano closed 3 months ago

ernestostifano commented 1 year ago

React version: v18.2.0

Steps To Reproduce

  1. Create a component with useId and log the generated ID inside the component body.
  2. Use an useEffect to log when the component mounts/un-mounts
  3. Mount the component in Strict Mode.
  4. Note how first and second render have different IDs (all of this before the component even un-mounts).

Link to code example: https://codesandbox.io/s/modern-currying-nqqn68

The current behavior

useId is returning different IDs across renders of the same component instance without un-mounting.

I know that Strict Mode does a lot of weird things to make sure components are pure and I understand the double-render idea and even skipping deps checks for some hooks, but I hope returning different IDs between two renders of the same component instance is not the expected behaviour.

As you can see in the linked sandbox, the following code:

  const id = useId();

  console.log("RENDER", id);

  useMemo(() => {
    console.log("MEMO");
  }, []);

  useEffect(() => {
    console.log("MOUNT");

    return () => {
      console.log("UN-MOUNT");
    };
  }, []);

Produces the following output:

RENDER :r0: 
MEMO 
RENDER :r1: 
MEMO 
MOUNT 
UN-MOUNT 
MOUNT 

Which makes absolutely no sense to me.

The expected behavior

useId should provide the same ID during the entire life of a component instance. It should only change if the component mounts again.

TamashiiD commented 1 year ago

I see the same issue and I think it has something to do with the useEffect. I'm not sure but for me useEffect always makes everything run 2 times for me instead of the 1 time that it's supposed to. Like not only is it rendering a different ID but it is also mounting twice. Is it supposed to Mount twice? I'm a beginner and just looking into these things. please tell me what you think. thank you.

TamashiiD commented 1 year ago

hmm I'm looking more into this and yes the useId Hook has a problem it says : "With server rendering, useId requires an identical component tree on the server and the client. If the trees you render on the server and the client don’t match exactly, the generated IDs won’t match" maybe this is the problem although I still don't know what the identical component tree on a server and a client is. But what you are experiencing is "normal"? I'm not sure.

icyJoseph commented 1 year ago

Not sure if related, but https://codesandbox.io/p/sandbox/empty-butterfly-y36f8d?file=%2Fapp%2Flayout.tsx%3A1%2C1 in Next.js App Dir pages, a similar issue is happening.

Edit

Nope, looks like https://github.com/vercel/next.js/issues/53110 is a Next.js issue.

JSerZANP commented 1 year ago

I guess this is expected behavior.

  1. under <StrictMode/>, component is rendered twice https://github.com/facebook/react/blob/main/packages/react-reconciler/src/ReactFiberHooks.js#L597-L610

  2. and for useId(), it falls back to a global counter if it is no in hydration.https://github.com/facebook/react/blob/main/packages/react-reconciler/src/ReactFiberHooks.js#L2613-L2632

In your demo, you are NOT doing hydration and also under <StrictMode/> so you get different ids.

This is not about useId(), if you switch to const [id] = useState(() => performance.now()), you still get the unstable ids. I'm not React maintainer, but I don't think this has an easy fix because of what StrictMode does. Also this usually should not cause trouble.

ernestostifano commented 1 year ago

I'm not React maintainer, but I don't think this has an easy fix because of what StrictMode does. Also this usually should not cause trouble.

@JSerZANP I am aware of what StrictMode does. The problem is the order in which everything is happening. Each instance of a component should have its own unique and persistent ID. So, it makes sense that the ID changes after the component un-mounts and mounts again, but not across re-renders of the same instance (before un-mount happens).

Now, I am also aware that having useId working as described above (which is the only way it should work in my opinion) opens the possibility to bypass other hooks strict controls (like useMemo that will be called twice and useRef that will reset its value after the second render). For example, by keeping an external storage using the instance ID.

But, if this is in fact on purpose, then I really think it is an exaggeration. It makes useId unsound, broken. If people want to consciously do strange things and maybe navigate the grey areas of StrictMode rules, so be it. But we cannot make useId "useless".

As it is right now, it could only be reliably used for input/label HTML IDs... so, at least, change the hook name right? So people will not get confused thinking that ID actually remains constant through the entire life of the instance.

It also makes it very easy to create memory leaks.

Again, in some way it is genius. The ID is coherent inside the useEffect (mount/unmount). Makes sense if it is on purpose to avoid non-pure logic in the component body, but it should be at least documented.

I've been working with React for years and I cannot wrap my head around this.

nvie commented 1 year ago

I also ran into this issue! One workaround I found that respects React’s rules and StrictMode rules is to move the generation of such IDs to outside of the component—if that’s an option for you.

For example:

function Outer() {
  const id = useId()
  return <Inner id={id} />
}

function Inner() {
  const id = props.id;  // Read stable ID from props instead

  console.log("RENDER", id);

  useMemo(() => {
    console.log("MEMO");
  }, []);

  useEffect(() => {
    console.log("MOUNT");

    return () => {
      console.log("UN-MOUNT");
    };
  }, []);
}

This way, each component behaves correctly in its own “strict mode” context.

I hated this solution as it definitely feels like a workaround / fighting the system, though. But maybe this sparks a new branch of thoughts. I’m still open to finding a better solution for this too.

github-actions[bot] commented 6 months ago

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

ernestostifano commented 6 months ago

bump

github-actions[bot] commented 3 months ago

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

github-actions[bot] commented 3 months ago

Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please create a new issue with up-to-date information. Thank you!