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

Loading should not be initially true when the data is provided server-side [reproduction provided] #6651

Closed
daryn-k opened this issue Jul 20, 2020 · 10 comments

Comments

@daryn-k
Copy link

daryn-k commented Jul 20, 2020

Intended outcome

new InMemoryCache().restore(window.apolloState || {}) should populate the cache from window.apolloState. It works with apollo-client (version 2.6.x), but not with @apollo/client.

Actual outcome

new InMemoryCache().restore(window.apolloState || {}) doesn't populate the cache, it's empty. By the way, there is no any problem with server and server-side rendering. The problem is in client side. It doesn't restore the cache. We are getting the warning from React because of that.

Warning: Text content did not match.
Server: "Success!sunt aut facere repellat provident occaecati excepturi optio reprehenderit"
Client: "Loading..."

How to reproduce

Here are demos:

Github: https://github.com/daryn-k/apollo-client-3-ssr-demo.git [link]
CodeSandBox: https://codesandbox.io/s/optimistic-dust-wu0u6 [link]

npm install
npm run client
npm run server
npm start

You can comment and uncomment the lines with 🕷 and ✅ and clearly see how it works with old version 2.6.x and doesn't work with the 3.x.

./src/client.js
./src/server.js

import { ApolloClient } from '@apollo/client' // 🕷 It doesn't work with @apollo-client
import { InMemoryCache } from '@apollo/client' // 🕷 same here

// import { ApolloClient } from 'apollo-client' // ✅ But it works with apollo-client v2.6.x
// import { InMemoryCache } from 'apollo-cache-inmemory' // ✅ and apollo-cache-inmemory
@daryn-k daryn-k changed the title InMemoryCache.restore (Apollo Client 3) doesn't work InMemoryCache.restore from Apollo Client 3 doesn't work [reproduction provided] Jul 20, 2020
@maapteh
Copy link

maapteh commented Jul 20, 2020

Can you make it simply runnable from https://codesandbox.io/s/github/daryn-k/apollo-client-3-ssr-demo
(you can use concurrently package to kick server and client and after that run start) I get errors like 'DevTools failed to load SourceMap: Could not load content for webpack:///node_modules/@apollo/client/index.js.map: HTTP error: status code 404, net::ERR_UNKNOWN_URL_SCHEME'

But from your looking at your code can you maybe this?

- ssrMode: true,
+ ssrMode: typeof window === 'undefined', // Disables forceFetch on the server (so queries are only run once)

@daryn-k
Copy link
Author

daryn-k commented Jul 20, 2020

No, the reason of the problem is not in ssrMode option. There is no any problem with server side. As you noticed, server and client have different ApolloClient instances.

Rewrote some stuff and it's ready: https://codesandbox.io/s/optimistic-dust-wu0u6

For hide DevTools failed to load SourceMap warnings please uncheck Enable JavaScript source maps (it's because of new version of Chrome) in Dev Tools Preferences.

Снимок экрана 2020-07-20 в 20 39 11

@maapteh
Copy link

maapteh commented Jul 20, 2020

Maybe its me but your title claims it doesn't work. While in the sample it works, only the fact it renders twice because of 'loading' suddenly being true. But data is not being fetched again (filled from window.apolloState). So the restore mechanism works, only the 'loading: true' effect is unwanted behaviour and seems like the bug. Maybe adjust the title more about loading state, which is indeed new since v3. Good luck!

Which is also briefly described here: #6631 (comment)

@daryn-k
Copy link
Author

daryn-k commented Jul 20, 2020

Sorry, but you are not correct. You didn't understand what is the problem. Twice fetching is not normal behavior.

new InMemoryCache().restore() doesn't work. Because it should restore the cache from window.apolloState object. So if cache populated it will not fetch data one more time.

Everything worked and works perfectly with apollo-client version 2.6.x. But it breaks with @apollo/client version 3.x.

Author of the comment you linked to is complaining on renderToString, but I don't have any problem with this method.

One more time.

It's actual outcome:

Снимок экрана 2020-07-20 в 23 09 20

It's Intended outcome:

Снимок экрана 2020-07-20 в 23 12 18

@maapteh
Copy link

maapteh commented Jul 20, 2020

"Author of the comment you linked to is complaining on renderToString, but I don't have any problem with this method."

please read the second issue mentioned:

"But there's a second issue, which is client side. The first call to useQuery for a given query, with ssr: true, returns the correct data, but with loading: true. The cache is correctly pre-populated and no network request is sent, and it immediately does an update/rerender with loading:false, but all the extra rerendering has a noticeable performance imact."

exactly your issue, it doesnt get the data twice! So your "So if cache populated it will not fetch data one more time." statement is not correct. You dont see any xhr request happening client-side! Only the loading state is incorrect! If you remove that you will see no hydration warning anymore. Its just for a split second it starts in loading state, then it takes the data from your initial data state.

@maapteh
Copy link

maapteh commented Jul 20, 2020

https://wu0u6.sse.codesandbox.io/

no xhr call client-side, data populated from initial state. Only problem is loading:true for split second:

@daryn-k
Copy link
Author

daryn-k commented Jul 20, 2020

Oops... You are absolutely correct. Sorry, it's my bad. Right, there is no second fetch. Give me some time to dig into this problem.

If I write:

if (!data) return <div>Loading...</div>

there is no error.

@maapteh
Copy link

maapteh commented Jul 20, 2020

Yes, so change the title into this bug behaviour :)

Loading should not be initially true when the data is provided server-side.

@daryn-k daryn-k changed the title InMemoryCache.restore from Apollo Client 3 doesn't work [reproduction provided] Loading should not be initially true when the data is provided server-side [reproduction provided] Jul 20, 2020
@kiyasov
Copy link

kiyasov commented Jul 23, 2020

When will this bug be fixed?

@daryn-k
Copy link
Author

daryn-k commented Jul 27, 2020

Should be fixed here: #6710

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants