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

Update with-apollo examples to merge existing cache #15804

Merged
merged 4 commits into from
Aug 5, 2020

Conversation

JClackett
Copy link
Contributor

At the moment, any data that is cached from client side data fetching is completely wiped when navigating to a SSG/SSR page.

This PR merges the existing cached data with the data fetched from getServerSideProps/getStaticProps

@ijjk ijjk added the examples Issue was opened via the examples template. label Aug 2, 2020
@ijjk
Copy link
Member

ijjk commented Aug 2, 2020

Stats from current PR

Default Server Mode (Decrease detected ✓)
General
vercel/next.js canary JClackett/next.js canary Change
buildDuration 12.4s 12.1s -236ms
nodeModulesSize 65.5 MB 65.5 MB
Page Load Tests Overall decrease ⚠️
vercel/next.js canary JClackett/next.js canary Change
/ failed reqs 0 0
/ total time (seconds) 2.158 2.176 ⚠️ +0.02
/ avg req/sec 1158.54 1148.81 ⚠️ -9.73
/error-in-render failed reqs 0 0
/error-in-render total time (seconds) 1.218 1.218
/error-in-render avg req/sec 2053.38 2052.22 ⚠️ -1.16
Client Bundles (main, webpack, commons)
vercel/next.js canary JClackett/next.js canary Change
677f882d2ed8..39a4.js gzip 10.2 kB 10.2 kB
framework.HASH.js gzip 39.1 kB 39.1 kB
main-06588f2..f65e.js gzip 6.73 kB 6.73 kB
webpack-488d..c0e7.js gzip 751 B 751 B
Overall change 56.8 kB 56.8 kB
Client Bundles (main, webpack, commons) Modern
vercel/next.js canary JClackett/next.js canary Change
677f882d2ed8..dule.js gzip 6.13 kB 6.13 kB
framework.HA..dule.js gzip 39.1 kB 39.1 kB
main-62e1f09..dule.js gzip 5.81 kB 5.81 kB
webpack-4f62..dule.js gzip 751 B 751 B
Overall change 51.8 kB 51.8 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary JClackett/next.js canary Change
polyfills-05..1236.js gzip 30.8 kB 30.8 kB
Overall change 30.8 kB 30.8 kB
Client Pages
vercel/next.js canary JClackett/next.js canary Change
_app-8f5f611..1f7b.js gzip 1.28 kB 1.28 kB
_error-a98d9..5cb7.js gzip 3.45 kB 3.45 kB
hooks-f7f3d0..7465.js gzip 887 B 887 B
index-08fb3f..c0e9.js gzip 227 B 227 B
link-6f8445b..99e1.js gzip 1.3 kB 1.3 kB
routerDirect..8aa1.js gzip 284 B 284 B
withRouter-f..e777.js gzip 284 B 284 B
Overall change 7.72 kB 7.72 kB
Client Pages Modern
vercel/next.js canary JClackett/next.js canary Change
_app-669dbe5..dule.js gzip 626 B 626 B
_error-d5979..dule.js gzip 2.3 kB 2.3 kB
hooks-805c40..dule.js gzip 387 B 387 B
index-6ba5a4..dule.js gzip 226 B 226 B
link-91516ae..dule.js gzip 1.25 kB 1.25 kB
routerDirect..dule.js gzip 284 B 284 B
withRouter-d..dule.js gzip 282 B 282 B
Overall change 5.36 kB 5.36 kB
Client Build Manifests
vercel/next.js canary JClackett/next.js canary Change
_buildManifest.js gzip 274 B 274 B
_buildManife..dule.js gzip 282 B 282 B
Overall change 556 B 556 B
Rendered Page Sizes
vercel/next.js canary JClackett/next.js canary Change
index.html gzip 946 B 946 B
link.html gzip 952 B 952 B
withRouter.html gzip 939 B 939 B
Overall change 2.84 kB 2.84 kB

Serverless Mode
General
vercel/next.js canary JClackett/next.js canary Change
buildDuration 13.8s 13.7s -137ms
nodeModulesSize 65.5 MB 65.5 MB
Client Bundles (main, webpack, commons)
vercel/next.js canary JClackett/next.js canary Change
677f882d2ed8..39a4.js gzip 10.2 kB 10.2 kB
framework.HASH.js gzip 39.1 kB 39.1 kB
main-06588f2..f65e.js gzip 6.73 kB 6.73 kB
webpack-488d..c0e7.js gzip 751 B 751 B
Overall change 56.8 kB 56.8 kB
Client Bundles (main, webpack, commons) Modern
vercel/next.js canary JClackett/next.js canary Change
677f882d2ed8..dule.js gzip 6.13 kB 6.13 kB
framework.HA..dule.js gzip 39.1 kB 39.1 kB
main-62e1f09..dule.js gzip 5.81 kB 5.81 kB
webpack-4f62..dule.js gzip 751 B 751 B
Overall change 51.8 kB 51.8 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary JClackett/next.js canary Change
polyfills-05..1236.js gzip 30.8 kB 30.8 kB
Overall change 30.8 kB 30.8 kB
Client Pages
vercel/next.js canary JClackett/next.js canary Change
_app-8f5f611..1f7b.js gzip 1.28 kB 1.28 kB
_error-a98d9..5cb7.js gzip 3.45 kB 3.45 kB
hooks-f7f3d0..7465.js gzip 887 B 887 B
index-08fb3f..c0e9.js gzip 227 B 227 B
link-6f8445b..99e1.js gzip 1.3 kB 1.3 kB
routerDirect..8aa1.js gzip 284 B 284 B
withRouter-f..e777.js gzip 284 B 284 B
Overall change 7.72 kB 7.72 kB
Client Pages Modern
vercel/next.js canary JClackett/next.js canary Change
_app-669dbe5..dule.js gzip 626 B 626 B
_error-d5979..dule.js gzip 2.3 kB 2.3 kB
hooks-805c40..dule.js gzip 387 B 387 B
index-6ba5a4..dule.js gzip 226 B 226 B
link-91516ae..dule.js gzip 1.25 kB 1.25 kB
routerDirect..dule.js gzip 284 B 284 B
withRouter-d..dule.js gzip 282 B 282 B
Overall change 5.36 kB 5.36 kB
Client Build Manifests
vercel/next.js canary JClackett/next.js canary Change
_buildManifest.js gzip 274 B 274 B
_buildManife..dule.js gzip 282 B 282 B
Overall change 556 B 556 B
Serverless bundles
vercel/next.js canary JClackett/next.js canary Change
_error.js 1.02 MB 1.02 MB
404.html 4.18 kB 4.18 kB
hooks.html 3.82 kB 3.82 kB
index.js 1.02 MB 1.02 MB
link.js 1.06 MB 1.06 MB
routerDirect.js 1.05 MB 1.05 MB
withRouter.js 1.05 MB 1.05 MB
Overall change 5.2 MB 5.2 MB
Commit: 33bf656

@ijjk
Copy link
Member

ijjk commented Aug 2, 2020

Stats from current PR

Default Server Mode (Decrease detected ✓)
General
vercel/next.js canary JClackett/next.js canary Change
buildDuration 12.6s 12.5s -105ms
nodeModulesSize 65.5 MB 65.5 MB
Page Load Tests Overall decrease ⚠️
vercel/next.js canary JClackett/next.js canary Change
/ failed reqs 0 0
/ total time (seconds) 2.192 2.223 ⚠️ +0.03
/ avg req/sec 1140.77 1124.78 ⚠️ -15.99
/error-in-render failed reqs 0 0
/error-in-render total time (seconds) 1.183 1.266 ⚠️ +0.08
/error-in-render avg req/sec 2113.42 1974.63 ⚠️ -138.79
Client Bundles (main, webpack, commons)
vercel/next.js canary JClackett/next.js canary Change
677f882d2ed8..39a4.js gzip 10.2 kB 10.2 kB
framework.HASH.js gzip 39.1 kB 39.1 kB
main-06588f2..f65e.js gzip 6.73 kB 6.73 kB
webpack-488d..c0e7.js gzip 751 B 751 B
Overall change 56.8 kB 56.8 kB
Client Bundles (main, webpack, commons) Modern
vercel/next.js canary JClackett/next.js canary Change
677f882d2ed8..dule.js gzip 6.13 kB 6.13 kB
framework.HA..dule.js gzip 39.1 kB 39.1 kB
main-62e1f09..dule.js gzip 5.81 kB 5.81 kB
webpack-4f62..dule.js gzip 751 B 751 B
Overall change 51.8 kB 51.8 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary JClackett/next.js canary Change
polyfills-05..1236.js gzip 30.8 kB 30.8 kB
Overall change 30.8 kB 30.8 kB
Client Pages
vercel/next.js canary JClackett/next.js canary Change
_app-8f5f611..1f7b.js gzip 1.28 kB 1.28 kB
_error-a98d9..5cb7.js gzip 3.45 kB 3.45 kB
hooks-f7f3d0..7465.js gzip 887 B 887 B
index-08fb3f..c0e9.js gzip 227 B 227 B
link-6f8445b..99e1.js gzip 1.3 kB 1.3 kB
routerDirect..8aa1.js gzip 284 B 284 B
withRouter-f..e777.js gzip 284 B 284 B
Overall change 7.72 kB 7.72 kB
Client Pages Modern
vercel/next.js canary JClackett/next.js canary Change
_app-669dbe5..dule.js gzip 626 B 626 B
_error-d5979..dule.js gzip 2.3 kB 2.3 kB
hooks-805c40..dule.js gzip 387 B 387 B
index-6ba5a4..dule.js gzip 226 B 226 B
link-91516ae..dule.js gzip 1.25 kB 1.25 kB
routerDirect..dule.js gzip 284 B 284 B
withRouter-d..dule.js gzip 282 B 282 B
Overall change 5.36 kB 5.36 kB
Client Build Manifests
vercel/next.js canary JClackett/next.js canary Change
_buildManifest.js gzip 274 B 274 B
_buildManife..dule.js gzip 282 B 282 B
Overall change 556 B 556 B
Rendered Page Sizes
vercel/next.js canary JClackett/next.js canary Change
index.html gzip 946 B 946 B
link.html gzip 952 B 952 B
withRouter.html gzip 939 B 939 B
Overall change 2.84 kB 2.84 kB

Serverless Mode
General
vercel/next.js canary JClackett/next.js canary Change
buildDuration 13.6s 14.3s ⚠️ +783ms
nodeModulesSize 65.5 MB 65.5 MB
Client Bundles (main, webpack, commons)
vercel/next.js canary JClackett/next.js canary Change
677f882d2ed8..39a4.js gzip 10.2 kB 10.2 kB
framework.HASH.js gzip 39.1 kB 39.1 kB
main-06588f2..f65e.js gzip 6.73 kB 6.73 kB
webpack-488d..c0e7.js gzip 751 B 751 B
Overall change 56.8 kB 56.8 kB
Client Bundles (main, webpack, commons) Modern
vercel/next.js canary JClackett/next.js canary Change
677f882d2ed8..dule.js gzip 6.13 kB 6.13 kB
framework.HA..dule.js gzip 39.1 kB 39.1 kB
main-62e1f09..dule.js gzip 5.81 kB 5.81 kB
webpack-4f62..dule.js gzip 751 B 751 B
Overall change 51.8 kB 51.8 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary JClackett/next.js canary Change
polyfills-05..1236.js gzip 30.8 kB 30.8 kB
Overall change 30.8 kB 30.8 kB
Client Pages
vercel/next.js canary JClackett/next.js canary Change
_app-8f5f611..1f7b.js gzip 1.28 kB 1.28 kB
_error-a98d9..5cb7.js gzip 3.45 kB 3.45 kB
hooks-f7f3d0..7465.js gzip 887 B 887 B
index-08fb3f..c0e9.js gzip 227 B 227 B
link-6f8445b..99e1.js gzip 1.3 kB 1.3 kB
routerDirect..8aa1.js gzip 284 B 284 B
withRouter-f..e777.js gzip 284 B 284 B
Overall change 7.72 kB 7.72 kB
Client Pages Modern
vercel/next.js canary JClackett/next.js canary Change
_app-669dbe5..dule.js gzip 626 B 626 B
_error-d5979..dule.js gzip 2.3 kB 2.3 kB
hooks-805c40..dule.js gzip 387 B 387 B
index-6ba5a4..dule.js gzip 226 B 226 B
link-91516ae..dule.js gzip 1.25 kB 1.25 kB
routerDirect..dule.js gzip 284 B 284 B
withRouter-d..dule.js gzip 282 B 282 B
Overall change 5.36 kB 5.36 kB
Client Build Manifests
vercel/next.js canary JClackett/next.js canary Change
_buildManifest.js gzip 274 B 274 B
_buildManife..dule.js gzip 282 B 282 B
Overall change 556 B 556 B
Serverless bundles
vercel/next.js canary JClackett/next.js canary Change
_error.js 1.02 MB 1.02 MB
404.html 4.18 kB 4.18 kB
hooks.html 3.82 kB 3.82 kB
index.js 1.02 MB 1.02 MB
link.js 1.06 MB 1.06 MB
routerDirect.js 1.05 MB 1.05 MB
withRouter.js 1.05 MB 1.05 MB
Overall change 5.2 MB 5.2 MB
Commit: 3979591

Copy link
Member

@Timer Timer left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@ijjk
Copy link
Member

ijjk commented Aug 5, 2020

Stats from current PR

Default Server Mode (Decrease detected ✓)
General
vercel/next.js canary JClackett/next.js canary Change
buildDuration 14.9s 14.8s -121ms
nodeModulesSize 66 MB 66 MB
Page Load Tests Overall decrease ⚠️
vercel/next.js canary JClackett/next.js canary Change
/ failed reqs 0 0
/ total time (seconds) 2.749 2.771 ⚠️ +0.02
/ avg req/sec 909.34 902.27 ⚠️ -7.07
/error-in-render failed reqs 0 0
/error-in-render total time (seconds) 1.713 1.737 ⚠️ +0.02
/error-in-render avg req/sec 1459.03 1439.46 ⚠️ -19.57
Client Bundles (main, webpack, commons)
vercel/next.js canary JClackett/next.js canary Change
677f882d2ed8..afbf.js gzip 10.2 kB 10.2 kB
framework.HASH.js gzip 39.1 kB 39.1 kB
main-76d73eb..95d8.js gzip 6.73 kB 6.73 kB
webpack-ccf5..276a.js gzip 751 B 751 B
Overall change 56.8 kB 56.8 kB
Client Bundles (main, webpack, commons) Modern
vercel/next.js canary JClackett/next.js canary Change
677f882d2ed8..dule.js gzip 6.11 kB 6.11 kB
framework.HA..dule.js gzip 39.1 kB 39.1 kB
main-0b6ca7d..dule.js gzip 5.81 kB 5.81 kB
webpack-10c7..dule.js gzip 751 B 751 B
Overall change 51.8 kB 51.8 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary JClackett/next.js canary Change
polyfills-75..1629.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary JClackett/next.js canary Change
_app-874bd8a..0103.js gzip 1.28 kB 1.28 kB
_error-fa39c..ec40.js gzip 3.45 kB 3.45 kB
hooks-585f07..95a3.js gzip 887 B 887 B
index-c7b63f..fc02.js gzip 227 B 227 B
link-e97733c..af21.js gzip 1.3 kB 1.3 kB
routerDirect..ebc7.js gzip 284 B 284 B
withRouter-2..db68.js gzip 284 B 284 B
Overall change 7.72 kB 7.72 kB
Client Pages Modern
vercel/next.js canary JClackett/next.js canary Change
_app-97e743e..dule.js gzip 626 B 626 B
_error-b4004..dule.js gzip 2.3 kB 2.3 kB
hooks-696209..dule.js gzip 387 B 387 B
index-a4dd74..dule.js gzip 226 B 226 B
link-fbb1d2e..dule.js gzip 1.25 kB 1.25 kB
routerDirect..dule.js gzip 284 B 284 B
withRouter-1..dule.js gzip 282 B 282 B
Overall change 5.36 kB 5.36 kB
Client Build Manifests
vercel/next.js canary JClackett/next.js canary Change
_buildManifest.js gzip 272 B 272 B
_buildManife..dule.js gzip 280 B 280 B
Overall change 552 B 552 B
Rendered Page Sizes
vercel/next.js canary JClackett/next.js canary Change
index.html gzip 945 B 945 B
link.html gzip 953 B 953 B
withRouter.html gzip 939 B 939 B
Overall change 2.84 kB 2.84 kB

Serverless Mode
General
vercel/next.js canary JClackett/next.js canary Change
buildDuration 16.4s 16.1s -238ms
nodeModulesSize 66 MB 66 MB
Client Bundles (main, webpack, commons)
vercel/next.js canary JClackett/next.js canary Change
677f882d2ed8..afbf.js gzip 10.2 kB 10.2 kB
framework.HASH.js gzip 39.1 kB 39.1 kB
main-76d73eb..95d8.js gzip 6.73 kB 6.73 kB
webpack-ccf5..276a.js gzip 751 B 751 B
Overall change 56.8 kB 56.8 kB
Client Bundles (main, webpack, commons) Modern
vercel/next.js canary JClackett/next.js canary Change
677f882d2ed8..dule.js gzip 6.11 kB 6.11 kB
framework.HA..dule.js gzip 39.1 kB 39.1 kB
main-0b6ca7d..dule.js gzip 5.81 kB 5.81 kB
webpack-10c7..dule.js gzip 751 B 751 B
Overall change 51.8 kB 51.8 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary JClackett/next.js canary Change
polyfills-75..1629.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary JClackett/next.js canary Change
_app-874bd8a..0103.js gzip 1.28 kB 1.28 kB
_error-fa39c..ec40.js gzip 3.45 kB 3.45 kB
hooks-585f07..95a3.js gzip 887 B 887 B
index-c7b63f..fc02.js gzip 227 B 227 B
link-e97733c..af21.js gzip 1.3 kB 1.3 kB
routerDirect..ebc7.js gzip 284 B 284 B
withRouter-2..db68.js gzip 284 B 284 B
Overall change 7.72 kB 7.72 kB
Client Pages Modern
vercel/next.js canary JClackett/next.js canary Change
_app-97e743e..dule.js gzip 626 B 626 B
_error-b4004..dule.js gzip 2.3 kB 2.3 kB
hooks-696209..dule.js gzip 387 B 387 B
index-a4dd74..dule.js gzip 226 B 226 B
link-fbb1d2e..dule.js gzip 1.25 kB 1.25 kB
routerDirect..dule.js gzip 284 B 284 B
withRouter-1..dule.js gzip 282 B 282 B
Overall change 5.36 kB 5.36 kB
Client Build Manifests
vercel/next.js canary JClackett/next.js canary Change
_buildManifest.js gzip 272 B 272 B
_buildManife..dule.js gzip 280 B 280 B
Overall change 552 B 552 B
Serverless bundles
vercel/next.js canary JClackett/next.js canary Change
_error.js 1.02 MB 1.02 MB
404.html 4.18 kB 4.18 kB
hooks.html 3.82 kB 3.82 kB
index.js 1.02 MB 1.02 MB
link.js 1.06 MB 1.06 MB
routerDirect.js 1.05 MB 1.05 MB
withRouter.js 1.05 MB 1.05 MB
Overall change 5.2 MB 5.2 MB
Commit: c8b8308

@Timer Timer merged commit 026785d into vercel:canary Aug 5, 2020
@matthewlilley
Copy link
Contributor

matthewlilley commented Oct 3, 2020

@JClackett

This actually causes the local cache to be overwritten by potential old data on a client transition using next/link, since getStaticProps will not revalidate on a next/link transition, the old initialState is overwriting new existingCache. Also the ROOT_QUERY is not merged correctly. I assume that it should contain merged data from both but right now it doesn't.

I had to merge this way.

const data = {
  ...initialState,
  ...existingCache,
  ROOT_QUERY: {
    ...initialState.ROOT_QUERY,
    ...existingCache.ROOT_QUERY,
  },
};
_apolloClient.cache.restore(data);

@lfades
Copy link
Member

lfades commented Oct 5, 2020

@matthewlilley Can you create a PR to fix it? 🙏

@matthewlilley
Copy link
Contributor

@lfades My previous comment was a little premature, the only real fix is to use a deep merge to avoid more of the same problems. I used https://lodash.com/docs/4.17.15#merge

@lfades
Copy link
Member

lfades commented Oct 5, 2020

@matthewlilley Should we revert this change or is it in a better state than before?

@matthewlilley
Copy link
Contributor

@lfades I would add a dependency for lodash.merge and replace https://github.com/vercel/next.js/blob/canary/examples/with-apollo/lib/apolloClient.js#L36 with below to avoid more pain in the future 😂.

const data = merge(initialState, existingCache);

_apolloClient.cache.restore(data);

@lfades
Copy link
Member

lfades commented Oct 5, 2020

@matthewlilley Tbh if that fixes the issues, feel free to do so 👍

@matthewlilley
Copy link
Contributor

@lfades That was the simple option for me but I can imagine you don't want to add the dependency, however, this should be fixed or else it will cause much pain to other people who use the apollo examples, especially those not familiar with Apollo.

@matthewlilley
Copy link
Contributor

matthewlilley commented Oct 6, 2020

I would be happy to open a PR, if you want to avoid adding a dependency a Vanilla JS deep merge would be sufficient, something like this https://vanillajstoolkit.com/helpers/deepmerge/ - Let me know!

@lfades
Copy link
Member

lfades commented Oct 6, 2020

@matthewlilley it's be better with an external lib because including it manually will make the example more complicated. I'm not sure if lodash.merge is the best option so feel free to choose another one if it's better 👍

@matthewlilley
Copy link
Contributor

I used lodash simply because I knew I could include the single function as a dependency, deepmerge would suffice and is about 1/4 the size, or do you have another suggestion?

@vercel vercel locked as resolved and limited conversation to collaborators Jan 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
examples Issue was opened via the examples template.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants