Simplify initialState for server-side rendering

[sedubois]

It looks like the Redux initialState for SSR should be something like:

  const initialState = {
    ...client.store.getState(),
    [client.reduxRootKey]: client.getInitialState()
  };

Which still looks more complex than it should IMHO? Would it be possible to simplify this API as follows?:

  const initialState = client.getInitialState();

[calebmer]

Unfortunately not if the user provides their own selector.

[sedubois]

Why would that not be possible? The client knows what the selector is, through its client.reduxRootKey property.

[calebmer]

The ApolloClient may take a reduxRootKey option, or a reduxRootSelector option. If we are passed a reduxRootKey then we may able to reverse engineer what the initial state should look like. However, if the user provides a reduxRootSelector function then we effectively only have a “getter” for the location in the store where the state is kept.

[sedubois]

OK that's sad, I'll come back to my example app to explain what I thought would be a simpler and more intuitive API. I actually thought I could simply use store.getState() as below:

      return {
        initialState: reduxStore.getState(),
        ...
      }

However, folks over at next.js advised against it (discussion here vercel/next.js#780 (comment), pointing at this Apollo comment: apollographql/apollo-client#759 (comment)).

So instead I did the following:

      return {
        initialState: {
          ...state,
          [apolloClient.reduxRootKey]: apolloClient.getInitialState(),
        },
        ...
      };

But that's quite sad, because the first snippet above is quite cleaner, and actually worked for me so far! Would there not be a way to make it "official" so that it could be reliably used? In the linked discussion, @stubailo said: "we should make that happen automatically when using react-apollo, and document it better."

[calebmer]

Going off of the issues you linked, the correct way to write this would be a little more complex 😊

      return {
        initialState: {
          ...state,
          [apolloClient.reduxRootKey]: {
            data: apolloClient.getInitialState().data,
          },
        },
        ...
      };

This would avoid some of the bugs mentioned in the issues you linked.

I don’t think a little complexity at this point hurts, the real problem is there is a lack of documentation. We will be refactoring the Redux store soon, so hopefully, this will get cleaner in the future. For now, I’m going to close.

[sedubois]

Yes at least lack of documentation, and there should be clear error messages if the user isn't using the Apollo API correctly.

But I still do think that this complexity hurts, as you can see from the comments of people in the linked comments that you mentioned. Can lead to hidden issues with weird symptoms and code is harder to understand than it should be.

A good API should be as simple as possible, as well documented as possible, and not error-prone.

[helfer]

A good API should be as simple as possible, as well documented as possible, and not error-prone.

I think we can all agree about that, but the API also needs to let you get things done. Providing a store and specifying which part apollo should write to is a requirement, so I'd love to hear suggestions and see PRs for improving the initialState API while preserving the option to provide a redux selector.

[stubailo]

Also, this is something you only have to do once for your entire app, right? It's not like people have to write this initial state code every day.

[sedubois]

@stubailo yes of course I'm not saying this is anything critical.

I don't know the details as I'm not an Apollo developer. But using reduxRootKey, I need to define this function myself:

function getInitialState(apolloClient, reduxStore) {
  return {
    ...reduxStore.getState(),
    [apolloClient.reduxRootKey]: {
      data: apolloClient.getInitialState().data,
    },
  };
}

I guess there must be some equivalent kind of function for the case when people use reduxRootSelector (I'm not familiar with that one). (And those two functions could be combined into a single one.)

I'm basically just saying that such a function could be provided as a helper within the Apollo codebase, instead of having people needing to skim through confusing docs and rebuild it themselves. But I might be naive as I don't have the whole picture about reduxRootSelector, maybe it's not possible for some reason, then please just close the issue.

[leebenson]

Apologies for posting in a closed issue, but FWIW my client.reduxRootKey is undefined - is that normal?

[sedubois]

@leebenson I think that option has been deprecated, then removed altogether, but don't know if that fact has been documented anywhere.

There's a link in the Slack group here, though: vercel/next.js#1394 (comment)

[leebenson]

thanks @sedubois. The docs definitely haven't been updated yet.

I solved this in my RelayQL starter kit by creating a separate Redux store, and specifying the root key manually using reduxRootSelector: state => state.apollo.