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

Add a ready: boolean to Router returned by useRouter #8259

Closed
jtomaszewski opened this issue Aug 6, 2019 · 53 comments · Fixed by #20628
Closed

Add a ready: boolean to Router returned by useRouter #8259

jtomaszewski opened this issue Aug 6, 2019 · 53 comments · Fixed by #20628
Assignees
Milestone

Comments

@jtomaszewski
Copy link
Contributor

Because of the way Next.js does initial render of the pages, and that the initial render on client-side and server-side has to be the same (see https://reactjs.org/docs/react-dom.html#hydrate ), sometimes when a page is rendered, its' query (returned i.e. by useRouter() hook) will be empty, even though it actually has query params. After the initial render, soon after there will be another render in which the query will be then populated with proper query params. ( Example described also in i.e. #7880 )

My code relies on those query params very heavily and we need to know "are those the real current query params or not yet?". (i.e. because if query params are missing or incorrect, we'd like to redirect the user to another page)

Describe the solution you'd like

I think that either:

  • query should be always populated with the current query params (even on initial client-side hydration that happens after a server-side render)
  • or, (IMO better), add a ready: boolean flag to Router, so the component can listen to this using i.e. useEffect and ignore the query params when they aren't parsed yet.

Describe alternatives you've considered

I think we'll have to go with some setTimeout or sth for now until a better solution is found.

@lfades
Copy link
Member

lfades commented Aug 6, 2019

query should be available in useEffect because it's only used in the browser, your page is most likely being prerendered and therefore the query params are empty until the browser does the render.

You can either keep all the changes that depend in the query params inside useState or add getInitialProps if you need to have the query params always defined, in exchange of not having a statically exported page

@jtomaszewski
Copy link
Contributor Author

jtomaszewski commented Aug 6, 2019

You can either keep all the changes that depend in the query params inside useState or add getInitialProps if you need to have the query params always defined, in exchange of not having a statically exported page

We can't use getInitialProps - those query params aren't known at the time of server-side render. Also, we want that page to be exported (we actually explicitly call next export, because that's the way our app has to be built like).

query should be available in useEffect because it's only used in the browser, your page is most likely being prerendered and therefore the query params are empty until the browser does the render.

I wish that was true... but it isn't. Besides useEffect there also needs to be a setTimeout. Only after that the route params are finally there. (If you do only useEffect, then params might still be empty at that moment.)

For now we have a workaround that looks like the following, but ideally, I'd prefer next/router to provide that out-of-the-box:

import NextRouter, {
  useRouter as useNextRouter,
  NextRouter,
} from 'next/router';

const RouterContext = React.createContext<NextRouter & { ready: boolean }>(
  null as any,
);
export function RouterContextProvider({
  children,
}: {
  children?: React.ReactNode;
}) {
  const router = useNextRouter();

  // In order to know if the params that are passed by router
  // are actually correct
  // (because sometimes on initial render they might be equal to server-side params,
  //  not the ones that are currently visible in the browser),
  // we'll be relying on this `ready: boolean` property.
  // ( Workaround until next.js give us a proper solution for it
  //  - see https://github.com/zeit/next.js/issues/8259 )
  const [ready, setReady] = useState(false);
  useEffect(() => {
    const timeout = setTimeout(() => {
      setReady(true);
    });
    return () => clearTimeout(timeout);
  }, []);

  const wrappedRouter = useMemo(
    () => ({
      ...router,
      ready,
    }),
    [router, ready],
  );

  return (
    <RouterContext.Provider value={wrappedRouter}>
      {children}
    </RouterContext.Provider>
  );
}

export function useRouter() {
  return useContext(RouterContext);
}

@Timer
Copy link
Member

Timer commented Aug 6, 2019

Sorry, but you cannot branch rendering logic based on booleans -- this causes hydration mismatches and typically indicates a code pattern that needs fixed.

My code relies on those query params very heavily and we need to know "are those the real current query params or not yet?"

You can rely directly on location.search within useEffect if you cannot tolerate Next.js informing you of new query parameters in the way it does.

Also, this setTimeout you're using trick provides no guarantees the updated query object is going to be to you.


If you provide a full example of your exact use case, we may be able to suggest a better React pattern.

@Timer Timer closed this as completed Aug 6, 2019
@jtomaszewski
Copy link
Contributor Author

jtomaszewski commented Aug 6, 2019

Sorry, but you cannot branch rendering logic based on booleans -- this causes hydration mismatches and typically indicates a code pattern that needs fixed.

Yeah, but RouterContext is updated once router.query is updated, right? So together with it, it might as well set the router.ready boolean on that context, after we know it was the client who updated the params.

I want to avoid branching rendering logic based on "am I on ssr?" booleans to make hydration work, thus raised this issue.

You can rely directly on location.search within useEffect if you cannot tolerate Next.js informing you of new query parameters in the way it does.

We have routes with dynamic parameters. Parsing location.search on our own would be non-trivial and a duplication of what Next router already does.

@jtomaszewski
Copy link
Contributor Author

Our example is like this:

  • We have a route like /rooms.
  • Our app is served using CDN, through next export
  • That /rooms accepts multiple query params like ?region=us&property=hotel. At least one of them needs to be present. If none of them are present or if they are invalid, we want to show the user an error (or redirect him to a 404 page).

So basically, we need to be able get the initial route params on/after the page loads, and then act on it (i.e. show an error to the user if the params are incorrect).

However, currently, without the above workaround, if we navigated to the /rooms?region=us:

  1. On page load, the route params are empty.
  2. After a certain moment (let's say the useEffect + setTimeout), the route params get populated with actual params that are visible at the client-side.

If we won't rely on sth like router.ready, then if we show error at the 1st moment, then it will be incorrect UI behaviour: first there would be an error visible ("your URL is wrong!"), and a subsequent render after that, the error would be hidden, because the component would then assume that URL is actually correct.

We'd like to avoid having weird workarounds like surrounding the "show error code" with setTimeout(..., 500). Instead, it would be perfect to know if the route params are actually the params that are visible on client-side, or if they are just the old version from server-side (because the client-side query params aren't known yet).

@iwarner
Copy link

iwarner commented Sep 9, 2019

Hiya I ended up putting the query into state:

  const [bankId, setBankId] = useState(null)
  const { query } = useRouter()

  useEffect(() => {
    setBankId(query.bankId)

    if (bankId) {
      const filesFiltered = filter(files, (o) => { return o.dir === bankId })
      setFiles(filesFiltered)
      setIsLoading(false)
    }
  }, [query, bankId])

This cause multiples render cycles, but I do not see any other way of getting the router params.

I am using a Static build, so do not want to use getInitial... be great if examples could showcase static methods also to state best practices.

@defx
Copy link

defx commented Oct 1, 2019

+1 for better documentation around static patterns.

@bmvantunes
Copy link
Contributor

bmvantunes commented Oct 22, 2019

I was having exactly the same problem - I solved it the following way:

@jtomaszewski you can have something like this:

import { NextRouter } from 'next/router';

export function isRouterReady(router: NextRouter) {
  return router.asPath !== router.route;
}

If you are using pre-rendered pages, immediately after hydration the router.route property and router.asPath property will have exactly the same value.

In your case, in the first render you'll have:

{
  asPath: "/rooms"
  router: "/rooms"
}

In the second render you'll have something like:

{
  asPath: "/rooms?myProperty=something"
  router: "/rooms"
}

So in your component you can do:

export function MyComponent() {
  const router = useRouter();

  useEffect(() => {
    if (isRouterReady(router)) {
      // here you have your query params
    }
  }, [router]);
}

@Timer can you confirm if this is a valid approach?

@Timer
Copy link
Member

Timer commented Oct 22, 2019

Upon further thought, I don't see the harm in supporting Router.ready. Feel free to send a PR!

@Timer Timer reopened this Oct 22, 2019
@Timer Timer added this to the backlog milestone Oct 22, 2019
@Timer Timer added good first issue Easy to fix issues, good for newcomers help wanted labels Oct 22, 2019
@Soundvessel
Copy link

I ran into a similar issue where I am using the useRouter hook inside a custom hook used for several components to update query params (table column sorting, filter UI, all param based). The first component of four would always have a stale query value reported at the console.log. This occurred even after several interactions with the other components.

import { useCallback } from 'react'
import { useRouter } from 'next/router'
import { Router } from '../routes'
import { getRouteNameFromAsPath } from '../lib/utils'
export default function usePushQueryParams() {
  const { asPath, query } = useRouter()
  const pushQueryParams = useCallback(
    function pushQueryParamsUpdate(newQueryParams) {
      console.log('useRouter Query', query)
      const routeName = getRouteNameFromAsPath(asPath)
      const newQuery = {
        ...query,
        ...newQueryParams,
      }
      Router.pushRoute(routeName, newQuery)
    },
    [asPath, query]
  )
  return pushQueryParams
}

It is rather confusing in these instances, where hooks should be compostable in custom hooks, that useRouter doesn't reliably return the current query. Perhaps an update to the documentation can at least provide others a sanity check?

@lfades
Copy link
Member

lfades commented Oct 31, 2019

@Soundvessel In the case of prerendered pages (pages without getInitialProps), the query object will be empty in the first render, that will get populated after client-side hydration. A place that always has access to a populated query object is useEffect, so try accessing to it from there.

I agree that docs don't currently do a good job explaining it, but it will get fixed soon 👍

@Soundvessel
Copy link

Soundvessel commented Oct 31, 2019

Ah and I think a ref is a good way to set something in that useEffect?

Here is my now working updated hook.

import { useCallback, useEffect, useRef } from 'react'
import { useRouter } from 'next/router'
import { Router } from '../routes'
import { getRouteNameFromAsPath } from '../lib/utils'

export default function usePushQueryParams() {
  const { asPath, query } = useRouter()

  const queryRef = useRef(null)

  useEffect(() => {
    queryRef.current = query
  }, [query])

  const pushQueryParams = useCallback(
    function pushQueryParamsUpdate(newQueryParams) {
      const routeName = getRouteNameFromAsPath(asPath)
      const newQuery = {
        ...queryRef.current,
        ...newQueryParams,
      }

      Router.pushRoute(routeName, newQuery)
    },
    [asPath]
  )

  return pushQueryParams
}

@Soundvessel
Copy link

What is rather weird is that I made a custom hook since I needed to use the query in several other components. For some reason the above works but when I switch to it using this hook which uses the same code I get the stale query again. Looks like the useEffect needs to exist where you are using the query and can't be from a custom hook?

export default function useUrlQuery() {
  const { query } = useRouter()

  const queryRef = useRef(query)

  useEffect(
    function setHydratedQuery() {
      queryRef.current = query
    },
    [query]
  )

  return queryRef.current

@julian-sf
Copy link

I was having exactly the same problem - I solved it the following way:

@jtomaszewski you can have something like this:

import { NextRouter } from 'next/router';

export function isRouterReady(router: NextRouter) {
  return router.asPath !== router.route;
}

If you are using pre-rendered pages, immediately after hydration the router.route property and router.asPath property will have exactly the same value.

In your case, in the first render you'll have:

{
  asPath: "/rooms"
  router: "/rooms"
}

In the second render you'll have something like:

{
  asPath: "/rooms?myProperty=something"
  router: "/rooms"
}

@Timer can you confirm if this is a valid approach?

This only works if you expect query params to be populated. IE, if there were NO query params to be added, then this would never return true, which is a bug.

I think the only clean way to "know" at least from client code, is to have the query params in the useEffect hook. I'm going to look through the next code and see if there is a clean way to decipher this "ready" state. It is essentially tied to whether or not you are "post" hydration, aka ready === haveIHydratedYet.

julian-sf pushed a commit to julian-sf/next.js that referenced this issue Nov 10, 2019
@julian-sf julian-sf mentioned this issue Nov 10, 2019
2 tasks
@julian-sf
Copy link

julian-sf commented Nov 10, 2019

Made a PR that adds a new parameter called isReady here: #9370

const { query, isReady } = useRouter();

where isReady is false until query has been populated.

I HAD to use something that wasn't ready as ready was already defined on the router singleton here:

type SingletonRouterBase = {
  router: Router | null
  readyCallbacks: Array<() => any>
  ready(cb: () => any): void
}

After speaking with the maintainers, we decided on isReady to match with the pattern of isFallback.
Please see the PR for more detailed reasoning...

julian-sf pushed a commit to julian-sf/next.js that referenced this issue Nov 11, 2019
@sidujjain
Copy link

sidujjain commented Dec 17, 2019

I was having exactly the same problem - I solved it the following way:
@jtomaszewski you can have something like this:

import { NextRouter } from 'next/router';

export function isRouterReady(router: NextRouter) {
  return router.asPath !== router.route;
}

If you are using pre-rendered pages, immediately after hydration the router.route property and router.asPath property will have exactly the same value.
In your case, in the first render you'll have:

{
  asPath: "/rooms"
  router: "/rooms"
}

In the second render you'll have something like:

{
  asPath: "/rooms?myProperty=something"
  router: "/rooms"
}

@Timer can you confirm if this is a valid approach?

This only works if you expect query params to be populated. IE, if there were NO query params to be added, then this would never return true, which is a bug.

I think the only clean way to "know" at least from client code, is to have the query params in the useEffect hook. I'm going to look through the next code and see if there is a clean way to decipher this "ready" state. It is essentially tied to whether or not you are "post" hydration, aka ready === haveIHydratedYet.

I'm seeing multiple rendering issue only with dynamic routes. This seems to work:

import { isDynamicRoute } from "next/dist/next-server/lib/router/utils/is-dynamic";

export function isRouterReady(router) {
  return !isDynamicRoute(router.route) || router.asPath !== router.route;
}

@wilsonpage
Copy link

wilsonpage commented Jan 8, 2020

Running into this issue also. From my ignorant next internals perspective it seems strange that router.asPath can't mirror window.location on first render for static pages. router.ready would be an acceptable workaround, but it's still a gotcha that nextjs user's will need to be aware of.

I'm using similar to @bmvantunes's approach for now:

const isDynamicPage = (router) => /\[.+\]/.test(router.route);
const testRouterReady = (router) => !isDynamicPage(router) || router.asPath !== router.route;

@urlund

This comment has been minimized.

@mycolaos
Copy link

Facing the same problem I tried to use getServerSideProps to read the query in SSR. It automatically makes the useRouter have the correct query from the very first render, i.e. there's no empty {} query at the start.

  1. Is it consistent behavior? I didn't find any docs on that.
  2. Can it be used as the solution to the problem?
  3. Why SSR just doesn't always pass the router state to the client?

@hexetia
Copy link

hexetia commented Dec 12, 2020

I have faced the same problem and published a package with a workaround

It's a tiny package that wrap the next.js router to provide the router.query and router.pathname on client-side first render

https://www.npmjs.com/package/use-client-router

@timneutkens
Copy link
Member

I have faced the same problem and published a package with a workaround

It's a tiny package that wrap the next.js router to provide the router.query and router.pathname on client-side first render

npmjs.com/package/use-client-router

This will very likely cause hydration mismatches causing React to throw away the pre-rendered HTML causing performance issues and jumps/issues with the pre-rendered HTML.

@Timer Timer added point: 2 and removed point: 5 labels Dec 30, 2020
@Timer Timer added point: 3 and removed point: 2 labels Dec 31, 2020
@kodiakhq kodiakhq bot closed this as completed in #20628 Dec 31, 2020
kodiakhq bot pushed a commit that referenced this issue Dec 31, 2020
Adds an `isReady` field on `next/router` specifying whether the router fields are updated client-side and ready for use. Should only be used inside of `useEffect` methods and not for conditionally rendering on the server.

Closes: #8259
Closes: #9370
@AlexandraKlein
Copy link

My query params are still not showing up in production when router.isReady is true. I'm only receiving a value for router.query.slug.

@jackguoAtJogg
Copy link

This could be a good solution: https://github.com/morhogg/use-client-router

@timneutkens
Copy link
Member

@jackguoAtJogg it'll break hydration causing pages to get into a broken state so certainly would not recommend it.

@rohanx96
Copy link

As a workaround, I created a pageQuery hook that parses the utm params manually and appends them to router.query. This makes sure that the utm params are available on the first render on client where the hook is used.

  const router = useRouter();
  const query = useRef({});
  // Parse query from URL - this avoids delay in query params by next js router
  useEffect(() => {
    const urlQuery = new URLSearchParams(router.asPath.split('?')[1]);
    Array.from(urlQuery.entries()).forEach(([key, value]) => {
      query.current[key] = value;
    });
  }, [router.asPath]);
  query.current = { ...query.current, ...router.query };
  return query.current;

@balazsorban44
Copy link
Member

This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@vercel vercel locked as resolved and limited conversation to collaborators Jan 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.