[Bug]: useNavigate function triggers constant rerender when used in useEffect

[chrishoermann]

What version of React Router are you using?

6.0.2

Steps to Reproduce

In ReactRouter 6.0.0.beta2 the following code worked and executed only when isAuthenticated changed.

export const AppRouter = () => {
  const { isAuthenticated } = useAuthenticatedUser();
  const navigate = useNavigate();
  const routes = useRoutes(APP_ROUTES);

  useEffect(() => {
    if (isAuthenticated) {
      navigate("Dashboard");
    } else {
      navigate("Login");
    }
  }, [isAuthenticated, navigate]);

  return <Suspense fallback={<LoadingPage />}>{routes}</Suspense>;
};

also this snippet ran useEffect only when activeUser.id changed:

export const Users = () => {
  const { activeUser } = useActiveUser();
  const navigate = useNavigate();

  useEffect(() => {
    if (activeUser.id) {
      navigate(`${activeUser.name}/${activeUser.view}`);
    }

    if (activeUser.id === '') {
      navigate(``);
    }
  }, [navigate, activeUser]);

  return (
    <ContentWithDrawer drawerContent={<UsersNav />}>
      <Suspense fallback={<ContentLoading loading={true} Icon={AccountCircleIcon} withFade={false} />}>
        <Routes>
          <Route path=":name/*" element={<UsersContent />} />
          <Route path="" element={<ContentLoading loading={false} Icon={AccountCircleIcon} />} />
        </Routes>
      </Suspense>
    </ContentWithDrawer>
  );
};

In the current version a re-render is triggered every time navigate() is called somewhere in the App.

This is, I assume, caused by one of the dependencies in the dependecy array of navigate's useCallback. Everytime navigate is called a dependency changes and forces useCallback to create a new function and in the process kill referential equality of navigate which triggers the useEffect. This is just a wild guess because this is one of the main differences that I noticed in a quick overview of useNavigate between 6.0.0-beta.2 vs 6.0.2. I also checked 6.0.0-beta.3. and this behavior exists since then.

Expected Behavior

When navigate function of useNavigate is used as a useEffect dependecy the referential-equality of navigate should be ensured on every route change.

Actual Behavior

The navigate function of useNavigate triggers useEffect on every route change.

[MeiKatz]

useEffect() runs the first time on mount. Then navigate() gets executed. Therefore each. Mount effects a re-mount and this in an eternal loop.

[chrishoermann]

@MeiKatz actually it does not initiate an endless loop but just toggles back to "Dashboard" on every route change. That is it seems, because navigate is not referentially equal to the navigate function of the previous render.

The strange thing is that this useEffect worked in 6.0.0-beta.2. So back then the navigate function obviously had referential-equality and the useCallback did what it was meant to do: keep referential equality to prevent remounts of useEffect when they are not intended. The code of useNavigate back then was:

function useNavigate() {
  !useInRouterContext() ?  invariant(false, // TODO: This error is probably because they somehow have 2 versions of the
  // router loaded. We can help them understand how to avoid that.
  `useNavigate() may be used only in the context of a <Router> component.`)  : void 0;
  let navigator = useContext(NavigatorContext);
  let {
    pathname,
    basename
  } = useContext(RouteContext);
  let activeRef = useRef(false);
  useEffect(() => {
    activeRef.current = true;
  });
  let navigate = useCallback((to, options = {}) => {
    if (activeRef.current) {
      if (typeof to === "number") {
        navigator.go(to);
      } else {
        let path = resolvePath(to, pathname, basename);
        (!!options.replace ? navigator.replace : navigator.push)(path, options.state);
      }
    } else {
       warning(false, `You should call navigate() in a useEffect, not when ` + `your component is first rendered.`) ;
    }
  }, [basename, navigator, pathname]);
  return navigate;
}

The code of useNavigate now is:

export function useNavigate(): NavigateFunction {
  invariant(
    useInRouterContext(),
    // TODO: This error is probably because they somehow have 2 versions of the
    // router loaded. We can help them understand how to avoid that.
    `useNavigate() may be used only in the context of a <Router> component.`
  );

  let { basename, navigator } = React.useContext(NavigationContext);
  let { matches } = React.useContext(RouteContext);
  let { pathname: locationPathname } = useLocation();

  let routePathnamesJson = JSON.stringify(
    matches.map(match => match.pathnameBase)
  );

  let activeRef = React.useRef(false);
  React.useEffect(() => {
    activeRef.current = true;
  });

  let navigate: NavigateFunction = React.useCallback(
    (to: To | number, options: { replace?: boolean; state?: any } = {}) => {
      warning(
        activeRef.current,
        `You should call navigate() in a React.useEffect(), not when ` +
          `your component is first rendered.`
      );

      if (!activeRef.current) return;

      if (typeof to === "number") {
        navigator.go(to);
        return;
      }

      let path = resolveTo(
        to,
        JSON.parse(routePathnamesJson),
        locationPathname
      );

      if (basename !== "/") {
        path.pathname = joinPaths([basename, path.pathname]);
      }

      (!!options.replace ? navigator.replace : navigator.push)(
        path,
        options.state
      );
    },
    [basename, navigator, routePathnamesJson, locationPathname]
  );

  return navigate;
}

So this hook grew and something must have happend along the way that broke this ref-equality thing and it must be somewhere between 6.0.0-beta.2 and 6.0.0-beta.3 because in this version the behavior started.

For me it is kind of essential that this function has referential equality between route changes because some things like the examples above rely on it.

[chrishoermann]

I found a (temporary?) solution for my problem that seems to work using a ref - it just is not as elegant as without the ref.

export const AppRouter = () => {
  const { isAuthenticated } = useAuthenticatedUser();
  const routes = useRoutes(APP_ROUTES);
  const navigate = useNavigate();
  const initialRoutesSetRef = useRef(false);

  // This `useEffect` hook sets the route based on the `isAuthenticated` state
  // of `authUserVar` when the app is renderd the first time.
  // `authUserVar` is a reactive variable (state management from ApolloClient)
  // that is initially populated with undefined values or with values from localStorage.
  //
  // The initial route is set to `dashboard` when the user had the `stayLoggedIn` flag set
  // on the last login and therefor the login data was stored in local storage.
  // If this data could not be found in localStorage the user is redirected to the `login` page.
  //
  // Due to some changes in the react router past 6.0.0-beta.2 the referential-equality
  // of `navigate` that should be ensured by `useCallback` in `useNavigate` was not given anymore.
  //
  // To circumvent this drawback, the `initialRoutesSetRef` is set to `true` once the initial
  // decision, where the user should be directed to, was made and therefore `navigate` is not called again.

  useEffect(() => {
    if (!initialRoutesSetRef.current) {
      if (isAuthenticated) {
        initialRoutesSetRef.current = true;
        navigate(DASHBOARD.absolutePath);
      } else {
        initialRoutesSetRef.current = true;
        navigate(LOGIN.absolutePath);
      }
    }
  }, [isAuthenticated, navigate]);

  return <Suspense fallback={<LoadingPage />}>{routes}</Suspense>;

The question is if the useCallback for navigate() makes sense at all, when one of the the dependencies (locationPathname ??) change on every route change?

[otakustay]

It seems navigate's reference will change if we support relative navigation in useNavigate, maybe we can add a new hook to support ONLY absolute navigation to ensure it is reference stable?

[flexdinesh]

I created a small provider that prevents updates from router context for this.

import React from 'react';
import {useNavigate as useNavigateOriginal, useLocation as useLocationOriginal} from 'react-router-dom';

// eslint-disable-next-line @typescript-eslint/no-explicit-any
const RouterUtilsContext = React.createContext<any>(null);

/*
  With this RouterUtilsContext - we tank the updates from react-router context and
  drill down navigate and location from a separate context.
  This will prevent re-render of consumer components of these hooks for every route change
  and allow using these hooks as utilities instead of context subscribers
*/
const RouterUtils: React.FC = ({children}) => {
  const navigate = useNavigateOriginal();
  const location = useLocationOriginal();

  // useRef retains object reference between re-renders
  const navigateRef = React.useRef<ReturnType<typeof useNavigateOriginal>>(navigate);
  const locationRef = React.useRef<ReturnType<typeof useLocationOriginal>>(location);

  navigateRef.current = navigate;
  locationRef.current = location;

  // contextValue never changes between re-renders since refs don't change between re-renders
  const contextValue = React.useMemo(() => {
    return {navigateRef, locationRef};
  }, [locationRef, navigateRef]);

  // since contextValue never changes between re-renders, components/hooks using this context
  // won't re-render when router context updates
  return <RouterUtilsContext.Provider value={contextValue}>{children}</RouterUtilsContext.Provider>;
};

/* 

  useNavigate() re-rendering all components is a known bug in react-router
  and might get fixed soon. https://github.com/remix-run/react-router/issues/8349
  Please be aware: when the url changes - this hook will NOT re-render 
  Only use it as a utility to push url changes into Router history
  which will then re-render the whole route component.
  Eg. const navigate = useNavigate();
*/
export const useNavigate = () => {
  const {navigateRef} = React.useContext(RouterUtilsContext);
  return navigateRef.current as ReturnType<typeof useNavigateOriginal>;
};

/* 
  Please be aware: when the url changes - this hook will NOT re-render 
  Only use it as a utility to get latest location object.
  Eg. const location = useLocation();
*/
export const useLocation = () => {
  const {locationRef} = React.useContext(RouterUtilsContext);
  return locationRef.current as ReturnType<typeof useLocationOriginal>;
};

export default RouterUtils;

And in your app — add this provider below Router

  return (
    ...
      <Router>
        <RouterUtils>
          ...
        </RouterUtils>
      </Router>
    ...
  );

[chrishoermann]

It seems navigate's reference will change if we support relative navigation in useNavigate, maybe we can add a new hook to support ONLY absolute navigation to ensure it is reference stable?

Or we can introduce a config variable in the useNavigate hook that returns either a navigate function for absolute or relative navigation like:

const navigateAbsolute = useNavigate({ navStyle: "ABSOLUTE" })
const navigateRelative = useNavigate({ navStyle: "RELATIVE" })

One option should then be set as default to minimize the necessity of refactors in existing code.

Another way could be to set the standard navStyle in providers like:

<BrowserRouter navStyle="ABSOLUTE" >
  <Routes > 
        {/* uses "ABSOLUTE" */}
  </ Routes >
  <Routes navStyle="RELATIVE" > 
        {/* uses "ABSOLUTE" */}
  </ Routes >
</ BrowserRouter>

This navStyle is then used througout the whole app and can be overwritten in each useNavigate hook as mentioned above or in each Routes provider. This could introduce great flexibility for each use case and, by choosing a reasonable default, could also prevent any further refactors in existing code.

[HansBrende]

@chrishoermann I don't think devs should have to choose between an "ABSOLUTE" nav style and buggy rendering. Why not simply fix the bug? See my comment on this duplicate issue for a bugfix:

#7634 (comment)

[chrishoermann]

closing this issue in favor of #7634 where an actual solution is suggested.