React-apollo: Possibly unnecessary component updates after mount

[Peleg]

Hey guys!

First off, you rock and thank you for making react-apollo. it's made our migration to graphql a much smoother operation at a pretty large scale ;-)

Lately we found out we had a perf issue with apollo-wrapped components updating immediately after mounting despite already having their data in the redux store (we server-render all the things 🙉).

Since there don't seem to be changes in the underlying data (no client requests even made) between componentWillMount and willReceiveProps, we were wondering if those updates are necessary and how could they be avoided w/out having to shim-in a shouldComponentUpdate in all of our apollo-wrapped components.

We would love to help and submit a PR if it is agreed to be a bug and if we could be pointed in the right direction.

I created a demo repo demonstrating the issue right here: https://github.com/Peleg/apollo-demo

Steps to Reproduce

1. wrap a component with the graphql decorator,
2. render it on the server and pass it the initial state
3. mount it on the client and see how it renders twice, first time when it mounts ✅, and the second time when it updates despite not making any requests, nor updating any props ❓

Version

• [email protected]
• [email protected]

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions to React Apollo!

[tmickel]

@probot-stale[bot] a little aggressive! I'm curious to hear a quick theory here :)

would it help if we did any more digging?

[jbaxleyiii]

@Peleg Thank you for this great demo! We should definitely take a look!

@tmickel I'm sorry! The pro-bot was set way to aggressive! I'm changing that today!

[stale[bot]]

This issue has been automatically labled because it has not had recent activity. If you have not received a response from anyone, please mention the repository maintainer (most likely @jbaxleyiii). It will be closed if no further activity occurs. Thank you for your contributions to React Apollo!

[jhk2020]

Hey guys, any update on this? Running into the same issue

[chriszarate]

@Peleg I think there is a simple fix. I opened a PR against your example repo:

https://github.com/Peleg/apollo-demo/pull/1

Provide reduxRootSelector to correctly select the initial state data.

[Peleg]

@chriszarate oh interesting!

from looking at the apollo docs it looks like you don't need to specify the root selector if you're using apollo as the key tho:

If you’d like to use a different root key for the client reducer (rather than apollo), use the reduxRootSelector: selector option when creating the client

what am i missing here 🤔 ?

EDIT: nevermind, seems like an error is thrown instead, which is the reason the second render is not happening:

Uncaught Error: Cannot initialize the store because "reduxRootSelector" is provided. reduxRootSelector should only be used when the store is created outside of the client. This may lead to unexpected results when querying the store internally. Please remove that option from ApolloClient constructor.

[chriszarate]

@Peleg You're right, very sorry for the misdirection. Not sure how I missed that.

[stale[bot]]

This issue has been automatically labled because it has not had recent activity. If you have not received a response from anyone, please mention the repository maintainer (most likely @jbaxleyiii). It will be closed if no further activity occurs. Thank you for your contributions to React Apollo!

[stale[bot]]

This issue has been automatically closed because it has not had recent activity after being marked as no recent activyt. If you belive this issue is still a problem or should be reopened, please reopen it! Thank you for your contributions to React Apollo!

[tmickel]

@jbaxleyiii can stalebot be turned off on certain issues that might take a long time to investigate? :)