React-apollo: Support Context

[ghost]

You are already supporting props and state in the mapQueriesToProps function, but what about context?

Its useful to pass down a varibale from the root to a child without needing to pass the variable as prop into every component.

ROOT > MainLayout > Sidebar > Header > MYCOMPONENT

[stubailo]

Can you give an example of what you're passing down? If it's some state from the ROOT, then it's probably a better idea to put that in Redux, no?

[ghost]

For example global settings like default limit. I could store it in redux, but I just wanted to try out this new feature.

[stubailo]

If the data isn't changing, then I'd just make a variable and import it. If it's changing I'd put it in Redux... either way context seems to be on the path to big changes: https://facebook.github.io/react/docs/context.html

I dunno I just don't think there is a real need for this, and I don't really think it's good to encourage people to use context in this way. Open to comments from other people though.

[ghost]

Yeah you are right, Redux is probably better. But for example material ui is using Context for setting the Custom Themes. In a scenario where you want to request a different icon if its a dark theme, you would want to get it via context.

I don't think many people need it, but i think it should be added to make the transition to apollo smoother: You don't have to change the app to redux, before switching to apollo.

[stubailo]

OK - curious to see what @jbaxleyiii thinks.

[jbaxleyiii]

@Krabaul @stubailo while I agree that using context is dangerous, it is a supported feature of react. We don't block context for usage in children component scurrently, the question is should we expose context to the mapQueriesToProps.

Because we are using an object as the argument, I see no reason to not add it in personally. I need to see how much of a perf loss we have by tracking context changes before I'm 100% though. We are already tracking props and the queries themselves, I don't want to expose unnecessary re-renders.

I'll play around with it tonight and see what effect it has

[jbaxleyiii]

Actually, I think getting your context from a parent component and passing it as a prop is a better way to go. I don't think redux exposes context to its map functions either so I'd rather not since passing it as a prop is so easy. @Krabaul will that work for you?

Still on the fence. One use case for context that I could realistically see is I18n support. Specifying your local / language on the context if you aren't using redux seems like the right call to me. However, the above is still true that wrapping your querying component and passing the prop isn't hard, but it does add boilerplate

[jbaxleyiii]

At this point, I think using a wrapped container that manually specifies the context and passes it to react-apollo as a prop is the way I would recommend. I'm open for further discussion but I think keeping the api and watched data limited is the right call for now