Bug: React 18 Strict mode does not simulate unsetting and re-setting DOM refs

[JMartinCollins]

React version: 18.1.0

Steps To Reproduce

1. Use <React.StrictMode>
2. Set a ref to a JSX element in a component
3. Create a useLayoutEffect/useEffect in the component where the returned cleanup function console logs the ref
4. Save and refresh the app
5. You should have access to the ref element in the the useLayoutEffect/useEffect cleanup function during the simulated unmount.

Link to code example: https://codesandbox.io/s/proud-snow-ox4ngx?file=/src/App.js

The current behavior

Does not unset/re-set refs in simulated unmount. This could lead to unexpected bugs in development like having double-set event listeners, i.e.if( ref.current ){ //add eventlistener to it }and might not match the behavior of actually unmounting the DOM node as described in the docs: https://reactjs.org/docs/strict-mode.html

On the second mount, React will restore the state from the first mount. This feature simulates user behavior such as a user tabbing away from a screen and back, ensuring that code will properly handle state restoration.

The expected behavior

In normal unmounts and mounts refs are unset(before layout effects are cleaned up) and set(before layout effects).

[gaearon]

Thanks, will check with the team.

[Razzwan]

I confirm. In my case, this causes a nasty bug. Everything related refs that worked in version 17 just stopped working in version 18 without any error messages. I definitely can't use React.StrictMode right now.

Moreover, during the first installation using create-react-app, a person will not be able to use any library that uses the refs functionality, thinking that the library does not work. My suggestion is to remove React.StrictMode from default create-react-app set up until the bug is fixed.

[JMartinCollins]

According to this post:

Whether a component gets hidden or unmounted, refs that are attached to host components (like HTML elements) will be cleaned up and re-initialized the same as they are today during unmount/remount.

Refs that store use-managed values won't be reset when a component is hidden though. They allow things to be persisted for the case where a component is hidden and later shown again.

It makes sense to keep the user-managed values around as they are part of the component state that will be restored, however refs to DOM elements maybe should not be set?

[gaearon]

a person will not be able to use any library that uses the refs functionality, thinking that the library does not work

I may be missing something but this statement sounds massively overblown. There are lots of libraries using refs that work fine with React 18. While the issue discussed here might be legit (we're discussing that), can you share what kind of code you have that doesn't work? Specific examples are most helpful.

[JMartinCollins]

sandbox link

Also something to consider is that the current effect cleanup order is different between simulated(bubble) and standard(capture) unmounts.

[huynhit24]

in React 18 every time I render the component mount, the callback runs twice, I don't know what to do?

[gaearon]

@huynhit24 That's intentional behavior in Strict Mode and unrelated to this thread. Please refer to https://reactjs.org/blog/2022/03/29/react-v18.html#new-strict-mode-behaviors.

[dmitryKochergin]

Hi! Any updates for this?

While the issue discussed here might be legit (we're discussing that), can you share what kind of code you have that doesn't work?

For example, useChangeEffect hook, quite commonly used, I guess.
Initial implementation was based on the fact, that after mount ref always has initial value, but now it isn't true:
https://codesandbox.io/s/dry-grass-0uyhx4

Basically, it was like axiom - refs always have initial values after mount.
Now the axiom is broken.

Of course, for this particular case you can use cleanup function to set isMounted ref back to false, but if you want to implement useChangeEffect with cleanup functions provided by user, you need to use two useEffects instead of one - one for setting isMount ref (without deps), and the other for actual effect with deps.
Something like this:

const useChangeEffect = (effect, deps) => {
  const isMounted = useRef(false);
  
  useEffect(() => {
    if (isMounted.current) return effect();
    
    isMounted.current = true;
  }, deps);

  useEffect(() => {
    isMounted.current = true;
    
    return () => {
      isMounted.current = false;
    };
  }, []);
};

And now we have to use two effects instead of one for implementing the same functionality, just because of artefact of dev mode. That's kind of strange to me.

[gaearon]

Initial implementation was based on the fact, that after mount ref always has initial value, but now it isn't true:

I think you're confusing the issue in this thread (<div ref=...> not getting "detached" during simulated cleanup) with something different. I'm not sure what behavior you want exactly, but I suggest to file a separate issue about that. More generally, I'd say Hooks like useChangeEffect are not a good idea, but it's hard to say more without actually seeing what you're using it for. Regardless, it's off-topic for this thread.

[dmitryKochergin]

I'm not sure, but it looked like these cases have the same reason.
Because at the moment all refs are not getting "detached" during simulated unmount.
I was going to create a separated issue, but found this one.

Anyway, thank you for the answer! I'll create a separated issue, then.

[HenrikBechmann]

@gaearon I use

const isMountedRef = useRef(true)

useEffect(() => {

  return () => {isMountedRef.current = false}

},[])

Then I check isMountedRef.current before changing state in a hook component. The use case is an infiniteScroller, where async unmounting is common.

This business of a "test" unmount in strict mode is screwing that up. It means if someone downloads my library during dev mode with strict mode, my library doesn't work.

So, I'm not happy!

Any suggestions of an alternative way of checking if a component is still mounted?

As a general comment, this looks like one of those cases in which a kind of patronizing attitude to developers with unexpected behaviours causes unwanted side effects. Just saying...

[HenrikBechmann]

Further to this, your documentation about this says that you 'restore state' after your faux unmount, so I used a setIsMounted(false) call from useState in a useEffect cleanup function (returned from non-dependent useEffect), hoping that the state would be returned to true, as your documentation states.

But, no such luck. State is not restored after the faux unmount, according to my tests.

Maybe the solution here is to finally give us a native isComponentMounted() function.

Just a thought...

Any advice from anyone would be appreciated. This is a nasty issue for me.

[HenrikBechmann]

Ok, so my workaround is this. I find that with React.StrictMode useEffect with no dependencies actually runs twice (in spite of your documentation promising it only runs once). So my revised code:

const isMountedRef = useRef(true)

useEffect(() => {
  isMountedRef.current = true // to compensate for faux unmount
  return () => {isMountedRef.current = false}

},[])

I have my fingers crossed that this happens fast enough to avoid some of my processes being wrongly missed while isMountedRef.current is incorrectly set to false.

@Gaeron you've been quoted as saying that this approach is "lazy"; that the correct thing to do is to "cancellation". But here's what I'm missing - if the reason to cancel a process is because your component has been unmounted, and there's NO WAY TO TELL IF YOUR COMPONENT IS UNMOUNTED, then how the heck are you supposed to know when to cancel something??

There's 'abortController.abort()', but that also is run in a useEffect cleanup routine, so with React.StrictMode it is run incorrectly!!

Like, seriously, what are you talking about?

[JMartinCollins]

@HenrikBechmann I think what you're getting it is at the core of the issue with this (possible) bug. I think an effective way to check whether a component is mounted would be to use a ref to a jsx/DOM element in the component:

const isMountedRef = useRef(null);
const isMountInitialized = useRef(false);

useEffect(() => {
    if(isMountedRef.current && !isMountInitialized.current) {
        // do something to initialize the mount
        isMountInitialized.current = true;
    }
    return () => {
        if(!isMountedRef.current) { // on unmount, isMountedRef.current should be null. This isn't currently the case on simulated unmounts though
            // perform unmount cleanup
            isMountInitialized.current = false; // in case the component state is being saved to remount later
        }
    }
});

return (
    <div ref={isMountedRef}>
        {/*contents*/}
    </div>
)

That is the main issue with the functionality mentioned in the issue. If the simulated unmount doesn't clear the refs like a normal unmount, this pattern can't be properly used and tested in development StrictMode.

That's also not to say that this pattern is good or even necessary to use, but in my opinion it should be expected to work this way, however it can't work this way at the moment due to the (possible) bug.

[KhaledElmorsy]

Are there any updates to this issue? Using refs to track a component's first mount isn't possible in strict mode at the moment because of this bug. Unless with a workaround cleanup function to reset it.

function App() {
  let firstMount = useRef(true)

  useEffect(()=>{
    if (firstMount.current) {
      firstMount.current = false;
      console.log('This should log')
      return () => firstMount.current = true // I'd rather not leave workarounds that will eventually expire in my code
    }
    console.log('This shouldnt log')
  },[])
}

Having to choose between the benefits of strict mode and having callbacks that'll need to be removed in the future throughout my app is definitely not easy. I hope the team can fix this soon.

[georeith]

I'm having the same issue but with useState.

In useState I create a class that does some binding to event listeners in its constructor. On unmount I want to clean up those event listeners. In strict mode, the effect runs twice (cleaning up my event listeners) but the useState is not rerun so I'm left in a broken state:

const [pluginServer] = useState(
  () => new PluginServer({ url, name, document, ...options })
);

useEffect(() => {
  return () => {
    pluginServer.dispose();
  }
}, [pluginServer]);

[tnorling]

We just ran into this as well. In our case we're attempting to set React state with the result of an async operation, so in order to prevent state updates on an unmounted component we use a ref to track whether the component is mounted or not (as seems to be a common pattern). With this change to StrictMode mounted.current always returns false when it's time to update state and breaks our hook.

Contrived example:

const [result, setResult] = useState(null);
const mounted = useRef(true);

useEffect(() => {
   return () => mounted.current = false;
}, []);

useEffect(() => {
   asyncAPI.then((asyncResponse) => {
     if (mounted.current) { // Always false with new StrictMode behavior
       setResult(asyncResponse) // State never becomes anything other than null
     }
   });
}, []);

The solution presented in this comment appears to fix this. But I'm wondering if addressing this is indeed our responsibility or if this should be addressed in React itself? Or is there a better way to determine if it's safe to update state?

[tsarapke]

On our project we also caught up into that trap. For now we can't use StrictMode at all as it breaks all our applications.
We have usePrevious recommend by react team, which looks like:

function usePrevious<T>(value: T): T | null {
  const ref = useRef<T | null>(null);

  useEffect(() => {
    ref.current = value;
  }, [value]);

  return ref.current;
}

And we have component that display tiled image. Each tile - is independent instance that use custom hooks to load image, cache it, aborts previous call if was invoked. So it looks like:

function useTileLoad<T>(params: T, abortableCallback: AbortableCallback<T>) {
   const abortRef = useRef<() => void>();

   const [tile, setTile] = useState<TileImage | null>(null);

   const prevParams = usePrevious(params); // Store previous params
   
   // Callback to load tile image or re-request new one if params was changed before request completed
   const handleLoad = useCallback(async (params: T) => {
      if (abortRef.current) {
         abortRef.current(); // If abortable is exist - abort it, as we will request new one below
      }

      let aborted = false; // Temp variable to avoid state update, when request was canceled due to new request or when component unmounts

      const abortable = abortableCallback(params); // Call abortable request that returns abortion callback

      // Store current abortion request to ref
      abortRef.current = () => {
         aborted = true;
         abortable.abort();
      };

      try {
         const response = await abortable();
      
         if (!aborted) {
            setTile(response);
         } else {
           throw new Error('Request was canceled');
         }
      } catch (e) {
        console.log('Request was failed or aborted', e);
      }
   }, [abortableCallback]);

   // (1) Abort current request Promise wrapper to avoid unhandled state update
  useEffect(() => {
    return () => {
      abortRef.current?.();
    };
  }, []);

  // (2) Compare next params with previous params and request new tile in prams are different
  useEffect(() => {
      if (!isEqual(params, prevParams)) {
        handleLoad(params);
      }
  }, [props, prevProps, handleLoad])
}

What will happen above in StrictMode? Let's see:

1. Component mounts, it goes to useEffect (2), compare params and call handleLoad
2. Component immediately unmounts, and goes to useEffect (1), and abort request that was called in p1.
3. Component mounts back, it goes to useEffect (2) and does nothing next. params and prevParams are equal. usePrevious was not re-created. As result - tile image was not loaded, nothing displayed to the developer.

useRef is useState under the hood. Why does React keep state in StrictMode between mounting/umounting?
What if I want it only in few components? What if want decide by myself when it should happed to component or even custom hook, but not React?

[JMartinCollins]

@tsarapke unrelated. The issue relates specifically to refs set to DOM elements.

Looks like maybe yours can be solved by:

// ...
const prevParams = useRef<T>(null);
// ...
if(!aborted){
    prevParams.current = params;
    setTile(response)
}
// ...

But hard to say exactly when your effect (2) will run because you didn't respect exhaustive-deps and your dependencies are totally wrong. I recommend first making your effects work properly with no deps array, then add it back with all requirements for optimization.

[tsarapke]

@JMartinCollins,
Apologise, actually I missed prevParams.current = params;, we set it right before handleLoad in useEffect (2).
And I'm not sure about putting in into if statement. There some another logic, that cause <Tile /> re-rendering. It will lead call of that custom hook, which will lead to re-call useEffect (2), that will lead to call handleLoad(because prevParams still old in that case), which will abort request and make a new one.

As about issue - I think it still related. The root cause is the same. Many people at the beginning of topic provides same issue-cases.

But can you tell why does useEffect didn't respect exhaustive-deps and why dependencies are totally wrong?
There are nothing extra excess, and there nothing missed. If I'll remove anything - ts will tell me that I missed deps. If I will add something... it wouldn't change anything.

[JMartinCollins]

@tsarapke in a way it kind of is related. I think the approach that you should be taking with this hook is to pass it a DOM element ref. Then, you'll only run your abort cleanup if that ref is null because it signals that the actual component is no longer mounted. Likewise you'll only run handleLoad if the previousValue of that ref is null && the current value is an element, because that means your component just mounted. This is the behavior that's (maybe) incorrect with the current strict mode simulated unmount, which doesn't set DOM refs to null during simulated unmount.

Also the incorrect deps may just be a typo?

[tsarapke]

@JMartinCollins,

Yes, there is a typo. Must be [params, prevParams, handleLoad]. Sorry about that.
As for having additional DOM reference - isn't a bad approach? Hooks brings up to help us to split the code, to make it re-usable, to be less components dependent, to be more abstract.
That hook could be called in another hook. So it would mean that I should pass ref to real DOM from parent to nested hooks. Use context in that case? Look weird too. If you provides your hook as 3rd library, it would be not really obvious to others why does it requires DOM ref.

I really understand the proposal that React team wants to provide in a future by simulation that thing with StrictMode, but it looks like it shouldn't be compulsory and developer should be able to choose where to use it, and where to not.
So it should provide some kind of callback, like a memo or forwardRef, that will make you component to be working in that way.

[trainiac]

I want to put things a little differently than what has been said here.

By not resetting refs, the React "double render" stress test creates behavior that only exists in development and would never occur in production build and therefore creates a use case that we now have to develop for just for the sake of the "double render" stress test in development. Here's an example:

In the following code I want this behavior:

• First time a user comes to a page
  • check to see if a scroll position was stored from the last time they were on the page
    • if yes restore their scroll position
    • if no, try to scroll the first appointment of the day into view
• If the user is already on the page and changes the day they want to view, try to scroll to the first appointment of the day into view

const Schedule = ({ day }: { day: string }) => {
  const scrollRef = useRef(null)
  const hasMounted = useRef(false)

  // I think useLayoutEffect is need because we want to get 
  // the scroll position of an element before unmounting
  useLayoutEffect(() => {
    if(!hasMounted.current) {
      hasMounted.current = true
      const scrollPosition = getLastScrollPositionLocalStorage(day)
      if(scrollPosition && scrollRef.current) {
         scrollRef.current.scrollTo(scrollPositon)
         return () => {
           setLastScrollPositionLocalStorage(day,   getScrollPosition(scrollRef.current))
         }
      }
    }

    if(scrollRef.current) {
       const firstAppointmentScrollPosition = getFirstAppointmentScrollPosition()
       scrollRef.current.scrollTo(firstAppointmentScrollPosition)
    }
  
   return () => {
     setLastScrollPositionLocalStorage(day, getScrollPosition(scrollRef.current))
   }
  }, [day])
  return <ScheduleView ref={scrollRef} />
}

This code will have different behavior in development than in production. In development since refs are not reset this component will always try to scroll the first appointment into view and will not try to restore the last scroll position a user had when they were on this page.

Why? In production this effect would only run once if the day never changes. In development this runs twice no matter what.

We have workarounds to make the behavior consistent but again, we have to develop a workaround that would never happen in production just to satisfy the "double render" stress test.

It's possible React has some future plans to say "hey we are gonna run those damn effects as many times as we please" and if so makes this point moot. Thanks for your thoughts!

[runfaj]

Just my two cents here to re-iterate the problem in a different way. In my case, I'm setting up a store instance in a component via a ref hook (simplified version):

const function useCreateStore() {
  const ref = useRef(null);
  if (ref.current === null) {
    ref.current = createStoreFn();
  }
  return [ref.current, () => destroyStore(ref)]; // return the store and a "disposer" function
}

then I have a disposer that looks something like this:

function destroyStore(ref) {
  if (!ref?.current) return;

  const store = ref.current;
  store.beforeDestroy();

  ref.current = null; // set to null so garbage collection deletes the store instance
}

And here's the bit in the component:

const [storeInstance, storeDisposer] = useCreateStore(storeCreateFn);

useEffect(() => {
  storeInstance.fetch(id);
  
  return () => storeDisposer();
}, []);

console.log(storeInstance.resultData);

In my case, the store is calling an api call that uses cancel tokens. However, in strict mode, because the refs aren't automatically cleaned up as part of the second rerender, it gets into a weird state where the new component does the whole mounting flow, but the ref link gets lost, causing the component's second mount to not see the resulting data. As soon as I comment out the ref.current = null; line above, it works because the ref now can keep the link behind the scenes, therefore only a single store instance is actually used. However, this defeats the point of the strict mode, since now you've got components rerendering, but a store instance that is shared between the two mounts.

Long story short (and a lot of trial and error later), I landed on this github issue because I had to turn off the strict mode to properly keep my garbage collection flow working. It would be nice to be able to verify the api promise canceling via the strict mode (which is what I would expect), but the refs not getting cleared properly makes this impossible.

[3rd]

👍 @runfaj Just found this ticket by running into the same issue for the same use case
Edit: solved it with a reference counter and a setTimeout that checks if there are no more references which runs after the effect-cleanup-effect dance.

[ajwootto]

Also just ran into this, in our case we were using a ref to store a reference to an instance of our SDK client which was being created in a Provider-style component. The useEffect cleanup function was calling close on that client ref to make sure that on unmount the client was cleaned up.

However, the ref to that client was shared across the strict mode unmount-remount process, so two "distinct" component instances were actually sharing a reference and interfering with each other. The client instance of the remounted component was already closed by the time it rendered..

Here's the PR to work around the bug:
https://github.com/DevCycleHQ/js-sdks/pull/659/files

[dangoodman]

Any update?

It takes a lot of time to investigate and work around issues related to this buggy behavior.

[AleksandrHovhannisyan]

See also this thread: reactjs/react.dev#6123