Next.js: Router context breaking change in v8.0.0-canary.6

[jaydenseric]

Bug report

Describe the bug

The next/router migration to React.createContext in https://github.com/zeit/next.js/pull/6030 (Next.js v8.0.0-canary.6) is a breaking change for the GraphQL SSR related examples such as with-graphql-react and probably with-apollo.

To Reproduce

In the with-graphql-react example, update the next dependency to 8.0.0-canary.6 and replace the contents of pages/index.js with:

import { Query } from 'graphql-react'
import { RouterContext } from 'next/router'

export default () => (
  <RouterContext.Consumer>
    {router => {
      console.log(router)
      return null
    }}
  </RouterContext.Consumer>
)

With npm run dev, in terminal you will see first undefined (bad) for the GraphQL preload render and then a router instance when the final SSR happens:

I imagine this reproduction would work for the with-apollo examples too:

https://github.com/zeit/next.js/blob/v8.0.0-canary.6/examples/with-apollo/lib/with-apollo-client.js#L27

Expected behavior

Perhaps the API would be non-breaking from Next.js v7 if <RouterContext.Consumer> was used inside of <App> instead of immediately outside.

System information

• Version of Next.js: v8.0.0-canary.6

Additional context

Is this breaking change intentional? If so, I can work it in to next-graphql-react and cut a new release for Next.js v8 compatibility.

The Apollo examples will also need updating.

[timneutkens]

Right so the issue is the non-standard rendering of the React tree in getInitialProps, I guess the best solution would be to wrap Component in the provider before Component is actually passed to _app, so that when it's rendered the value is provided automatically. This would require no user-side changes.

[todesstoss]

@timneutkens Hello, i'm currently using Next.js v8.1.0, and as I understand after implementation of useRouter, Provider implemented with a same issue described here, legacy context witRouter works just fine, but useRouter breaks my getDataFromTree in with-apollo example, as useRouter return null in this method, is it smth that will be fixed or it is expected behaviour? Thanks

[timneutkens]

See https://github.com/zeit/next.js/issues/7075#issuecomment-484780337

[todesstoss]

Yes, sure, but i think it's smth you should be aware of

[adamsoffer]

Hey @jaydenseric 👋 I'm encountering this as well. Did you by any chance figure out a workaround?

[adamsoffer]

Can confirm Next router is not working when using Apollo as well.

[jaydenseric]

@adamsoffer perhaps you are trying to use the useRouter hook, or using one of the broken Next.js versions?

The original issue was resolved when modern context for router was reverted before v8 landed. Modern context would be fine for us, if implemented in a different way so the <App> API doesn't break:

Perhaps the API would be non-breaking from Next.js v7 if was used inside of instead of immediately outside.

I'm looking forward to being able to use the useRouter hook once it's implemented properly.

[timneutkens]

I'm looking forward to being able to use the useRouter hook once it's implemented properly.

Note that it is implemented properly and it's Apollo that causes this for you, not Next.js.

[adamsoffer]

@timneutkens Would you mind explaining what's causing this bug so I can open up an issue in react-apollo?

[timneutkens]

Well getDataFromTree does a full server-render (one for every single query component) so it needs the full tree as otherwise it doesn't have certain values in context (this was already the case).

So basically I guess we'll have to provide some kind of way to provide the tree, even though we really wouldn't want to as it can lead to misusage.

[adamsoffer]

@timneutkens @jaydenseric I updated my next-apollo-example to use @apollo/react-hooks and useRouter 😀

For now, I get around this issue by wrapping router in a conditional: https://github.com/adamsoffer/next-apollo-example/blob/master/components/Header/index.js#L9

Not ideal, so if anyone has any ideas here let me know!