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

with-apollo & with-apollo-auth #8691

Closed
rheaditi opened this issue Sep 10, 2019 · 8 comments
Closed

with-apollo & with-apollo-auth #8691

rheaditi opened this issue Sep 10, 2019 · 8 comments

Comments

@rheaditi
Copy link

rheaditi commented Sep 10, 2019

with-apollo & with-apollo-auth example - request

Hey!

So we're using Next.js with Apollo & are updating our app with the new example in with-apollo[-auth] & want to be able to use it in a default on + opt-out way rather than default off + opt-in way.

Is your feature request related to a problem? Please describe.

Currently, the examples are all in an opt-in mode - meaning if you need apollo, you will need to wrap your page component with withApollo explicitly. Similarly for the with-apollo-auth example, the pages add their getInitialProps for each page that needs to do something with user context.

Describe the solution you'd like

In our case, a lot more of the pages will need apollo + auth rather than not - eg maybe just 2 pages will not need auth and all other pages will; and almost ALL pages will need apollo. So we're looking for a way to go back to the _app wrapping example but we're unable to use the current withApollo implementation with _app.

Describe alternatives you've considered

In order to use the same example with _app, we tried changing the return pageProps in all places to { pageProps }, but we're getting a bunch of errors regarding circular structure in JSON. Looks like App does get passed the same AppTree component we're using but could not find any documentation regarding this AppTree and slightly stuck here.

Any help would be appreciated. Thanks :)

@huv1k
Copy link
Contributor

huv1k commented Sep 10, 2019

Hey @rheaditi, you can check old examples in the GitHub history. We changed this example to reflect our prioritization of automatic prerendering.

People were blindly copying this example without knowing of its downsides. This reflected that all pages were not automatically exported and getInitialProps was executed even that page was static. This resulted in bad use practice, which is not our intention.

@rheaditi
Copy link
Author

rheaditi commented Sep 10, 2019

Thanks for the response @huv1k!

Completely understand. Tried the older example & it seems to work.

Do you think there's a possible middle ground?
Some pros of the new way was each page can provide its own independent initial state, independent Provider and pages which do not need apollo can still exist..

Something along the lines of using the same withApollo which works with pages, but wrap the page during runtime in the _app component based on a static property on the Page component to be rendered.

So if one sets a static property on the page, PageA.apollo === false, it won't be wrapped with the provider & if PageA.ssr === false it'll only be client-side rendered..

But in order to achieve that we have to add getInitialProps to _app & that means all pages will be server rendered (no-prerender), and we'll have to maintain some state in _app so we don't create a new component in each render.

A bit stuck here. 🤔

Will try something out to achieve this..

@huv1k
Copy link
Contributor

huv1k commented Sep 10, 2019

We are right now experimenting with a prerendering option on-page levels. This would allow prerender page even that there getInitialProps in _app, but this is an advanced use case.

In examples, we want to stick to best practices so end users always get cached pages, when need.

@tstenel
Copy link

tstenel commented Sep 11, 2019

Hi I am experiencing the same issue, but not for the same use case, I want to stick to best practices and use cached pages, but I also need to set some graphql subscription before loading the page.
I have an initiator used to set a subscription that needs to be initiated before any pageComponent loading, depending on API authentication, but the last update made it so that I can't use my initiator where it was placed before (between the components in _app.js).
I tried putting it in withApollo.js between the apolloProvider & the PageComponents, but it leads to invariant violations Error while running getDataFromTree { Invariant Violation: Element type is invalid: expected a string (for built-in components) or a class/function (for composite components) but got: undefined. You likely forgot to export your component from the file it's defined in, or you might have mixed up default and named imports (I checked every component's exports, and they're all correct)

@rheaditi
Copy link
Author

We are right now experimenting with a prerendering option on-page levels. This would allow prerender page even that there getInitialProps in _app, but this is an advanced use case.

In examples, we want to stick to best practices so end users always get cached pages, when need.

@huv1k
Sure! Sounds good. 🙂 Also per-page pre-rendering options sound great!

Just want your opinion on the situation - how do you guys suggest handling this currently:

  • only certain pages will require auth via apollo, and those pages will have a particular layout (which is usually done via <App />, so we need an apollo provider at the app level)
  • some other pages will be static & if we add the provider at the app level it will turn off pre-rendering.

ps: this is not related to the changing the current examples so let me know if I should ask this elsewhere 😅

@huv1k
Copy link
Contributor

huv1k commented Sep 17, 2019

Hey @rheaditi, yes for questions we have spectrum, so please ask your questions there next time. I will close this issue because it's not how we want to examples looks like. Regarding your questions:

  1. You can use HoC component for auth, this component could have your Layout for these pages.
  2. Yes, if you add top-level getInitialProps in your page/app your pages lose static option until we introduce prerendering during build time.

@huv1k huv1k closed this as completed Sep 17, 2019
@huv1k
Copy link
Contributor

huv1k commented Sep 17, 2019

@tstenel please create an issue with propper reproduction so we can take a look.

@balazsorban44
Copy link
Member

This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@vercel vercel locked as resolved and limited conversation to collaborators Jan 31, 2022
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

4 participants