adamsoffer / next-apollo

React higher-order component for integrating Apollo Client with Next.js
MIT License
482 stars 64 forks source link

Hydratation bug with first render #90

Open adrienpsl opened 3 years ago

adrienpsl commented 3 years ago

Hello,

I have followed the instructions in the readme but my page when it loads does the same query as the ssr page. However there is the _NEXTDATA with the right data. My cache is not hydrated, is it possible to have the source code of the demo so that I understand what I did wrong?

also:

In a page contained by WithApollo

export default WithApollo( { ssr : true } )( Home );

I have as props:

(2) [{…}, {…}]
0: {apolloState: {…}, apolloClient: null}
1: {}

The apolloState has the right data, how can I pass them to Apollo client ?

Thanks! :).

adrienpsl commented 3 years ago

I found a solution, the store did not get the the store at the first render, and has it is save in globalApolloClient, remaining empty.

here the file that works with my modification :).

https://gist.github.com/adrienpsl/b99a6d0ae10803ea433bed544ce38142

Have a nice day ! :

aboveyunhai commented 3 years ago

@adrienpsl Your modification actually solved my hydration issue with some text content inconsistence issue between server and client side. I was able to do some semi dynamic persistent layout with next.js and apollo. Maybe you should reopen it and rise the attention to the package maintainer ?

adrienpsl commented 3 years ago

Hi @aboveyunhai :), I'm glad I that was able to help you !

You right if we are 2 I reopen this issu.

adamsoffer commented 3 years ago

Here's the demo source code https://github.com/adamsoffer/next-apollo-example/tree/f4ea2ffd05e18af15589051ffcbd688d6b8019a1

(visible by visiting https://next-with-apollo.vercel.app/_src)

aboveyunhai commented 3 years ago

@adamsoffer just a weird question to elaborate the demo. What happen if you do something like

export default withApollo({ ssr: false })(App) inside your _app.js.

and then still do export default withApollo({ ssr: true })(Home) inside the index.js

adamsoffer commented 3 years ago

Not sure. The page config might override it.

aboveyunhai commented 3 years ago

@adamsoffer , that's the behavior I would expect, but it seems like it will cause some inconsistency on SSR (For example , SSR on page), it's hard to produce the issue.

https://gist.github.com/adrienpsl/b99a6d0ae10803ea433bed544ce38142#file-next-apollo-tsx-L99 (only fewer lines inside if statement) added by @adrienpsl somehow solved it. Probably you can take a look, there might be a bug from the current package. I'm still not quite understanding everything behinds Apollo state on initialization.

adamsoffer commented 3 years ago

@aboveyunhai feel free to submit a PR if that fixes something not working. If persistent layout is what you're after check this out https://adamwathan.me/2019/10/17/persistent-layout-patterns-in-nextjs/

aboveyunhai commented 3 years ago

@adrienpsl just want to further enhance your implementation in case if you hadn't meet the problem. if (globalApolloClient && initialState && !hasSetInitialStore) will be executed when initalState = {} because if ({}) is evaluated true but we don't want to restore the empty state obviously. So the true implementation is to do a true check on empty object such as

if (globalApolloClient && !isObjectEmpty(initialState) && !hasSetInitialStore)

export function isObjectEmpty(obj: Object) {
    for(var i in obj) return false;
    return true;
}

It seems like the problem only occurs on the really first render and and then you switch pages from SSR -> SSR -> go back to previous page SSR. The empty state kicks in and cause the unwanted store. SSR to CSR back and forth wouldn't create that empty state; And it only appears on the first time.