Need to manually re-call getInitialProps of sub-components

[sedubois]

As described here: #159 (comment) and as far as I understand, if some React component has a getInitialProps, then all of its parents are condemned to need it too, and each parent in turn needs to call getInitialProps on their children, in the right order, making sure not to duplicate the work between the different levels of ancestry (the linked issue shows the case of a higher-component style, but the same applies when passing components as React children the normal way).

Forgetting one of these calls can lead to silent bugs such as not having server-side rendering, and this code repetition can hinder proper componentization (one of React's key patterns).

From an end-user perspective, just like with React component lifecycle methods such as componentWillMount etc, IMHO the developer should trust that getInitialProps will be called no matter what, as long as it's spelled properly.

Could Next.js handle getInitialProps as part of the React component lifecycle or implement the same kind of magic to make sure it gets called whenever defined in a component?

I'm new to SSR and Next.js, so apologies if this makes no sense 😄

[richardnias]

I think this is mainly an issue when using a HOC to wrap page components, e.g. for adding a navigation bar or site wide footer etc. Perhaps if we went down the route being discussed in #88, where a user could supply a global component (_layout.js for want of a better name), then this component could perhaps extend a special version of React.Component that automatically calls getInitialProps of the child component(s)? Whether this would work depends on how exactly the issues described in #88 are solved in the end.

[nkzawa]

You can write your page components with HOC like the following:

import React from 'react'
import hoc from '../components/Hoc'

const Component = hoc((props) => <div>{props.value}</div>)
Component.getInitialProps = () => {
  return { value: 'my value' }
}

export default Component

I think there is no much difference whether if you use HOC or not.

[sedubois]

I agree, but if it's a sub-component which is wrapped in a HOC (in my case, an Apollo HOC), then this page will have to make sure to call it from here, otherwise the effects are unpredictable. It took me time to figure out these subtle bugs, I wouldn't wish every newcomer to have to figure it out the same way.

For example here is my current working code, it already uses two HOCs,page and apollo:
https://github.com/sedubois/realate/blob/master/pages/discover.js

If I decide to change discover.js to the following:

import React from 'react';
import Users from '../containers/Users';
import page from '../components/Page';
const Discover = () => (
  <div>
    <Users />
    <div>other fancy stuff</div>
  </div>
);
export default page(Discover);

then I will get a cryptic error ReferenceError: window is not defined thrown at containers/Apollo.js:38 (because the WrapWithApollo constructor will be called directly without having first called its getInitialProps). It takes quite some time to figure out that getInitialProps should be added to Discover and should in turn call getInitialProps on Users. Besides being hard to debug, it also adds unnecessary complexity for the end-user.

Don't get me wrong, I think Next.js is awesome 😄 but this makes it a bit hard to scale and leads to frustration.

[nkzawa]

I agree that it'll be hard to figure out the bug.

I think It'd be cool if we can display warning whengetInitialProps was detected on sub-components. Actually, I'm not sure if it's possible to do though.

[sedubois]

That'd be cool, and I'd say if we can auto-detect, then we could as well call these getInitialProps instead of forcing the user to explicitly call them. I won't pretend that I know how though 😄 but I guess it should be possible as React seems to do something like that with their component lifecycle methods?

[amccloud]

@nkzawa what are your thoughts on my proposal here? #186

[dstreet]

@nkzawa you could probably traverse the component tree prior to calling getInitialProps on the root component to detect any children that define it (or not), but I'm not sure what value that would add and could easily become a very expensive operation.

[dbismut]

I don't know much and haven't tried next.js yet, but if I remember correctly react-redux-universal-hot-example was using a fetchData method to fetch data asynchronously from its components. At some point the whole component tree was traversed to detect any fetchData method anywhere in the tree. This in-house fetchData has been replaced with a package called asyncConnect which uses redux.

It seems like https://github.com/ericclemmons/react-resolver is doing something similar though without redux.

Don't know if this helps really but we never know.

[sedubois]

@dbismut redux-connect (forked from redux-async-connect) looks promising 👍 However that requires a Redux store, and although I think that's great, I'm not sure the Next.js team would have the same opinion? @nkzawa

[dbismut]

@sedubois that's why react-resolver could be a more agnostic way to go.

[sedubois]

@dbismut thanks yes, I didn't pay enough attention, I thought the project was inactive as no commit since 3 months. We should try to build an example.

[nkzawa]

I checked how react-resolver works but it requires to render a component twice on server rendering, which would be expensive and introduce unexpected side effects.

We'd have to do the same for supporting getInitialProps of descendant components, and I think it's unacceptable. getInitialProps is required only for top-level components because no one can provide props to them without a provider. So basically, I think having getInitialProps or react-resolver to descendant components wouldn't make sense.

Maybe we can still display warnings by traversing the component tree after rendering (not before getInitialProps call since we need props to render the tree), and these expensive operations would be acceptable if it's only for development.

[sedubois]

So basically, I think having getInitialProps or react-resolver to descendant components wouldn't make sense

@nkzawa I might have misunderstood, but are you saying that we shouldn't be using getInitialProps in any components except the pages/? But what about the components included from there?

For example, how would I bind my Apollo HOCs, without getInitialProps? (NB: they are already two levels down in my component hierarchy.)

I'd want to ensure colocation, not having all async code gathering up in fat components at the root of the component hierarchy.

[nkzawa]

nope, I meant next.js shouldn't try to handle getInitialProps defined in descendants.
I think your HOC would become a top level component, or the getInitialProps would be manually called on top level one, which is no problem.

So it seems we shouldn't add warnings too, if people'd like to merge getInitialProps from wrapped components.

[sedubois]

@nkzawa as highlighted in previous comments and e.g in @luisrudge's implementation (improved by @impronunciable) of an authentication flow, (see commit), not only are we bound to repeatedly forget calling the child getInitilaProps, but it also makes the code much less clean and more verbose. I'm not saying I have a clean alternative to propose at this point, but it really is lots of time lost for newcomers like me. Minimalism is good but IMHO it should enhance the DX, not hinder it.

[sedubois]

@nkzawa Manually handling children's getInitialProps in the parent also means that the child data needs to be assembled and stored within the parent, only to then be re-provided to the children (for example see #186 (comment)). Would be great to get rid of this data roundtrip between children and parent (is also bug-prone).

If somehow feasible, the above example could be rewritten as:

import connectInitialProps from 'next/connect';

const Child = ({ name }) => <div>{name}</div>;
const Child1 = connectProps(() => ({ name: 'Child1' }))(Child);
const Child2 = connectProps(() => ({ name: 'Child2' }))(Child);

export default () => (
  <div>
    <Child1 />
    <Child2 />
  </div>
);

AFAIK Redux and Apollo allow that kind of thing thanks to their store provider. If Next doesn't want to force the use of a store, maybe we could find a middle ground where Next supports getting data from a store if one exists, otherwise works as before? (thinking out loud here)

[nickredmark]

I agree with @sedubois here, the need for getting data deep in the component hierarchy isn't easily dismissed. Imagine for example a list of items, one of which should be "expanded" with more data. In the top getInitialProps you fetch the data for the list, and in the individual component's getInitialProps you'd check whether the component is expanded and load more data if needed.

[amccloud]

@nmaro the "ListItem" component could be passed additional props when it is expanded. With the addition of React's context you have everything you need to pass the data down.

[nickredmark]

True @amccloud, but I see the following problems with this approach:

1. Polluting react's context with data is discouraged
2. More importantly, you loose modularity: the top component has to know all the data it will need to load for all the subcomponents. For every change of a component's data needs, one needs to change the data subscriptions in all pages that use that component. - in other words you end up reproducing your rendering tree in the data loading logic, which is what you could avoid if next.js goes through the tree anyway.

I mean, if (server-side-)rendering complex apps with modular data dependencies is out of the scope of next.js that's also fine, one just needs to say it. Right now I don't see a good pattern to handle this case.

[amccloud]

@nkzawa please correct me if I'm wrong:

getInitialProps is only for the initial server side render. It is the only way to get request/env data into the render tree. If necessary, anything derived from that request data could be passed to deeper through props or context. For your typical data fetching needs, you are free to use tools like redux, relay, and react-apollo to connect to any of your components directly to a data source.

[nickredmark]

@amccloud do you mean just passing down some context data, and then fetching the actual data down in the hierarchy? I don't think it's possible, of course would be great to hear otherwise.

I believe all the async/data-fetching operations on the server have to happen in getInitialProps - which I think is the problem.

Page > ... > ... > ... > ... > Component

Imagine Component needs some graphql data with a query defined specifically for that component.
The problem is that you need to load Page.getInitialProps and then pass the data down to that Component.

[timothyarmes]

Hi. I'm following the development of next with great interest and I wanted to chime in on this issue.

In my opinion, architecting even a moderately complex web application while having to hoist each pages's data to the root component will be very hard to handle cleanly. If a solution can be found then it would be much better.

Just off the top of my head, could each component simply await on data in the componentWillMount hook, therefore blocking the entire rendering process until all the data is available?

Clearly we would only want to do this on the server, so perhaps an API can be exposed that components can use.

[nkzawa]

@amccloud getInitialProps is not only for server side rendering (called on client too). Others are correct I think.

[nickredmark]

Anyway, this issue shows that it really is a problem with react, not next.js: facebook/react#1739

[timothyarmes]

@nkzawa I didn't mean to imply that getInitialProps is only called on the server. What I'm suggesting is that components could simply block the rendering process using an await while waiting for data on the server, so that renderToString returns with the fully rendered component.

On the client we wouldn't want to use an await. We want do render everything that we can and update state/props once the data's available.

next could provide an API to make that more seamless.

[nkzawa]

@timothyarmes Unfortunately, you can't do await on componentWillMount or any other life cycle methods. So as @nmaro pointed out, it's rather the problem with react. I think it's hard to solve the issue unless React changed the behavior.

[sedubois]

Can we still explore using a store such as Redux as react-resolver does (#192 (comment))?

I'd love to have @arunoda's opinion, considering your interest in GraphQL. IMHO we need to properly solve this, otherwise Next won't be able to scale for all projects with data-connected components.

[sedubois]

How about @VanCoding's solution in facebook/react#1739 (comment)? See code.

[nickredmark]

This issue seems trickier than it sounded. I'm curious about the developments around this, especially on the react side, but I believe that for the time being I'm going to settle for this pattern:

• load the main expected data in the root getInitialProps for ssr
• and then secondary data only client-side

[timothyarmes]

@nkzawa what happens if you try? Surely it should just look like a long synchronous call as far as React is concerned.

[amccloud]

@timothyarmes using await in componentWillMount will cause it to return a Promise. React will not wait for the promise to resolve.

[timothyarmes]

Yeah, I wasn't thinking clearly. To use an await the function has to be declared async, and clearly we can't do that.

[reel]

Made a simple repo with a solution to this issue:

https://github.com/reel/nextjs-initialprops-back

[sedubois]

@reel seems like you're still re-calling getInitialProps manually from the parent, so that doesn't solve the described issue.

I'm waiting for feedback from @nkzawa in #387, that's the best hope so far.

[reel]

Well, in the example, the parent does not handle the state. What I'm trying to achieve is bootstrap some stuff in the Application (in another project) and let the child handle data fetching (and injection with feathersjs)

[nkzawa]

@sedubois which part of #387 do you mean ?

[sedubois]

@nkzawa this issue now seems to be fixed! My need was for Apollo components and as pointed out by @ads1018 in #387 (comment), everything can be set up from the page root so that deeper components don't need any getInitialProps 🎉.

I updated my app: https://github.com/RelateNow/relate

This might be closed I think.

[sedubois]

From this week's React Conf, after React Fiber (React 16) is out:

Means async React lifecycle methods could be coming? Maybe that would make getIntialProps redundant?

facebook/react#1739 (comment)