useNavigate hook causes waste rendering

[iammrali]

Version

6.0.0-beta.0

Test Case

I create a pure component, there is no reason to execute again when i want to navigate to other page using useNavigate , I think useNavigate hook cause waste rendering on pure components, when using useHistory hook of last version(5) it doesn't have this behavior I have a question here for more detail Am i wrong or something changed? https://codesandbox.io/s/dawn-wave-cy31r?file=/src/App.js

Steps to reproduce

Create pure component Use useNavigate hook inside it

Expected Behavior

Navigate to other page should not cause execute of pure components when props are not changed

Actual Behavior

Navigate to other page in pure component causes waste rendering, component execute again even when props are same

[timdorr]

useNavigate changes when the current location changes. It depends on it for relative navigation. Wrapping it in memo only prevents re-renders from parent components. If hooks within the component cause re-renders, there is nothing memo can do.

[iammrali]

I create this test case in v5 and use useHistory hook it doesn't have this behavior, why useNavigate isn't like that?

[benneq]

Please reopen. This causes half of my application to rerender, just because some searchParams change.

useNavigate changes when the current location changes.

But the current location is only relevant for useNavigate's internal state. This should not affect any component.

[bennettdams]

(For future readers: Workarounds shown below)

Please consider changing this to the expected behavior. As @benneq said, this causes a lot of rerenders. To my understanding, useNavigate does not need existing search params on the current location. If you need them, you can provide them via the hook already, so why not letting useNavigate bail out of rerendering?

Why this is a problem

Example of a real life use case

In my case, I have a list of very render-intense charts. I want to click the charts to navigate to a new page for a detailed view. Imagine each chart showing the temperatures of one day:

// child
function DayChart({ dayData: DayData }) {
  const navigate = useNavigate();

  return (
    <div onClick={() => navigate(`/my-route/${dayData.id}`)}>
      <Chart data={dayData} />
    </div>
  );
}

// parent
function DaysList() {
  const { days } = useAPI(...)
  return <div>{days.map((day) => <DayChart dayData={day} />)}</div>;
}

(I'm using memoization in my code, just showing the structure here.)

Now, DaysList has filters that are applied as search params. So changing any filter in DaysList will always rerender ALL charts, which is triggered by useNavigate "rerendering", which furthermore is triggered by changes in the location (search params).

Proposed solution

If useNavigate would ignore the search params when checking for the location, this wouldn't happen (I guess?). This cannot be done by the user (aka. developer who's using this library).

Workarounds

There are two workarounds I came up with:

1. Provide a wrapper function for navigate from the parent to the child Working CodeSandbox: https://codesandbox.io/s/react-router-rerender-memoized-callback-x5c76?file=/src/App.tsx You can use useNavigate in the parent instead of the children. This will still cause a rerender in the parent when changing search params, but this will not trigger a rerender of your children when they're memoized. For my example above:

// child
export const DayChart = memo(function DayChart ({
  dayData,
  navigateToDay,
}: {
  dayData: DayData;
  navigateToDay: (dayId: string) => void;
}) {
  return (
    <div onClick={() => navigateToDay(dayData.dayId)}>
      <Chart data={dayData} />
    </div>
  );
});

// parent
function DaysList() {
  const { days } = useAPI(...)
  const navigate = useNavigate();

  const navigateToDay = useCallback(
    (dayId: string): void => {
      navigate(`/my-route/${dayId}`);
    },
    [navigate]
  );

  return <div>{days.map((day) => <DayChart dayData={day} navigateToDay={navigateToDay} />)}</div>;
}

BTW: I'm not really sure why this works. You can see thatnavigateToDay does not trigger a rerender of the memoized child when it's declared in the component - I thought it will be created in the parent everytime, but somehow React knows to not rerender the child, even the useCallback's reference in the parent is changed every time. I created a CodeSandbox to show this behavior without React Router: https://codesandbox.io/s/rerender-memoized-callback-without-react-router-urz9n?file=/src/App.tsx Does someone know why?

2. Use <Navigate ... /> instead of useNavigate: If you don't know, Navigate is a component that, when rendered, will "execute" a navigate. So you could have some small boolean and conditionally render this component (in my case on button click):

// child
function DayChart({ dayData: DayData }) {
  const [shouldNavigate, setShouldNavigate] = useState<boolean>(false)

  return (
    <div onClick={() => setShouldNavigate(true)}>
      {shouldNavigate && <Navigate to={`/my-route/${dayData.dayId}`} />}  
      <Chart data={dayData} />
    </div>
  );
}

// parent
function DaysList() {
  const { days } = useAPI(...)
  return <div>{days.map((day) => <DayChart dayData={day} />)}</div>;
}

[Lure5134]

Could this reopen? For me a lot rerenders if the state changes. This is irrelevant for the useNavigate hook. What you think about a usePathname hook that just returns the pathname? That could be really helpful and could be used within the useNavigation hook, so that it doesn't renders when the state or search Paramus changes. And the <Routes> components rerenders too if the state changes. Do they really need the state? @chaance @timdorr

[cybercoder]

same problem here, when using useNavigate there's wasted renders.

[chaance]

Re-opening for consideration. I'm not sure that we'll change this behavior but I don't want to close the door on some sort of solution just yet.

[Lure5134]

I think if this PR get merged https://github.com/facebook/react/pull/20646 into react it can be used to prevent a lot of rerenders.

[RandScullard]

I have a suggestion for a change to useNavigate that might resolve this. For the sake of argument, here is a version of useNavigate stripped down to only the parts relevant to this issue:

let { pathname: locationPathname } = useLocation();

let navigate: NavigateFunction = React.useCallback(
    (to: To | number, options: NavigateOptions = {}) => {

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

    },
    [basename, navigator, routePathnamesJson, locationPathname]
);

return navigate;

I believe the root cause of the re-rendering issue under discussion is that the dependencies array passed to useCallback must include locationPathname for this code to work correctly. I think you could work around this requirement by introducing useRef:

let { pathname: locationPathname } = useLocation();

let refLocationPathname = useRef(locationPathname);
refLocationPathname.current = locationPathname;

let navigate: NavigateFunction = React.useCallback(
    (to: To | number, options: NavigateOptions = {}) => {

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

    },
    [basename, navigator, routePathnamesJson]
);

return navigate;

This way, the navigate function will always get an up-to-date value for locationPathname, but the callback won't get regenerated when the location changes.

[LuisDev99]

Any news on this?

The only simple alternative I can think of is to reload the whole page when navigating to avoid two renders but there's no support for forceRefresh in V6. I don't know how else to avoid the additional render

[aparshin]

@timdorr @chaance It seems to me that from the end-user perspective, useNavigate hook itself shouldn't cause re-rendering of the component. The semantic of this hook is to perform navigation. If you intentionally update the result of useNavigate() hook each time after location change, you implicitly suppose that the navigate() method will be called each time. I don't think that it's a common behavior (personally I even can't imaging such use cases). If someone wants to perform some actions based on location, he/she might use the useLocation hook instead.

So, if you don't change result of useNavigate hook each time, you'll better meet end-user expectations. Also, this will improve performance out of the box in many situations.

A separate question is how to implement this semantic at technical level. If you agree with the general idea, I can try to propose a PR (for example, something based on @RandScullard 's idea above).

[WinterWoods]

When upgrading from V5 to V6, the sub components jump, resulting in re rendering of the whole page, resulting in low performance. Is there a solution If not, you can only roll back the V5 version

[HansBrende]

useRef won't fix this issue because just the call to useContext (to get the location pathname) will still trigger a re-render! (See: https://github.com/facebook/react/issues/14110, https://github.com/facebook/react/pull/20646)

The key to fixing this particular issue is: don't call useContext at all (i.e., don't call useLocation inside useNavigate). There is absolutely no reason to! Here's a better implementation that avoids all the icky problems (detailed in the links above) inherent with useContext:

export function useNavigate(): NavigateFunction {
  let { basename, navigator } = React.useContext(NavigationContext);
  let { matches } = React.useContext(RouteContext);
  // let { pathname: locationPathname } = useLocation(); // <-- DO NOT DO THIS!!!!!

  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: NavigateOptions = {}) => {
      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;
      }
      // Look up the current pathname *at call-time* rather than current behavior of:
      //   1. re-rendering on every location change (incl. query, hash, etc.) (BAD!)
      //   2. creating a new navigate function on every pathname change (BAD!)
      let { pathname: locationPathname } = (navigator as any).location;  // <--- THIS IS THE KEY!!!
      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 */] // <-- NO NEED FOR THAT!!!
  );

  return navigate;
}

[HansBrende]

@chaance hopefully my above comment opens "the door" to a solution?

[RandScullard]

@HansBrende You're right, my useRef suggestion wouldn't prevent the extra renders because of the underlying reference to the LocationContext. Thank you for clarifying! (I think my useRef would help with issue #8349 though.)

I have one question about your suggested implementation. You say "The key to fixing this particular issue is: don't call useContext at all" but the first two lines of your function are calls to useContext. Is the key difference that these two calls are referencing context that never (or rarely) changes?

[HansBrende]

@RandScullard

Is the key difference that these two calls are referencing context that never (or rarely) changes?

Yes, exactly! The navigation context, for example, is set once at the top of your application by the Router, and would only get reset (causing all callers of useContext(NavigationContext) to re-render) if the basename, static, or navigator properties on the router change (not likely). So those useContext calls aren't problematic. (NOT SURE ABOUT useContext(RouteContext) though, haven't looked into whether or not that one is problematic.)

LocationContext, on the other hand, is frequently changing so you have to watch out for those useLocation calls! In fact, there are other react-router hooks where it is called which could also be problematic (e.g. in useRoutes: see digression below).

<digression> What useRoutes really needs is a usePathname so it doesn't trigger re-renders every time the query or hash changes but not the pathname! E.g.:

function usePathname(locationOverride?: Partial<Path>) {
    const {navigator: nav} = React.useContext(UNSAFE_NavigationContext) as unknown as {navigator: History};
    const [navigator, pathname] = locationOverride ? [null, locationOverride.pathname] : [nav, nav.location.pathname];
    const [, triggerRerenderOnlyOnPathnameChange] = React.useReducer(pathnameReducer, pathname || '/');
    React.useLayoutEffect(() => navigator?.listen(triggerRerenderOnlyOnPathnameChange), [navigator]);
    return pathname || '/';
}

const pathnameReducer = (_: string, upd: Update): string => upd.location.pathname;

Then in useRoutes:

// let locationFromContext = useLocation(); // <-- Don't do this!
// let location;
// if (locationArg) {
//    location = typeof locationArg === "string" ? parsePath(locationArg) : locationArg;
//  } else {
//    location = locationFromContext;
//  }
//  let pathname = location.pathname || "/";

// INSTEAD:
let pathname = usePathname(typeof locationArg === "string" ? parsePath(locationArg) : locationArg)

</digression>

EDIT: I just realized that my "digression" wasn't a digression at all, since useNavigate also calls useContext(RouteContext), where the RouteContext is provided by useRoutes() (and not memoized) and useRoutes() in turn calls useLocation()...

[HansBrende]

Ok, so, based on my "EDIT" to my above comment (and thanks to @RandScullard for getting my gears turning), to really fix this problem you will need to remove both the useLocation() and useContext(RouteContext) calls. It is really easy to shoot yourself in the foot with useContext! The removal of useLocation() is quite easy, as I've already shown. Removing useContext(RouteContext) is a little more involved because it requires tweaking _renderMatches(). Here's an updated fix:

export function useNavigate(): NavigateFunction {
  let { basename, navigator } = React.useContext(NavigationContext) ?? {};
  invariant(
     navigator != null,  // useInRouterContext(),  <-- useInRouterContext() is just as bad as useLocation()!
     `useNavigate() may be used only in the context of a <Router> component.`
   );
  // let { matches } = React.useContext(RouteContext); // <-- DO NOT DO THIS!!!!!
  // let { pathname: locationPathname } = useLocation(); // <-- DO NOT DO THIS!!!!!
  // Instead, we need to use Contexts that do not get updated very often, such as the following:
  const routeContextRef = React.useContext(NonRenderingRouteContext);
  ...
  return React.useCallback(
    (to: To | number, options: NavigateOptions = {}) => {
      ...
      // Look up the current pathname AND ROUTE CONTEXT *at call-time* rather than current behavior of:
      //   1. re-rendering on every location change (incl. query, hash, etc.) (BAD!)
      //   2. creating a new navigate function on every pathname change (BAD!)
      let path = resolveTo(
        to,
        routeContextRef.current.matches.map(match => match.pathnameBase),
        stripBasename((navigator as History).location.pathname || "/", basename)
      );
      ...
      navigator.doSomethingWith(path); // ta-da!
    },
    [basename, navigator, routeContextRef]
  );
}

// Where the "NonRenderingRouteContext" is provided, for example, as follows:

const NonRenderingRouteContext = React.createContext<{readonly current: RouteContextObject}>({
  current: {
    outlet: null,
    matches: []
  }
});

function RouteContextProvider({value}: {value: RouteContextObject}) {
  const ref = React.useRef(value);
  React.useLayoutEffect(() => void (ref.current = value)); // Mutating a Ref does not trigger a re-render! 👍
  const match = value.matches[value.matches.length - 1];
  return ( // Refs are stable over the lifetime of the component, so that's great for Contexts 👍
      <NonRenderingRouteContext.Provider value={ref}> 
        <RouteContext.Provider value={value}>
          {match.route.element !== undefined ? match.route.element : <Outlet />}
        </RouteContext.Provider>
      </NonRenderingRouteContext.Provider>
  );
}

function _renderMatches(matches: RouteMatch[] | null, parentMatches: RouteMatch[] = []): React.ReactElement | null {
  if (matches == null) return null;
  return matches.reduceRight((outlet, match, index) => {
    return (
      <RouteContextProvider  // Simply swap out RouteContext.Provider for RouteContextProvider here!
        value={{
          outlet,
          matches: parentMatches.concat(matches.slice(0, index + 1))
        }}
      />
    );
  }, null);
}

[Edited to ALSO remove useInRouterContext(), which calls useContext(LocationContext) under the hood!] [Edit #2: added stripBasename to ensure consistency with the pathname retrieved from useLocation()]

[ryanflorence]

Is anybody measuring performance issues here or just console logging "rerender!" and stressing out?

I'd be interested to see an example where this actually affects the user experience.

The fact is that navigate depends on location for relative navigation, so it changes with it.

[ryanflorence]

Just reviewed some of the code samples here.

Refs shouldn't be assigned during render, only in effects or event handlers. Maybe the React team has changed their position on this recently, but mutating refs while rendering can cause "tearing" in concurrent mode/suspense. We're just following the rules here: pass dependencies to effects.

Since navigate isn't "rendered", it is unfortunate that its dependency on location causes re-renders.

If you've got actual performance problems (not just console logging "rerender!") the simplest solution I can think of right now is to call useNavigate at the top and make your own context provider with it--but remember not to use any relative navigation.

let AbsoluteNavigate = React.createContext();

function Root() {
  let [navigate] = useState(useNavigate())
  return (
    <AbsoluteNavigate.Provider value={navigate}>
      <Routes>
        {/*...*/}
      </Routes>
    <AbsoluteNavigate.Provider>
  )
)

Anyway, this isn't high priority, but the ref tricks could work if they were only assigned in a useEffect, or there's probably some mutable state on navigator/history that we can use to access the location pathname instead of useLocation. But it also depends on the current route matches, which change w/ the location, we'd have to call the route matching code in navigate to get that too.

I personally don't worry about "wasted renders", I trust React is good at what it does and just follow the rules of hooks.

I'd likely merge a PR that optimizes this though, so knock yourself out :)

[HansBrende]

@ryanflorence IMHO, the question "is anyone measuring performance issues here" completely sidesteps this issue. Any React library worth its salt (that a lot of people are using) should be very conscious of not contributing to "death by a thousand cuts"... that means unnecessary rendering because "who cares, it's not a big performance problem in and of itself" is a big no-no. The very reason the React team is experimenting with context selectors (and had the option for calculateChangedBits()) is because they know this is a problem, so the whole "I don't worry about unnecessary rendering because I trust the React team to optimize my code for me" rings a bit naïve. But don't believe me, there is a ton of literature out there on this. See, e.g. How to Destroy Your App Performance Using React Contexts

Refs shouldn't be assigned during render

I didn't.

Re: PR: I might get around to one at some point. I do love the concept of react-router, but the heavy usage of frequently changing Contexts at such a fundamental level is a sandy foundation to build upon, and thus a complete non-starter for me, so I've already rewritten the library for my own purposes to cut out most of the major inefficiencies. Now I'm just relying on the history package and the matchRoutes and resolvePath methods. (Unfortunately my version is now completely incompatible with yours hence why I haven't already submitted a PR... but maybe later I can think of a way to port some of my modifications into compatible PRs).

[LuisDev99]

@ryanflorence My issue: Whenever the component mounts, I make a HTTP request. Because of the double render issue, the component is making two HTTP requests, and for my particular case, we are logging every HTTP request made to our service, but since the component is making an extra request, we are logging two events when there should've been just one.

That's why the double render in my use case is a big problem. Surely we can implement some sort of caching to prevent additional requests, but I'm just trying to give one real example.

[deleteme]

@ryanflorence This isn't just an optimization problem. Another problem that caused user-facing bugs while upgrading from v5 to v6, was that a navigate call within a useEffect hook would incur extra unexpected calls.

Here's the scenario:

We have a field with an onComplete handler that, when triggered, navigates to a new route.

1. A custom input component receives an onComplete prop.
2. The input value may be set and change from multiple different ways, so some special logic isn't suitable across various event listeners and instead centralized in a reducer.
3. A useEffect hook receives onComplete and the input state as dependencies. When the hook determines the input state value to be complete, onComplete is called.

useEffect(() => {
  const isComplete = /* something with the state */
  if (isComplete) {
    onComplete(state);
  }
}, [state, onComplete]);

4. This custom input component is mounted within a page that has a couple routes and also uses search params. The component will remain mounted throughout all of these routes.
5. onComplete is memoized with useCallback, and calls navigate with one of those route urls.
6. Bug: When the input is complete, the location changes, then navigate is redefined, so onComplete is redefined, and because it's a new function, triggers the useEffect onComplete callback an extra time.

A work-around was to decouple the onComplete callback from that effect, then dispatch and listen to a custom event:

  // announce that the input is complete via a custom event
  useEffect(() => {
    const element = elementRef.current;
    if (isComplete && element) {
      element.dispatchEvent(
        new CustomEvent(ONCOMPLETE, {
          bubbles: true,
          detail: { template, valueString },
        })
      );
    }
  }, [template, valueString, isComplete, elementRef]);

  // call an onComplete callback
  useEffect(() => {
    const element = elementRef.current;
    if (onComplete && element) {
      const handler = (e) => onComplete(e.detail);
      element.addEventListener(ONCOMPLETE, handler);
      return () => {
        element.removeEventListener(ONCOMPLETE, handler);
      };
    }
  }, [onComplete, elementRef]);

[RandScullard]

@ryanflorence I agree with @deleteme, I am really more concerned with effects being triggered. Additional renders don't create a problem in my use case.

I still think the effect problem could be solved as in https://github.com/remix-run/react-router/issues/7634#issuecomment-1006997643. But good point about setting the ref in an effect, not inline in the render. (Full disclosure: I have not tested this.)

[Brammm]

Just chiming in, came to this issue after running into a race condition. I was optimistically updating state and navigating away from the page to an updated page slug, but because navigate isn't memoized, my app would load it's data again, which wasn't updated yet in the db.

[Lure5134]

What do you think about a hook like useAbsoluteNavigate() or so. The hook would be just like useNavigate() but would not support relative paths. Then you wouldn't need useLocation in the hook and would save some rerenderer.

[Lure5134]

Or can we just use window.location.pathname instead of useLocation().pathname?

[flexdinesh]

To anyone who's looking for an interim solution to prevent re-rendering of components using useNavigate() and useLocation() I've got something I've been using for a while now.

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

type RouterUtilsContextType = {
  navigateRef: React.MutableRefObject<NavigateFunction> | null;
  locationRef: React.MutableRefObject<Location> | null;
};
const RouterUtilsContext = React.createContext<RouterUtilsContextType>({
  navigateRef: null,
  locationRef: null,
});

/*
  react-router uses one big context to send changes down the react tree.
  So every route or query param change will re-render the context which will in-turn re-render 
  all the hooks subscribed to react-router context - useNavigate(), useLocation().
  This prevents us from using these hooks as utilities to get latest location or query param value 
  in a component since all the components using these hooks will re-render in addition to the 
  entire Route component re-rendering - which is not ideal.

  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(navigate);
  const locationRef = React.useRef(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>;
};

/* 
  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 = useNavigateNoUpdates();
*/
export const useNavigateNoUpdates = () => {
  const {navigateRef} = React.useContext(RouterUtilsContext);
  if (navigateRef === null) {
    throw new Error(
      'RouterUtils context should be added to the React tree right below BrowserRouter for useNavigateNoUpdates hook to work. If you need router in tests or stories, please use WrappedMemoryRouter utility.',
    );
  }
  return navigateRef.current;
};

/* 
  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 = useLocationNoUpdates();
*/
export const useLocationNoUpdates = () => {
  const {locationRef} = React.useContext(RouterUtilsContext);
  if (locationRef === null) {
    throw new Error(
      'RouterUtils context should be added to the React tree right below BrowserRouter for useLocationNoUpdates hook to work. If you need router in tests or stories, please use WrappedMemoryRouter utility.',
    );
  }
  return locationRef.current;
};

/* 
  Please be aware: when the query params change - this hook will NOT re-render. 
  Only use it as a utility to get latest query params value.
  Eg. const params = useQueryParamsNoUpdates();
  const sidebarGoalId = params['sidebar_goal_id'];
*/
export const useQueryParamsNoUpdates = () => {
  const {search} = useLocationNoUpdates();

  const queryParams = React.useMemo(() => {
    const urlSearchParams = new URLSearchParams(search);
    const params = Object.fromEntries(urlSearchParams.entries());
    return params;
  }, [search]);

  return queryParams;
};

/* 
  Please be aware: when the query param changes - this hook will NOT re-render. 
  Only use it as a utility to get latest query param value.
  Eg. const sidebarGoalId = useQueryParamNoUpdates('sidebar_goal_id');
*/
export const useQueryParamNoUpdates = (name: QueryParams) => {
  const params = useQueryParamsNoUpdates();

  if (!name) {
    throw new Error(`useQueryParam name arg cannot be empty — name: ${name}`);
  }

  return params[name];
};

export default RouterUtils;

And you add this Provider right below the Router provider in the component tree

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

[houmark]

@ryanflorence If the goal was to slow down user adoption of v6 and have people look for alternative routers, then that goal has certainly been reached with a breaking approach like this one.

The documentation on migration from v5 to v6 in relation to push and navigate goes like this:

In v6, this app should be rewritten to use the navigate API. Most of the time this means changing useHistory to useNavigate and changing the history.push or history.replace callsite.

Ref: https://reactrouter.com/docs/en/v6/upgrading/v5#use-usenavigate-instead-of-usehistory

Nowhere is it mentioned that the fingerprint of the useNavigate hook is changing on every function call. This leads to hard-to-fix unexpected bugs for many solutions in the wild.

In our case, and in many other cases I've read here and elsewhere, a useEffect does navigation to the same component. That's not a wrong pattern or use case, but there is no simple alternative or solution to solving that. Sure, we can start wrapping the "hack" you drafted above, but that is not what you'd expect when migrating from v5 to v6 — and if there are major difference you'd expect it would be highlighted and reasonable workarounds would be exampled. We have had a fairly simple migration up until we realized that we had several cases where this breaks the entire component navigation and that there was no simple way to fix it.

We can now either spend hours on doing some hacky workarounds, rethink our entire component approach (hard without major refactor) or stash our migration and once again put the v6 migration on the backburner. Unfortunately, React 18 seems to have issues with StrictMode with React Router v5 and while the rest of our app is ready to upgrade to React 18, upgrading to v6 together with React 18 made the most sense.

I echo other simple solutions above where useNavigate internally can keep track of the relative path, but it avoids changing on navigate. I understand it may not be "cleaner" to do it that way (or how React wants you to), but it's certainly a much friendlier backward compatibility approach that will have fewer users pulling their hair.

[houmark]

The solution outlined by @RandScullard in https://github.com/remix-run/react-router/issues/7634#issuecomment-1006997643 seems like a very simple and low-risk change, that will keep current functionality while also introducing better backward compatibility without breaking any existing adoptions to navigate — and I think using useRef is also an acceptable React approach for this use case.

There is literally no upside to the function changes that I can identify and also no negative side effects from the hook not returning the locationPathname, as one can fetch that using the useLocation() hook anyways.

Considering that, would you accept a PR that does that change @ryanflorence with the purpose of easing adoption? There should not be a lot of work involved in doing that change, but I would still like to wager your interest in such a change before working on a PR.

[chaance]

This conversation is getting a bit heated, so let's try to bring it down a notch. Please remember this is free software maintained by people who are trying to help in good faith.

Ryan stated above that he'd consider a PR that makes a change as long as it's safe and follow all the aforementioned rules, so if this is a priority for you please let us know when that PR is ready and we'll take a look as soon as we can. In the mean time, there's always patch-package!