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

Where do Providers Go? #659

Closed
zachariahtimothy opened this issue Jan 4, 2017 · 16 comments
Closed

Where do Providers Go? #659

zachariahtimothy opened this issue Jan 4, 2017 · 16 comments

Comments

@zachariahtimothy
Copy link
Contributor

zachariahtimothy commented Jan 4, 2017

Loving the idea of NextJS so far! One thing I am not getting is how I can wrap the root app component in a provider such as ApolloProvider or IntlProvider. I have tried in _document.js but it seems that

cannot be overwritten so the IntlProvider does not propagate down the component tree.

I tried this in the Document render method:

<IntlProvider locale={locale}>
   <Main />
</IntlProvider>

Using: [email protected]
Any ideas?

UPDATE 1/10/2016: I tried @rauchg 's suggestion deciding adding multiple providers in every page was acceptable and it still does not work. I tried a variety of layout components, straight page wrapping, etc and nothing worked. After reviewing the < Main /> component code it looks like it is not passing the right props to children, or the general structural hierarchy is not sufficient. At this point I do not believe it is possible for NextJS to work with ReactIntl.

@iamjacks
Copy link

iamjacks commented Jan 5, 2017

I'm not familiar with Apollo yet, but there was some discussion at #387 about it.

@nkzawa
Copy link
Contributor

nkzawa commented Jan 5, 2017

Also check this example.

@zachariahtimothy
Copy link
Contributor Author

@nkzawa Thank you for that reference. I checked the examples but never heard of Styletron so not familiar. I tried using react intl in the same fashion but when I try to use it, I get " needs to exist in the component ancestry.". I believe this has to do with the provider being at a lower level than the main app, but I could be wrong.

@adamsoffer
Copy link
Contributor

@zachariahtimothy I'm running into the same issue trying to get Apollo to play nice with Next. The fact that we can't pass down context from _document.js is problematic. Passing it down on every individual page isn't very dry. @nkzawa @rauchg is there any change we can make to the API so that context doesn't get wiped when wrapping <Main />?

@nkzawa
Copy link
Contributor

nkzawa commented Jan 6, 2017

@ads1018 What we recommend is to wrap every page with provider.

@rauchg It seems many people wonder about this. I think we should have a document about this topic.

@adamsoffer
Copy link
Contributor

adamsoffer commented Jan 6, 2017

@nkzawa I think many of us understand that it's recommended we wrap every page with providers. The issue I'm raising is that it makes more sense to wrap our app with a provider once at the root level (preferably in _document.js) as opposed to inside every single page.

If my app had a ton of pages it'd be nicer if they all looked like this:

export default <PostList />

as opposed to this:

export default ApolloHOC()(() => <PostList />) // yuck!

What do you think?

@zachariahtimothy
Copy link
Contributor Author

Totally agreed @ads1018 . I am a fan of the DRY principle so for my larger, enterprise app, adding multiple providers to every page is a total non-starter.

@rauchg
Copy link
Member

rauchg commented Jan 6, 2017

Personally I 100% prefer explicit wrapping over "nicer looking". It just scales better (in terms of understanding and the ability of others to work on your stuff), than hidden global files.

The only reason we have _document is that React can't control the entire markup, otherwise we'd be asking you to wrap everything :D

In my experience as well, you always think that every part of your system abides by the same rules, but you end up finding later that it's not the case.

When I set out to clone HN, it would have been very tempting to create a global layout in _document for everything.

image

After all every page seems to have some simple rules, like the navigation bar, background, etc.

Except that the login page is completely different. None of the global "rules" you thought you had apply: fonts, wrapper elements, widths, mobile queries…

image

<cue argument that it looks unpolished, but that's how it is>

What did it cost me? Just wrapping my pure components in <Page> when I wanted the layout:

    return <Page>
      <Stories page={page} offset={offset} stories={stories} />
    </Page>

This, in my opinion, is a strength of this system. You're not constrained by artificial layout systems that are later hard to escape from.

This also applies to providers. We'll do our best to document it in the FAQ that we'll be putting together in the Wiki.

@rauchg rauchg closed this as completed Jan 6, 2017
@adamsoffer
Copy link
Contributor

adamsoffer commented Jan 6, 2017

Thanks for sharing. I have a better understanding of why you went this route now. Are providers purposefully getting ignored in _document.js in order to enforce this opinion? Otherwise, we could cater to both camps: those who prefer a global provider and those who prefer to explicitly include them on a per-page basis.

@rauchg
Copy link
Member

rauchg commented Jan 6, 2017

Agreed. I think we should re-open an issue to explore / document the limitations of <Document> with regards to that

@sedubois
Copy link
Contributor

@rauchg AFAIK your example above has another cost (from what I understood of @arunoda's explanation about CommonsChunkPlugin): just because one page doesn't use some modules (like navigation in your example), it forces all other pages to have their own copy of those modules.

If I have a webapp which uses Apollo on 95% of its pages, it's indeed better if the 5% of static pages don't need to load Apollo, but it would also be better for Apollo to still load only once in the other pages.

It would be awesome if CommonsChunkPlugin could also extract the modules which aren't common to all pages but still common to several of them, would that be feasible?

@arunoda
Copy link
Contributor

arunoda commented Jan 11, 2017

@sedubois yep. We need such a functionality. But currently, it's not possible.
So I created an issue here: #738

@jgautheron
Copy link

@zachariahtimothy I integrated react-intl with the help of redux, if you need an example, I'll post one gladly.

@zachariahtimothy
Copy link
Contributor Author

zachariahtimothy commented Jan 25, 2017

@jgautheron That would be tremendous!

@jgautheron
Copy link

jgautheron commented Jan 25, 2017

Here you go: https://github.com/jgautheron/nextjs2-react-intl
It still needs some refinements but it'll get you started.

@sedubois
Copy link
Contributor

I also just added i18n to my app if it helps: https://github.com/relatenow/relate

@lock lock bot locked as resolved and limited conversation to collaborators May 12, 2018
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

8 participants