47ng / nuqs

Type-safe search params state manager for Next.js - Like React.useState, but stored in the URL query string.
https://nuqs.47ng.com
MIT License
3.17k stars 64 forks source link

Navigating between pages rerenders source page #524

Open mateogianolio opened 6 months ago

mateogianolio commented 6 months ago

Context

What's your version of nuqs?

^1.17.1

Next.js information (obtained by running next info):

Operating System:
  Platform: darwin
  Arch: arm64
  Version: Darwin Kernel Version 23.3.0: Wed Dec 20 21:31:00 PST 2023; root:xnu-10002.81.5~7/RELEASE_ARM64_T6020
Binaries:
  Node: 18.18.2
  npm: 9.8.1
  Yarn: 1.22.19
  pnpm: N/A
Relevant Packages:
  next: 14.1.3
  eslint-config-next: 14.1.3
  react: 18.2.0
  react-dom: 18.2.0
  typescript: 5.4.2
Next.js Config:
  output: N/A

Are you using:

Description

Navigating between pages renders both the source page and the target page. I would expect only the target page to render.

Reproduction

// pages/a.tsx
export default function A() {
  const [page] = useQueryState("page");
  console.info("[page a]", page);

  return <Link href="/b?page=b">b</Link>;
}

// pages/b.tsx
export default function B() {
  const [page] = useQueryState("page");
  console.info("[page b]", page);

  return <Link href="/a?page=a">a</Link>;
}

Navigate to /a.

[page a] null

Click on the link to b.

...
[page a] b
[page b] b

Click on the link to a.

...
[page b] a
[page a] a

As you can see both source and target pages are being rendered when I would expect only the target page to render.

If I replace useQueryState with useRouter it behaves as expected. Either I have misunderstood something about nuqs or this is a bug.

franky47 commented 6 months ago

The first render on page a is from the URL sync mechanism. For some reason, the pages router won't unmount the source page until after navigation has finished (possibly for transition purposes), and when nuqs picks up a URL change, it sends the info to all mounted hooks, which includes the previous page.

For the record, the behaviour in the app router is even weirder: the source page behaves as expected, but the target page mounts and renders first with no search params (as it was before clicking the link), then navigation occurs, and the state is updated with the correct values from the URL.

You can see all of this by enabling logging.

mateogianolio commented 6 months ago

Ah, that is annoying. Would it not be possible to somehow attach the pathname to the hooks (on mount) and then always compare the current pathname with the hook pathname before propagating changes? At least for the pages router. The app router behaviour seems more difficult to address.

mateogianolio commented 6 months ago

This custom hook solves the problem for me:

export default function useSafeQueryState(key, options?) {
  const [state, setState] = useQueryState(key, options);
  const [safeState, setSafeState] = useState(state);
  const [pathname, setPathname] = useState<string>();
  const router = useRouter();

  useEffect(() => {
    // Set pathname on mount
    setPathname(router.pathname);
  }, []);

  useEffect(() => {
    function onRouteChangeStart(url) {
      // Change pathname when starting to navigate to a different page
      setPathname(url.split('?')[0]);
    }

    router.events.on('routeChangeStart', onRouteChangeStart);

    return () => {
      router.events.off('routeChangeStart', onRouteChangeStart);
    };
  }, [pathname, router.events]);

  useEffect(() => {
    if (
      pathname !== router.pathname ||
      state === safeState ||
      JSON.stringify(state) === JSON.stringify(safeState)
    ) {
      return;
    }

    setSafeState(state);
  }, [pathname, router.pathname, safeState, state]);

  return [safeState, setState];
}

EDIT: Accidentally reversed equality check in useEffect EDIT 2: Fixed setting of pathname after testing

varayoudfederico commented 4 months ago

@mateogianolio Thanks for this snippet. We were having a similar issue where every state on our website that used useQueryState() rendered one more time with the default value before a page transition, resulting in a UI flash most noticeable on slower devices.

I hope something like this is fixed or added on a future release of the library.

hamzaboularbah commented 4 months ago

⚠️ : Im using next 13.5.6 where asPath is actually what we want to use instead of pathname, so the hook became like this:

EDIT: this is valid when you have dynamic routes: see comment below

export default function useSafeQueryStates(
  key,
  options?
) {
  const [state, setState] = useQueryStates(key, options)
  const [safeState, setSafeState] = useState(state)
  const [pathname, setPathname] = useState<string>()
  const router = useRouter()

  const routerPathname = router.asPath.split('?')[0]

  useEffect(() => {
    // Set pathname on mount
    setPathname(routerPathname)
    // eslint-disable-next-line react-hooks/exhaustive-deps
  }, [])

  useEffect(() => {
    function onRouteChangeStart(url: string) {
      console.log('onRouteChangeStart', { url })
      // Change pathname when starting to navigate to a different page
      setPathname(url.split('?')[0])
    }

    router.events.on('routeChangeStart', onRouteChangeStart)

    return () => {
      router.events.off('routeChangeStart', onRouteChangeStart)
    }
  }, [pathname, router.events])

  useEffect(() => {
    if (pathname !== routerPathname) {
      return
    }

    setSafeState(state)
  }, [pathname, routerPathname, safeState, state])

  return [safeState, setState]
}
jmnunezizu commented 4 months ago

@hamzaboularbah interesting. I'm on 13.5.6 and I didn't have to change anything and it worked fine. What were you experiencing? Maybe I missed it. Thanks.

hamzaboularbah commented 4 months ago

@jmnunezizu Oh my bad, the issue arises when you have dynamic routes.

Say for example you have a page /something/123/list with 123 as a dynamic id (let's call it somethingId), in this case when you call the hook in that route, the url from onRouteChangeStart will be /something/123/list which means that the pathname state will also be that. But the router.pathname will be /something/[somethingId]/list.

So when you run pathname !== router.pathname will always be true and you will not update the state, hence using router.asPath that contains the actual values similar the the url string.

I mean in your case if you don't have a dynamic route with this hook, it should be fine since router.pathname and router.asPath will be the same.

jeremy-habit commented 3 months ago

@mateogianolio Thanks for this snippet. We were having a similar issue where every state on our website that used useQueryState() rendered one more time with the default value before a page transition, resulting in a UI flash most noticeable on slower devices.

I hope something like this is fixed or added on a future release of the library.

Hello ! do we have more information about it pls ? @franky47

franky47 commented 3 months ago

For the reason why this happens, see this explanation by Tim Neutkens: https://x.com/timneutkens/status/1795809805774250336

Now for solutions in nuqs, I'd have to dive deeper into the issue, but I don't have a lot of free time at the moment.