use-query-params icon indicating copy to clipboard operation
use-query-params copied to clipboard

useQueryParam forces re-render on every param change in url, even if required value did not change in url

Open kushmisra opened this issue 3 years ago • 8 comments

Hi,

I am using useQueryParam in the following manner :

import React, { useState } from 'react';
import {Route, Switch} from 'react-router-dom';
import {ConnectedRouter} from 'connected-react-router';
import {QueryParamProvider, NumberParam, useQueryParam} from 'use-query-params';

const PrintingY = React.memo(() => {
    const [y, setY] = useQueryParam('y', NumberParam);
    return (
        <div >
            Click here to increment y. current y {0}
        </div>
    )
}, () => true);

const PrintingX = () => {
    const [x, setX] = useQueryParam('x', NumberParam);
    return (
        <div onClick={() => setX((x || 0)+1)}>
            Click here to increment x. current x {x}
        </div>
    )
}

const DummyComponent = () => (
    <div>
        <PrintingX />
        <PrintingY />
    </div>
)

/**
 * Top-level Component
 */
const MainComponent = ({history}) => (
    <ConnectedRouter history={history}>
        <QueryParamProvider ReactRouterRoute={Route}>
            <Switch>
                <Route path="/" component={DummyComponent} />
            </Switch>
        </QueryParamProvider>
    </ConnectedRouter>
);

There are two query params I am interested in : 'x' and 'y'. There are two components 'PrintingX' and 'PrintingY' which are individually interested in 'x' and y' respectively. Clicking on PrintingX will change the value of 'x'.

The problem is that whenever I am clicking on PrintingX, url is updated and all the components reload. What I expect is that only those components should reload which are actually interested in changed URL. In the above case, clicking on PrintingX, re-renders PrintingY as well. PrintingY is not at all interested in 'x' and is only concerned with 'y'. In short, whenever URL changes, I see that every useQueryParam forces re-render. I have checked that it's returning the same value for the param, which is correct, as the param hasn't changed, but it's re-rendering the component. Removing this line : const [y, setY] = useQueryParam('y', NumberParam); from PrintingY, stops re-rendering it.

Is this expected? I expect that useQueryParam will update the component only if the concerned value changed in the URL.

Any leads on what can be the issue?

Thanks in advance.

kushmisra avatar May 05 '21 15:05 kushmisra

Hi there, you're absolutely right about what's happening and it being undesirable. The way this hook works is that it uses React context that gets updated whenever the history changes (typically due to your router). Currently, this means that anything that cares about the URL at all will get updated whenever the URL changes. Once React releases context selectors we may be able to avoid this, but it will depend on how flexible they make that feature.

If we didn't use react context and instead used a different state system like zustand where we could subscribe to sub-sections of the URL, we'd incur a second render between when the route changed and when the URL params changed, which I think would break a lot of apps. I believe only React context lets us avoid this issue.

In most cases, the extra renders don't actually cause a performance issue. If they do, I would suggest separating the URL reading (useQueryParam call) into a parent component and then passing the value of the param as a prop to a React.memo'd child component. e.g.,


const PrintingY = () => {
  const [y, setY] = useQueryParam('y', NumberParam);
  return (
      <PrintingYInner y={y} setY={setY} />
  )
}

const PrintingYInner = React.memo(({ y, setY }) => {
  return (
      <div>
          Click here to increment y. current y {0}
      </div>
  )
});

In this case, PrintingY will render, but the render is very cheap since it is a very simple component. Since the props to PrintingYInner didn't change, it will not re-render avoiding any potential complexity in it (assuming it was much more complex than this example).

To simplify this process, you can also switch to using the HOC withQueryParams (see Usage).

pbeshai avatar May 05 '21 16:05 pbeshai

Hi @pbeshai,

Thanks a lot for the prompt reply!

I hope React adds context selectors soon so that we can seamlessly use this library 😄 .

Thanks a lot for the workaround, I will use withQueryParams.

kushmisra avatar May 05 '21 17:05 kushmisra

Hey,

Yes I could make it work. I used a HOC and then I used shouldComponentUpdate in my original component. So I am not actually dependent on withQueryParams to not re-render my component. Even if it does trigger a re-render, my shouldComponentUpdate will reject it.

I feel withQueryParams alone should have solved the issue and using shouldCompoenentUpdate should not be necessary, but I did not check it.

On Wed, 6 Oct 2021 at 6:39 PM, carolirod @.***> wrote:

@kushmisra https://github.com/kushmisra did it help you, using withQueryParams? Cause I replace it for that but it keeps re-rendering on every param change.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/pbeshai/use-query-params/issues/160#issuecomment-936207749, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGBCAM7FNOMR24NZXF63LH3UFRDCDANCNFSM44FHZTOQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

kushmisra avatar Oct 06 '21 19:10 kushmisra

Is it me, or even without useQueryParam the whole component tree from the base router re renders each time a query string changes? Is that expected?

cristianoccazinsp avatar May 27 '22 02:05 cristianoccazinsp

When migrating from v5, I actually made a wrapper like this so I didn't need to re-write the entire code (which are class based "pages" that expect all the route/navigation params).

const RouteWrapper = ({ component: Component, render }) => {
  const navigate = useNavigate();
  const location = useLocation();
  const match = {
    params: useParams(),
    type: useNavigationType(),
  };

  // if a render fun was given, use it
  if (render) {
    return render({ match, navigate, location });
  }
  // otherwise, use the given component.
  return <Component match={match} location={location} navigate={navigate} />;
};

With this code, changing any query parameter will cause the app to re-render the entire tree from the very first <BrowserRouter>.

Any way to prevent this and have the same behaviour as router v5, where changing query-strings wouldn't reload the entire browser router stack?

cristianoccazinsp avatar May 27 '22 15:05 cristianoccazinsp

Hi there, you're absolutely right about what's happening and it being undesirable. The way this hook works is that it uses React context that gets updated whenever the history changes (typically due to your router). Currently, this means that anything that cares about the URL at all will get updated whenever the URL changes. Once React releases context selectors we may be able to avoid this, but it will depend on how flexible they make that feature.

If we didn't use react context and instead used a different state system like zustand where we could subscribe to sub-sections of the URL, we'd incur a second render between when the route changed and when the URL params changed, which I think would break a lot of apps. I believe only React context lets us avoid this issue.

In most cases, the extra renders don't actually cause a performance issue. If they do, I would suggest separating the URL reading (useQueryParam call) into a parent component and then passing the value of the param as a prop to a React.memo'd child component. e.g.,

const PrintingY = () => {
  const [y, setY] = useQueryParam('y', NumberParam);
  return (
      <PrintingYInner y={y} setY={setY} />
  )
}

const PrintingYInner = React.memo(({ y, setY }) => {
  return (
      <div>
          Click here to increment y. current y {0}
      </div>
  )
});

In this case, PrintingY will render, but the render is very cheap since it is a very simple component. Since the props to PrintingYInner didn't change, it will not re-render avoiding any potential complexity in it (assuming it was much more complex than this example).

To simplify this process, you can also switch to using the HOC withQueryParams (see Usage).

Hi there, you're absolutely right about what's happening and it being undesirable. The way this hook works is that it uses React context that gets updated whenever the history changes (typically due to your router). Currently, this means that anything that cares about the URL at all will get updated whenever the URL changes. Once React releases context selectors we may be able to avoid this, but it will depend on how flexible they make that feature.

If we didn't use react context and instead used a different state system like zustand where we could subscribe to sub-sections of the URL, we'd incur a second render between when the route changed and when the URL params changed, which I think would break a lot of apps. I believe only React context lets us avoid this issue.

In most cases, the extra renders don't actually cause a performance issue. If they do, I would suggest separating the URL reading (useQueryParam call) into a parent component and then passing the value of the param as a prop to a React.memo'd child component. e.g.,

const PrintingY = () => {
  const [y, setY] = useQueryParam('y', NumberParam);
  return (
      <PrintingYInner y={y} setY={setY} />
  )
}

const PrintingYInner = React.memo(({ y, setY }) => {
  return (
      <div>
          Click here to increment y. current y {0}
      </div>
  )
});

In this case, PrintingY will render, but the render is very cheap since it is a very simple component. Since the props to PrintingYInner didn't change, it will not re-render avoiding any potential complexity in it (assuming it was much more complex than this example).

To simplify this process, you can also switch to using the HOC withQueryParams (see Usage).

cant we use , useMemo to avoid rerenders , just wrap each section of our component into useMemo or useCallBack so it wont be rerendered if we changed something like params in url? is it a good solution?

DOTL-Amir-R avatar Sep 28 '23 10:09 DOTL-Amir-R

So I've run into a similar issue using the route params in react-router, and the issue ultimately seems to be that the hook is obviously always called, and because of that it's constantly triggering downstream re-renders.

I've written a higher-order-component that solves my particular problem, but I wonder if it'll be of any use here too?

It might be the wrong approach / toral overkill, but here it is:

import { useRef } from "react";
import { Params, useParams } from "react-router-dom";
import isEqual from "react-fast-compare";
import React from "react";

type ParamsRes<T> = Readonly<[T] extends [string] ? Params<T> : Partial<T>>;

const withMemoizedRouteParams =
  <T extends {}, PT extends string = string, P = any>(
    Component: React.FC<T>,
    selector: (params: ParamsRes<PT>) => P
  ) =>
  (props: Pick<T, Exclude<keyof T, keyof P>>) => {
    const p = useParams<PT>();
    const selected = selector(p);
    const stored = useRef<P>(selected);
    const equal = isEqual(selected, stored.current);
    // The soft cast here is unfortunate, but cam't get the types lined up
    const newProps: T = { ...(props as T), ...stored.current };
    const MemoComponent = React.memo(
      Component,
      (p, n) => equal && isEqual(p, n)
    );
    if (!equal) {
      stored.current = selected;
    }
    return <MemoComponent {...newProps} />;
  };

export default withMemoizedRouteParams;

A lot of the types are just about the variability in the type that useParams might return, but the general idea is:

  1. Call the hook and get the data
  2. Map that object and produce the subset we want selector
  3. Using a ref, we store that subset object the first time round
  4. Using react-fast-compare isEqual work out whether anything has actually changed
  5. Build the new props object we're going to forward onto the component - this works by assuming our component will accept props that will contain the selected data
  6. Memoize the Component provided, and override the prop equality check, short circuit if we already know the params have changed, otherwise check whether any of the other props have changed
  7. If our quality check this time round failed on the route params, then update the version we have in the ref for next time
  8. Render out the memoized component.

Using it looks like this:

const MyContactComponent = (props: {
  contactId: string;
  someOtherProp: number;
}) => (
  <>
    <p>
      I am the contact component for ID: {props.contactId} and the number:{" "}
      {props.someOtherProp}
    </p>
  </>
);

const MyRoutedContactComponent = withMemoizedRouteParams(
  MyContactComponent,
  (p) => ({ contactId: p.contactId })
);

const MyOtherComponent = () => {
  return (
    <>
      <MyRoutedContactComponent someOtherProp={12} />
    </>
  );
};

There's some weirdness with the types which I think is mostly just me not being super knowledgeable of the type system,, but this does complain if props are missing that aren't the ones selected from the hook value, so I'm calling that a win.

When mounting these components in a basic nested route system, I can see that components now don't render unless the route params they're selecting or if other prop values change, the only one re-rendering is the higher order component.

You might be able to adapt something like this to support useQueryParams()

ckpearson avatar Nov 17 '23 02:11 ckpearson

Hey, we just hit this issue while trying to type and have the URL update; the rerender is causing inputs to lose focus, modals, and submenus to close, etc., breaking the UX. Is there any recommendation on keeping Input components focused while updating the query params with this hook?

bombillazo avatar Feb 23 '24 14:02 bombillazo