Add WrapApp prop to AppTree to work in HOC and with apollo #17598

mhhegazy · 2020-10-04T02:10:42Z

Proplem

All withApollo HOC examples and code dosn't work if we use AppTree and if we use Component we don't get the right context

Apollo client context is singleton (single Apollo context is created and tracked in global state).
and when we use more then one ApolloProvider we overide client instance with most inner ApolloProvider client
So when we create HOC APP to get apolloClient initial cache state by using getDataFromTree by using server side apolloClient and then extract cache like this

function withApollo(WrappedApp) {
  const WithApollo = ({ apolloState, ...props }) => {
    const apolloClient = React.useMemo(() => initApollo(apolloState), []);
    return (
      <ApolloProvider client={apolloClient}>
        <WrappedApp {...props} />
      </ApolloProvider>
    );
  };
  WithApollo.getInitialProps = async () => {
    const apolloClient = initApollo({});
    // ...

    // only on server
    await getDataFromTree(
      <ApolloProvider client={apolloClient}>
        <AppTree {...appProps} />
      </ApolloProvider>
    );
    const apolloState = apolloClient.cache.extract();
    // ...
    return {
      // ...
      apolloState,
    };
  };

  return WithApollo;
}

on first render in getDataFromTree on server getInitialProps the apolloClient is not used.. the second client in WithApollo Component will overide it so when extract cache apolloClient.cache.extract() we get empty result because we extract overwritten client

Solution

AppTree provided in appContext meant to provide right context (router, head manager, amp, ...)
And for HOC we have app that we want to wrap in this contexts, so we should be able to provide app to wrap.

So we can have one ApolloContext in first Render
To fix example in problem we do this in getInitialProps:

// ...
await getDataFromTree(
  <ApolloProvider client={apolloClient}>
    <AppTree WrapApp={WrappedApp} {...appProps} />
  </ApolloProvider>
);
// ...

then we have only one ApolloProvider on first render and there is no inner one that overwrite it

working example:

next-apollo-client-ssr-example

to try it:

yarn add next@npm:@mhhegazy/[email protected]

fixes #9336
Related issue #10126
Related pull request #7732

ijjk · 2020-10-04T02:20:43Z

Stats from current PR

Default Server Mode (Decrease detected ✓)

General Overall increase ⚠️

	vercel/next.js canary	mhhegazy/next.js app-tree	Change
buildDuration	12.5s	12.8s	⚠️ +314ms
nodeModulesSize	63.1 MB	63.1 MB	⚠️ +218 B

Page Load Tests Overall decrease ⚠️

	vercel/next.js canary	mhhegazy/next.js app-tree	Change
/ failed reqs	0	0	✓
/ total time (seconds)	2.27	2.281	⚠️ +0.01
/ avg req/sec	1101.33	1095.82	⚠️ -5.51
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	1.256	1.277	⚠️ +0.02
/error-in-render avg req/sec	1991	1957.62	⚠️ -33.38

Client Bundles (main, webpack, commons)

	vercel/next.js canary	mhhegazy/next.js app-tree	Change
677f882d2ed8..9339.js gzip	10.9 kB	10.9 kB	✓
framework.HASH.js gzip	39 kB	39 kB	✓
main-d493e7d..42f5.js gzip	7.17 kB	7.17 kB	✓
webpack-e067..f178.js gzip	751 B	751 B	✓
Overall change	57.8 kB	57.8 kB	✓

Client Bundles (main, webpack, commons) Modern

	vercel/next.js canary	mhhegazy/next.js app-tree	Change
677f882d2ed8..dule.js gzip	6.77 kB	6.77 kB	✓
framework.HA..dule.js gzip	39 kB	39 kB	✓
main-f8905d4..dule.js gzip	6.24 kB	6.24 kB	✓
webpack-07c5..dule.js gzip	751 B	751 B	✓
Overall change	52.7 kB	52.7 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	mhhegazy/next.js app-tree	Change
polyfills-4b..e242.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages

	vercel/next.js canary	mhhegazy/next.js app-tree	Change
_app-9a0b9e1..b37e.js gzip	1.28 kB	1.28 kB	✓
_error-ed1b0..8fbd.js gzip	3.44 kB	3.44 kB	✓
hooks-89731c..c609.js gzip	887 B	887 B	✓
index-17468f..5d83.js gzip	227 B	227 B	✓
link-d2344ce..8b36.js gzip	1.3 kB	1.3 kB	✓
routerDirect..924c.js gzip	284 B	284 B	✓
withRouter-7..c13d.js gzip	284 B	284 B	✓
Overall change	7.71 kB	7.71 kB	✓

Client Pages Modern

	vercel/next.js canary	mhhegazy/next.js app-tree	Change
_app-75d3a82..dule.js gzip	625 B	625 B	✓
_error-4469a..dule.js gzip	2.29 kB	2.29 kB	✓
hooks-cbf13f..dule.js gzip	387 B	387 B	✓
index-b9a643..dule.js gzip	226 B	226 B	✓
link-f8c0daf..dule.js gzip	1.26 kB	1.26 kB	✓
routerDirect..dule.js gzip	284 B	284 B	✓
withRouter-f..dule.js gzip	282 B	282 B	✓
Overall change	5.36 kB	5.36 kB	✓

Client Build Manifests

	vercel/next.js canary	mhhegazy/next.js app-tree	Change
_buildManifest.js gzip	322 B	322 B	✓
_buildManife..dule.js gzip	329 B	329 B	✓
Overall change	651 B	651 B	✓

Rendered Page Sizes

	vercel/next.js canary	mhhegazy/next.js app-tree	Change
index.html gzip	1 kB	1 kB	✓
link.html gzip	1.01 kB	1.01 kB	✓
withRouter.html gzip	996 B	996 B	✓
Overall change	3.01 kB	3.01 kB	✓

Serverless Mode (Increase detected ⚠️)

General Overall increase ⚠️

	vercel/next.js canary	mhhegazy/next.js app-tree	Change
buildDuration	14.8s	14.5s	-300ms
nodeModulesSize	63.1 MB	63.1 MB	⚠️ +218 B

Client Bundles (main, webpack, commons)

	vercel/next.js canary	mhhegazy/next.js app-tree	Change
677f882d2ed8..9339.js gzip	10.9 kB	10.9 kB	✓
framework.HASH.js gzip	39 kB	39 kB	✓
main-d493e7d..42f5.js gzip	7.17 kB	7.17 kB	✓
webpack-e067..f178.js gzip	751 B	751 B	✓
Overall change	57.8 kB	57.8 kB	✓

Client Bundles (main, webpack, commons) Modern

	vercel/next.js canary	mhhegazy/next.js app-tree	Change
677f882d2ed8..dule.js gzip	6.77 kB	6.77 kB	✓
framework.HA..dule.js gzip	39 kB	39 kB	✓
main-f8905d4..dule.js gzip	6.24 kB	6.24 kB	✓
webpack-07c5..dule.js gzip	751 B	751 B	✓
Overall change	52.7 kB	52.7 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	mhhegazy/next.js app-tree	Change
polyfills-4b..e242.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages

	vercel/next.js canary	mhhegazy/next.js app-tree	Change
_app-9a0b9e1..b37e.js gzip	1.28 kB	1.28 kB	✓
_error-ed1b0..8fbd.js gzip	3.44 kB	3.44 kB	✓
hooks-89731c..c609.js gzip	887 B	887 B	✓
index-17468f..5d83.js gzip	227 B	227 B	✓
link-d2344ce..8b36.js gzip	1.3 kB	1.3 kB	✓
routerDirect..924c.js gzip	284 B	284 B	✓
withRouter-7..c13d.js gzip	284 B	284 B	✓
Overall change	7.71 kB	7.71 kB	✓

Client Pages Modern

	vercel/next.js canary	mhhegazy/next.js app-tree	Change
_app-75d3a82..dule.js gzip	625 B	625 B	✓
_error-4469a..dule.js gzip	2.29 kB	2.29 kB	✓
hooks-cbf13f..dule.js gzip	387 B	387 B	✓
index-b9a643..dule.js gzip	226 B	226 B	✓
link-f8c0daf..dule.js gzip	1.26 kB	1.26 kB	✓
routerDirect..dule.js gzip	284 B	284 B	✓
withRouter-f..dule.js gzip	282 B	282 B	✓
Overall change	5.36 kB	5.36 kB	✓

Client Build Manifests

	vercel/next.js canary	mhhegazy/next.js app-tree	Change
_buildManifest.js gzip	322 B	322 B	✓
_buildManife..dule.js gzip	329 B	329 B	✓
Overall change	651 B	651 B	✓

Serverless bundles Overall increase ⚠️

	vercel/next.js canary	mhhegazy/next.js app-tree	Change
_error.js	1.05 MB	1.05 MB	⚠️ +52 B
404.html	4.34 kB	4.34 kB	✓
hooks.html	3.92 kB	3.92 kB	✓
index.js	1.05 MB	1.05 MB	⚠️ +52 B
link.js	1.1 MB	1.1 MB	⚠️ +52 B
routerDirect.js	1.09 MB	1.09 MB	⚠️ +52 B
withRouter.js	1.09 MB	1.09 MB	⚠️ +52 B
Overall change	5.4 MB	5.4 MB	⚠️ +260 B

Commit: f343242

lcswillems · 2020-10-04T09:11:34Z

@mhhegazy On the withApollo function you give, where is appProps defined?

mhhegazy · 2020-10-04T09:40:44Z

@lcswillems here

let appProps = {};
if (WrappedApp.getInitialProps) {
  appProps = await WrappedApp.getInitialProps(appContext);
}

the full code of working example that use this pull request is next-apollo-client-ssr-example

mhhegazy · 2020-10-05T13:54:27Z

@lcswillems did you test it?
does it solve your problem?

lcswillems · 2020-10-05T15:22:49Z

@mhhegazy I have just tested your example but it is not obvious it solves the issues: example is too complicated for what it should be and doesn't show why there is not this issue anymore.

Moreover, I think you should create a CodeSandBox as I did in my issue because it is much easier to test and verify it is correct.

timneutkens

This does not fix #9336 given that it affects client-side navigation only. Besides that AppTree is meant to not be changed based on properties as is introduced here so I'll close this PR.

You'll want to look into client/index.tsx's wrapApp function which is where the actual issue will likely be.

add WrapApp prop to AppTree to work in HOC with apollo

f343242

mhhegazy requested review from Timer, ijjk, lfades and timneutkens as code owners October 4, 2020 02:10

ijjk added the type: next label Oct 4, 2020

timneutkens requested changes Jan 23, 2021

View reviewed changes

timneutkens closed this Jan 23, 2021

vercel locked as resolved and limited conversation to collaborators Jan 29, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add WrapApp prop to AppTree to work in HOC and with apollo #17598

Add WrapApp prop to AppTree to work in HOC and with apollo #17598

Uh oh!

mhhegazy commented Oct 4, 2020

Uh oh!

ijjk commented Oct 4, 2020

Uh oh!

lcswillems commented Oct 4, 2020

Uh oh!

mhhegazy commented Oct 4, 2020 •

edited

Loading

Uh oh!

mhhegazy commented Oct 5, 2020

Uh oh!

lcswillems commented Oct 5, 2020

Uh oh!

timneutkens left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Add WrapApp prop to AppTree to work in HOC and with apollo #17598

Add WrapApp prop to AppTree to work in HOC and with apollo #17598

Uh oh!

Conversation

mhhegazy commented Oct 4, 2020

Proplem

Solution

working example:

Uh oh!

ijjk commented Oct 4, 2020

Stats from current PR

Uh oh!

lcswillems commented Oct 4, 2020

Uh oh!

mhhegazy commented Oct 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mhhegazy commented Oct 5, 2020

Uh oh!

lcswillems commented Oct 5, 2020

Uh oh!

timneutkens left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mhhegazy commented Oct 4, 2020 •

edited

Loading