Next.js: [Docs] Change Dynamic Routing Example to Avoid SPR Footgun

[karlhorky]

Feature request

First of all, thanks for all of your hard work on Next.js! There's so much amazing technology in this framework, and it really feels like standing on the shoulders of giants using it. Keep it up! 🚀

Is your feature request related to a problem? Please describe.

If you use the useRouter Dynamic Routing example below from the current docs, it will out of the box apply the Serverless Prerendering (SPR) optimization and parameters will for a short time be undefined (let's call this Flash of Missing Content).

import { useRouter } from 'next/router'

const Post = () => {
  const router = useRouter()
  const { pid } = router.query

  return <p>Post: {pid}</p>
}

export default Post

This is explained much further down in a box that can be easily missed:

Note: Pages that are statically optimized by automatic prerendering will be hydrated without their route parameters provided (query will be empty, i.e. {}). After hydration, Next.js will trigger an update to your application to provide the route parameters in the query object. If your application cannot tolerate this behavior, you can opt-out of static optimization by capturing the query parameter in getInitialProps.

The Flash of Missing Content is annoying and it is unclear why it's happening to a user not versed in all of the new features of Next.js.

In addition, this can be a problem in a simple case of trying to access a property on an object found using the query parameter ("Cannot read property 'name' of undefined"):

import { useRouter } from 'next/router'
import posts from '../posts'

const Post = () => {
  const router = useRouter()
  const { pid } = router.query
  const post = posts.find(p => p.id === pid)

  return (
    <p>
      Post: {post.name} {/* 💥 Error! */}
    </p>
  )
}

export default Post

Describe the solution you'd like

The example should work out of the box (copy + paste) without the flash of missing content, by at least recommending the use of the getInitialProps method.

Post.getInitialProps = async () => {
  return {};
};

Describe alternatives you've considered

An alternative (which would actually be preferable, because it's zero-config) would be for the useRouter hook to add an empty getInitialProps method automatically when its used (of course, only if it's not defined by the user). This would be the least footgun-y and offer the most pleasant DX.

A second alternative would be to have the getInitialProps part be in a separate box directly after the example, with some explainer text above it.

Another alternative would be to have the code in the example but commented out. This would probably lead to less usage of the code though, and would be much less desirable.

Additional context

By googling for the issue ("Next.js useRouter parameters undefined" or "Next.js useRouter server-side rendering"), you can find this issue, which provides the example that inspired this issue:

https://spectrum.chat/next-js/general/next-js-userouter-query-amp-undefined~c4954ea2-405c-4e66-98fa-f8704d4d7f6e?m=MTU2Mzk0MTIyOTk2OQ==

[karlhorky]

The more I think about it, the more that I actually prefer my first alternative I mentioned above - the zero-configuration option of the useRouter hook automatically configuring the component to have an empty getInitialProps if one doesn't exist...

[ianstormtaylor]

I agree, this is a super confusing pitfall. Just took me a long time to understand why Next would be calling my /examples/[example].tsx page without any { example: '...'} query. I don't understand why this is the default for dynamic routing?

[mguida22]

Spent a while searching for this, thanks for such a detailed write up!

It'd be really nice to have this in the example in the docs for dynamic routing so it's easier find when starting out with Next.js

[timneutkens]

This was an in-between state to get us here: https://github.com/zeit/next.js/issues/9524

Looking back it might have been a mistake to have introduced withRouter / useRouter. Or at least the part where it can read query. However when #9524 lands it's a lot clearer what the expected path is for users, you do data fetching at build time (getStaticProps + getStaticPaths ) or runtime (getServerProps).

[karlhorky]

Ok, thanks for the answer @timneutkens. So I suppose this is a wontfix until then? Should I close?

[timneutkens]

@karlhorky it's fine to keep it open.

[lifeiscontent]

@timneutkens what is the recommended approach to solving this issue now that #9524 has been merged?

[lifeiscontent]

@timneutkens it seems to me that depending on the client / server a query string should be parsed and set on the router instance. I've created a very small library called isomorphic-querystring which I think would sit nicely within the next.js Router. Would you like me to implement it?

[timneutkens]

@timneutkens what is the recommended approach to solving this issue now that #9524 has been merged?

Querystring can't be read while pre-rendering pages. Meaning it's expected that it's only available in the browser after the initial hydration.

This would solve the "knowing when it's ready": https://github.com/zeit/next.js/issues/8259

@timneutkens it seems to me that depending on the client / server a query string should be parsed and set on the router instance. I've created a very small library called isomorphic-querystring which I think would sit nicely within the next.js Router. Would you like me to implement it?

This doesn't seem like something we'd want to add to next/router as it does nothing different than the way that we currently handle things.

[Timer]

This behavior is working as expected and is not a foot gun. I'm closing this as it was primarily solved by Next.js' SSG support (getStaticPaths and getStaticProps).

The remaining change that is needed will be covered by https://github.com/vercel/next.js/issues/8259!