Make withApollo work with _app.js components #8801
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
HaNdTriX
force-pushed
the
examples/apollo-app-js
branch
2 times, most recently
from
32e50ad to
c755afa
Compare
Stats from current PRDefault Server Mode (Decrease detected ✓)General Overall decrease ✓
Client Bundles (main, webpack, commons)
Client Bundles (main, webpack, commons) Modern
Client Pages
Client Pages Modern
Client Build Manifests
Rendered Page Sizes
Serverless Mode (Decrease detected ✓)General Overall decrease ✓
Client Bundles (main, webpack, commons)
Client Bundles (main, webpack, commons) Modern
Client Pages
Client Pages Modern
Client Build Manifests
Serverless bundles
Commit: 9297526 |
Stats from current PRDefault Server ModeGeneral
Client Bundles (main, webpack, commons)
Client Bundles (main, webpack, commons) Modern
Client Pages
Client Pages Modern
Client Build Manifests
Rendered Page Sizes
Serverless ModeGeneral
Client Bundles (main, webpack, commons)
Client Bundles (main, webpack, commons) Modern
Client Pages
Client Pages Modern
Client Build Manifests
Serverless bundles
Commit: c755afa |
HaNdTriX
force-pushed
the
examples/apollo-app-js
branch
from
c755afa to
a4b02bd
Compare
Stats from current PRDefault Server ModeGeneral
Client Bundles (main, webpack, commons)
Client Bundles (main, webpack, commons) Modern
Client Pages
Client Pages Modern
Client Build Manifests
Rendered Page Sizes
Serverless ModeGeneral
Client Bundles (main, webpack, commons)
Client Bundles (main, webpack, commons) Modern
Client Pages
Client Pages Modern
Client Build Manifests
Serverless bundles
Commit: a4b02bd |
HaNdTriX
force-pushed
the
examples/apollo-app-js
branch
from
a4b02bd to
18f7000
Compare
Stats from current PRDefault Server ModeGeneral
Client Bundles (main, webpack, commons)
Client Bundles (main, webpack, commons) Modern
Client Pages
Client Pages Modern
Client Build Manifests
Rendered Page Sizes
Serverless ModeGeneral
Client Bundles (main, webpack, commons)
Client Bundles (main, webpack, commons) Modern
Client Pages
Client Pages Modern
Client Build Manifests
Serverless bundles
Commit: 18f7000 |
|
@HaNdTriX I'm still having trouble finding out how is this useful. We don't want users to use Apollo in Both the related issues can be solved and are better without |
|
@lfades - one use case I can see for this is accessing data inside a layout component. Persistent layout components are defined outside of pages and thus are not able to query data via apollo. This is actually an issue I'm running into now. What's the escape hatch here other than using Apollo in |
|
@adamsoffer I have to admit that I agree with @ifades. Feel free to use the following code: It allows you to use Have a nice day! |
|
@HaNdTriX I definitely agree too. But there's definitely a use case (as described above) that it doesn't cover. Say I have a sidebar component as part of my persistent layout defined outsides of pages, and I need data in that sidebar, what are my options besides disabling project wide automatic static optimization in _app.js? |
|
@HaNdTriX that works thanks. In my case, I am using |
|
I really would like that someone who defends not using Apollo in |
|
@matepaiva I recommend not using Check out the following example for an alternative: |
|
@HaNdTriX that example doesn't exhibit a persistent layout. See this example -> https://adamwathan.me/2019/10/17/persistent-layout-patterns-in-nextjs/ In nextjs, at least at the moment, you have to use _app.js to implement persistent layouts. |
yeah, totally agree
this leads to tons uf uneeded re-renderings, doesn't it? |
|
so where do we go from here? it seems like either we have to sacrifice performance or we have to add a ton of boilerplate. I don't care about It's perfectly valid to have one |
|
@macrozone feel free to use the following code: https://github.com/HaNdTriX/next.js/blob/examples/apollo-app-js/examples/with-apollo/lib/apollo.js It supports apollo on app level. You might have to maintain it on your own, since I prefer a little boilerplate over flexibility. In the end we are talking about three explicit lines of code :). |
HaNdTriX
force-pushed
the
examples/apollo-app-js
branch
from
18f7000 to
446ea16
Compare
Stats from current PRDefault Server ModeGeneral
Client Bundles (main, webpack, commons)
Client Bundles (main, webpack, commons) Modern
Legacy Client Bundles (polyfills)
Client Pages
Client Pages Modern
Client Build Manifests
Rendered Page Sizes
Serverless ModeGeneral
Client Bundles (main, webpack, commons)
Client Bundles (main, webpack, commons) Modern
Legacy Client Bundles (polyfills)
Client Pages
Client Pages Modern
Client Build Manifests
Serverless bundles
Commit: 446ea16 |
HaNdTriX
force-pushed
the
examples/apollo-app-js
branch
from
446ea16 to
53834c6
Compare
Stats from current PRDefault Server ModeGeneral
Client Bundles (main, webpack, commons)
Client Bundles (main, webpack, commons) Modern
Legacy Client Bundles (polyfills)
Client Pages
Client Pages Modern
Client Build Manifests
Rendered Page Sizes
Serverless ModeGeneral
Client Bundles (main, webpack, commons)
Client Bundles (main, webpack, commons) Modern
Legacy Client Bundles (polyfills)
Client Pages
Client Pages Modern
Client Build Manifests
Serverless bundles
Commit: 53834c6 |
|
@Timer since you have reopend my PR I have updated it to work with the latest changes in the Next.js world:
This gives us all the flexibility we need. The HOC now supports:
Status: ready to merge |
HaNdTriX
force-pushed
the
examples/apollo-app-js
branch
from
53834c6 to
7c93b73
Compare
Stats from current PRDefault Server ModeGeneral
Client Bundles (main, webpack, commons)
Client Bundles (main, webpack, commons) Modern
Legacy Client Bundles (polyfills)
Client Pages
Client Pages Modern
Client Build Manifests
Rendered Page Sizes
Serverless ModeGeneral
Client Bundles (main, webpack, commons)
Client Bundles (main, webpack, commons) Modern
Legacy Client Bundles (polyfills)
Client Pages
Client Pages Modern
Client Build Manifests
Serverless bundles
Commit: 7c93b73 |
HaNdTriX
force-pushed
the
examples/apollo-app-js
branch
from
7c93b73 to
205014f
Compare
Stats from current PRDefault Server ModeGeneral
Client Bundles (main, webpack, commons)
Client Bundles (main, webpack, commons) Modern
Legacy Client Bundles (polyfills)
Client Pages
Client Pages Modern
Client Build Manifests
Rendered Page Sizes
Serverless ModeGeneral
Client Bundles (main, webpack, commons)
Client Bundles (main, webpack, commons) Modern
Legacy Client Bundles (polyfills)
Client Pages
Client Pages Modern
Client Build Manifests
Serverless bundles
Commit: 205014f |
chibicode
pushed a commit
to chibicode/next.js
that referenced
this pull request
Feb 11, 2020
* Make withApollo work with _app.js components * Support wrapping functional _App * Add apolloClient to NextPageContext & NextPageContext * Propertly call App.getInitialProps if used in NextAppContext * Add Automatic Static Optimization warning
|
@adamsoffer you had any luck getting data/withApollo to your persistent layout components? We're running in to the exact problem you outlined above. |
This was referenced Mar 13, 2020
This was referenced Mar 22, 2020
|
@HaNdTriX Just wanted to let you know that I based my implementation off your example and it saved me a ton of time seeing the different ctx structures spelled out. Appreciate you sharing it. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
Since we changed the
withApolloHOC, there have been a lot of requests, regarding using the old pattern (wrapping_app.js). Even though it disables project wide automatic static optimization.Since it is quite easy to detect if someone wraps an
AppComponentor anPageComponentwe should allow both methods. In addition to that it is much easier to migrate between both approaches if the HOC supports both.This PR allows using the
withApollohoc in both ways:A.
withApollo(PageComponent)pages/index.js
B.
withApollo(App)orwithApollo(CustomApp)Disables project wide automatic static optimization. Therefore a devonly warning is logged to the console:
pages/_app.js
Related
#8770, #8691
Added value
ssr: falseon_app.jswould allows project wide static optimizationStatus: Waiting for feedback