Bug - animations get stuck in exiting, "exited" never fires

[joshribakoff-sm]

What is the current behavior?

Around 0.1% of the time [in React 18, not sure if related] onExited doesn't fire

What is the expected behavior?

onExit should always fire

Could you provide a CodeSandbox demo reproducing the bug?

Unfortunately I have only been able to reproduce this in a larger app using Material UI.

We have intermittent bugs where the MUI Menu randomly leaves a Backdrop covering our whole page with 0% opacity.

Through debugging what I originally thought was an MUI issue in the React devtools, I can see MUI fails to hit this condition https://github.com/mui/material-ui/blob/master/packages/mui-base/src/PopperUnstyled/PopperUnstyled.js#L202-L204

The only possible explanation (that I can think of) is that react-transition-group fails to inform the consuming MUI component that the animation is done. The result is that my backdrop has 0% opacity, but MUI never unmounts it, so it steals clicks from the whole page and appears our app is "frozen".

I noticed here is where you set the EXITING, https://github.com/reactjs/react-transition-group/blob/master/src/Transition.js#L272-L280 I'm not sure I follow this callback logic, but here you conditionally bail out of the callback https://github.com/reactjs/react-transition-group/blob/master/src/Transition.js#L302-L307 it must be related, I am speculating.

Here is where I think it's canceling the callback https://github.com/reactjs/react-transition-group/blob/master/src/Transition.js#L190-L192 and the reason that line runs is that when I select a menu item I update state in my app that triggers something unrelated to suspend in a transition, and I think React 18 reconciler may be doing something funky where it unmounts and remounts stuff with the original state preserved https://reactjs.org/blog/2022/03/29/react-v18.html#new-strict-mode-behaviors

React simulates mounting the component with the previous state.

I think it may do something similar if the tree above suspends?

If it happens to unmount in between EXITING and EXITED, it cancels the callback. Later on when React 18 restores the tree, it restores it to the EXITING state but there is no longer any callback pending to move it towards the terminal state of EXITED, therefore I'm left with this artifact in my UI of a div with 0% opacity preventing clicks.

[joshribakoff]

This still happens after turning off strict mode fyi

[OllyHodgson]

I ran into a very similar issue, where in certain scenarios the transition never reaches the exited state.

In my case it happens when navigating to a new route where suspense and lazy got involved (i.e. the route components are lazy-loaded). For me the workaround/fix was to pass a key attribute to the Transition component, e.g.

// Get a key from Reach Router
const { key } = useLocation();

return (
  <Transition key={key} 
  {/* etc... */}

Unfortunately I wasn't able to produce a reduced test case - it always worked fine in a simpler environment 🤯

[joshribakoff-sm]

Unfortunately I wasn't able to produce a reduced test case

Yeah same. I'm hoping the maintainers can assume this is a valid bug and the burden can be on proving the logic correct, the logic is hard to follow and "looks like" it probably has race conditions.

The way I solved this was to get rid of the MUI menus, and write my own. I wrote my own transitions, too. In my transition, I use a little known React trick where it's actually valid to call setState directly in render, so I have logic like this:

  if (state === 'closed' && in) {
    setState('opening');
  } else if (state === 'opened' && !in) {
    setState('closing');
  }

I'm also using react-spring and I use the onRest callback prop to set the opened and closed state when the animation ends, depending on the state of the animation. I'm being facetiously snarky here, but in like 6 lines of code I eliminated all the nested callbacks and race conditions that react-transition has (in reality I took on a lot of maintenance burden like the CSS for the transitions, etc). My point is, I don't think it would be hard to rethink the state machine and avoid the callback hell

[circlingthesun]

I've tried to create a minimal reproduction but failed. Here is a starting point for anyone else who'd like to try: https://codesandbox.io/s/stoic-gates-qvu5lh?file=/src/App.js

[haxxxton]

this might help with y'all above, as i was encountering the same issue.

My application is such that i have a Context around my routes that would store an array of alerts. I then had an AlertRoot component that would render Alerts based upon the data coming from the Context within my base route. When an alert is dismissed it would tell the Context it's no longer the "top" alert, changing the Transition in prop, with an onExited function that would tell the Context to remove the alert from its array. Like so:

// ./index.jsx
<AlertProvider>
  <RouterProvider router={myRouter} />
</AlertProvider>

// ./components/LayoutWrapper.jsx
<div>
  {children}
  <AlertRoot />
</div>

// ./routes/*.jsx
// ie. any route component
<LayoutWrapper>
  {/* route specific content */}
</LayoutWrapper>

// ./components/AlertRoot
<div>
  {alerts.map((alert) => (
    <Alert
      key={alert.key}
      in={alert.key === topAlert} // <-- when false, begins Transition exit
      onExited={() => deleteAlert(alert.key)} // <-- callback when Transition onExited
    />
  ))}
  {alerts.length > 0 && <Backdrop />}
</div>

This worked perfectly, until introduced an alert that would change the route (eg. "our privacy policy has changed, would you like to view it? [Yes] [No]", where "yes" would take the user to /privacy). What was happening, was that AlertRoot would unmount (which in turn would unmount the Transition), before the transition could complete, not firing the onExited callback, and leaving my Backdrop component rendered.

I managed to solve this by removing AlertRoot from LayoutWrapper, and moving it to ./index.jsx. This prevents AlertRoot from being unmounted when the route changes, allowing onExited to fire, as the Transition gets to complete properly

// ./index.jsx
<AlertProvider>
  <RouterProvider router={myRouter} />
+  <AlertRoot />
</AlertProvider>

// ./components/LayoutWrapper.jsx
<div>
  {children}
-  <AlertRoot />
</div>

Unsure if this will help anyone, as your mileage may vary, but thought it might be worth mentioning

[joshribakoff-sm]

What was happening, was that AlertRoot would unmount (which in turn would unmount the Transition), before the transition could complete, not firing the onExited callback, and leaving my Backdrop component rendered.

Interesting theory, but if the transition was unmounted why was the backdrop also not unmounted? Transition or not, portal or not, if the route unmounts all of it's children should also unmount. I think because this library uses too many "escape hatches" (or techniques that may have been only necessary in older versions of React)

[haxxxton]

@joshribakoff-sm , in the example code i showed AlertRoot is rendered "outside" the route rendering location (after being moved to the ./index.jsx file).

The render logic i had within AlertRoot says "render a backdrop if there are any alerts (regardless of if they are "in")", and the onExited callback is what actually deletes the alert from my list of alerts.

In my initial code, this was leaving alerts in my array undeleted, with no alert being "active" (ie. topAlert)

I can't speak to the escape hatches, in my case this felt like user error on my part. Perhaps a "unmounted before onExited could fire" warning while in development mode may help to "catch" these sorts of issues?

[mihanizm56]

Get the following error with react 18. Any updates?

[joacub]

any updates in this ??

[tomas-c]

I'm also getting this issue.

Spent some time investigating and discovered that for some reason when the transition ends, this.setState on this line here may not actually update the state or trigger the callback provided as the second argument.

The only case where I would expect React to ignore this.setState is when the component is being unmounted. But in my case componentWillUnmount is not called so must be something else.