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

Make adjustment to cache config of with-apollo example #32733

Merged
merged 3 commits into from Feb 6, 2022
Merged

Make adjustment to cache config of with-apollo example #32733

merged 3 commits into from Feb 6, 2022

Conversation

glenngijsberts
Copy link
Contributor

Description

This year we implemented the new Apollo config using this example. We recently moved to getServerSideProps as well, however, this was giving us some cache problems. The issue was that the cache was older than the actual data that was coming from the server side query.

Because the merge of the cache in apolloClient.js is merging the existingCache in the initialState it will overwrite the "fresh" initialState with properties that already exists. This means that if you have something in your cache from previous client render, for example user with the value null, and you go to a new page and do a new query on the server which returns a value for the user field, it will still return null because of that merge function.

Since this was weird in our opinion, we've changed this in our own environment by always overwriting the existing cache with the new initialState, so that the initialState that is coming from a fresh server side query is overwriting. We thought it was a good idea to reflect this also in this example, because we couldn't think of a reason why the existing cache should overwrite fresh queries.

I've added a reproduction that shows what this is exactly about. I hope my description makes sense, let me know what you think!

Reproduction of old scenario

Created a reproduction branch here: https://github.com/glenngijsberts/with-apollo-example. Using a different API than in the example, because of #32731.

  1. checkout the example
  2. yarn
  3. yarn dev
  4. visit http://localhost:3000/client-only
  5. Hit "Update name". This will run a mutation that will update the cache automatically. Because I use a mocked API, the actual value on the API won't be updated, but this is actually the perfect scenario for the problem because in reality data can already change in the meantime when you're doing a new request.
  6. Go to the SSR page
  7. This will display two values: one is coming from the server side request (which is the latest data, because no cache is used in getServerSideProps) and the other is using the cache, which is outdated at that point, yet it's still returned because the old way of merging the cache was picking the existing cache over the initialState that was coming from the fresh server side query.

Documentation / Examples

  • Make sure the linting passes by running yarn lint

@ijjk ijjk added the examples Issue/PR related to examples label Dec 22, 2021
@exipnus
Copy link

exipnus commented Dec 28, 2021

Hi,

This does not solve the main problem though. A new API request is made in getServerSideProps each time you visit the SSR page.

Am I wrong?

@glenngijsberts
Copy link
Contributor Author

glenngijsberts commented Dec 28, 2021

Hi,

This does not solve the main problem though. A new API request is made in getServerSideProps each time you visit the SSR page.

Am I wrong?

@exipnus What's the main problem according to you? You're correct, in the sense that getServerSideProps is always doing a request. The reason for this, as how I understand it, is because you need to initialise a new Apollo client on each server side request, which doesn't have a cache (because it was just initialised), so it will query a new result.

Anyway, this PR is fixing a different problem, so i'm not sure how exactly related your comment is.

Copy link
Member

@ijjk ijjk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch this sounds like a good change, thanks for the PR!

@kodiakhq kodiakhq bot merged commit 91146b2 into vercel:canary Feb 6, 2022
natew pushed a commit to natew/next.js that referenced this pull request Feb 16, 2022
## Description
This year we implemented the new Apollo config using this example. We recently moved to `getServerSideProps` as well, however, this was giving us some cache problems. The issue was that the cache was older than the actual data that was coming from the server side query. 

Because the `merge` of the cache in `apolloClient.js` is merging the existingCache in the initialState it will overwrite the "fresh" initialState with properties that already exists. This means that if you have something in your cache from previous client render, for example `user` with the value `null`, and you go to a new page and do a new query on the server which returns a value for the `user` field, it will still return `null` because of that `merge` function.

Since this was weird in our opinion, we've changed this in our own environment by always overwriting the existing cache with the new initialState, so that the initialState that is coming from a fresh server side query is overwriting. We thought it was a good idea to reflect this also in this example, because we couldn't think of a reason why the existing cache should overwrite fresh queries.

I've added a reproduction that shows what this is exactly about. I hope my description makes sense, let me know what you think!

## Reproduction of old scenario
Created a reproduction branch here: https://github.com/glenngijsberts/with-apollo-example. Using a different API than in the example, because of vercel#32731.

1. checkout the example
2. yarn
3. yarn dev
4. visit http://localhost:3000/client-only
5. Hit "Update name". This will run a mutation that will update the cache automatically. Because I use a mocked API, the actual value on the API won't be updated, but this is actually the perfect scenario for the problem because in reality data can already change in the meantime when you're doing a new request.
6. Go to the SSR page
7. This will display two values: one is coming from the server side request (which is the latest data, because no cache is used in `getServerSideProps`) and the other is using the cache, which is outdated at that point, yet it's still returned because the old way of merging the cache was picking the existing cache over the initialState that was coming from the fresh server side query.

## Documentation / Examples

- [x] Make sure the linting passes by running `yarn lint`
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
examples Issue/PR related to examples
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants