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

<Navigate> with React 18 causes "Maximum update depth exceeded" #88

Closed
frankruegamer opened this issue Aug 3, 2022 · 7 comments
Closed

Comments

@frankruegamer
Copy link

frankruegamer commented Aug 3, 2022

This error seems to occur under very specific circumstances: CodeSandbox example

  • Navigating to /redirect using either
    • The third <NavLink/>
    • "Native Navigate" using useNavigate() hook
    • "Navigate with Redux" using dispatch(push(..))
    • Navigating to /redirect using the address bar (in the CodeSandbox browser)

will crash the App and display

Error

Maximum update depth exceeded. This can happen when a component repeatedly calls setState inside componentWillUpdate or componentDidUpdate. React limits the number of nested updates to prevent infinite loops.

Modifications to the Example that will stop the App from crashing

  • Using render from "react-dom" instead of createRoot() in index.tsx, the app will behave as if it's running React 17
  • Using useNavigate() instead of <Navigate/> (swap the <Route/> for /redirect in MainContent.tsx with the commented one, which uses <NavigateWithHook/>)
  • Not using redux-first-history, for example by replacing <HistoryRouter> with <BrowserRouter/> in App.tsx from react-router-dom
  • And strangely using useLocation() instead of useSelector() in MainContent.tsx with the state of the router

Modifications to the Example that will NOT stop the App from crashing

  • <StrictMode/> in React 18 unmounts and remounts every component once after the initial render in dev mode. But disabling Strict Mode in index.tsx will still cause the app to crash
@talkohavy
Copy link

talkohavy commented Aug 6, 2022

Same is happening to me...
That is a real let down tbh... I really like redux-first-history, and i'm not ready to give up on it.
I like how it exposes the LOCATION_CHANGE to the developer, and even provides a push for programmatic navigation when outside a component. For us this was a big deal, since all our business logic is located under the redux middlewares.

Regarding Your Solutions / Modifications :

  • Solution 1: Using render
    I dislike the fact that we need to go backwards to past versions of libraries for things to work, as we like to keep things up-to-date as much as possible. So for us - this is a no-go.
  • Solution 2: Not using redux-first-history
    As i've mentioned, I like this library a lot, and appreciate what it does, and i'm not ready to give up all its benefits. BrowersRouter doesn't provide us with all the settings and fine-tuning capabilities that we need out of the box, and HistoryRouter does.
  • Solution 3: use NavigateWithHook
    Now, this is actually a nice one! ☺ I played around with your sandbox code, and I liked the implementation of NavigateWithHook. One thing i've noticed about it though - when trying it out on our code, and adding the state option to it (to the navigate function), I still get the "Maximum update depth exceeded", but if I remove it, and stay only with the "to" and maybe the "replace" option (true or false), it is working perfectly fine! ♥
    So I still need to figure out why for you the state property doesn't ruin it, and for me it does.
    All and all, this is a nice work-around 🙏

Still,
I wish the maintainer would find the cause and fix it properly, so that we would be able to use <Navigate/> as it is, and not get this error, instead of relying on work-arounds.

@salvoravida
Copy link
Owner

salvoravida commented Sep 10, 2022

@talkohavy @frankruegamer

demo ok -> https://codesandbox.io/s/sweet-einstein-u8gi6r?file=/src/App.tsx

TL;DR

createReduxHistoryContext({
  history: createBrowserHistory(),
  batch: (fn) => Promise.resolve().then(fn)
});

@arnriu
Copy link

arnriu commented Sep 12, 2022

Hello. Just tried this in my app:

Results: Warning: Maximum update depth exceeded.

    "history": "^5.3.0",
    "react": "^18.2.0",
    "react-dom": "^18.2.0",
    "react-redux": "^8.0.2",
    "react-router-dom": "^6.3.0",
    "react-scripts": "^5.0.1",
    "redux": "^4.2.0",
    "redux-first-history": "^5.1.1",

@salvoravida
Copy link
Owner

@arnriu can you provide a codesandbox?

@salvoravida salvoravida reopened this Sep 15, 2022
@arnriu
Copy link

arnriu commented Sep 15, 2022

Sorry, didn't have time to reproduce it in a code sandbox so far, I have a huge bug in my service worker in production and I have to deal with it asap.
But, I have a lead: It could be that I use react-scripts (from create react-app) with react-app-rewired. On localhost, I changed some things to work without react-app-rewired. When I run react-app-rewired start, I have the Maximum update depth exceeded. When I run react-scripts start, it works fine.

I still need to work more on it to be able to reproduce in a code sandbox and find out more about what's happening, and be sure this is the problem and not anything else.

@salvoravida
Copy link
Owner

@arnriu no worries. feel free to update the issue if need to.

@salvoravida salvoravida pinned this issue Sep 15, 2022
@kurtwaldowski-echelon
Copy link

kurtwaldowski-echelon commented Dec 16, 2022

@talkohavy @frankruegamer

demo ok -> https://codesandbox.io/s/sweet-einstein-u8gi6r?file=/src/App.tsx

TL;DR

createReduxHistoryContext({
  history: createBrowserHistory(),
  batch: (fn) => Promise.resolve().then(fn)
});

@salvoravida Why does adding this batch resolve this issue? I just ran into this and it did indeed work, but still not sure why. One thing I did notice though is that using is substantially slower, but no longer crashes with "Maximum update depth exceeded". Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants