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

Update with-apollo example with next-apollo package. #3221

Closed
wants to merge 2 commits into from

Conversation

adamsoffer
Copy link
Contributor

@adamsoffer adamsoffer commented Nov 1, 2017

I've abstracted the withData higher order component into its own package. This pull request removes the boilerplate inside the with-apollo example lib directory in favor of this package.

Feedback welcome.

Note: I left the other two apollo examples as is (with-apollo-and-redux and with-apollo-auth), but I'd love to see those examples get updated to Apollo Client 2.0 and using the same abstraction if someone wants to submit a pull request for those as well 🙂

@flippidippi
Copy link

flippidippi commented Nov 1, 2017

I think there's a whole lot of use cases to not use a package for something like this and I'm not sure it should be the example. Having the boilerplate code allows the user to learn more about what's going on, setup custom auth in the wrapper, customize redux further, and add hooks to apollo. I think the example should lean more towards teaching how something works rather than "install this".

I think keeping the current example with a note in the README about this package would make more sense.

@jthegedus
Copy link
Contributor

@ads1018 With a with-next-apollo example you could explain to users under which use cases this would be more beneficial to use. As @flipxfx pointed out there are certainly times in which you would not want to use next-apollo.

@adamsoffer
Copy link
Contributor Author

adamsoffer commented Nov 2, 2017

Yeah I can see both sides. The goal is for the package to eventually support those use cases (custom auth, redux integration). Until it supports them, I agree that mentioning it in the README is probably a good idea for now. I do think, ultimately, the community would benefit greatly if we use the package since it's much more user friendly. There's a lot of heavy lifting going on in this boilerplate and perhaps the teaching moment should happen at the package level.

@adamsoffer
Copy link
Contributor Author

Closing until next-apollo package covers all use cases.

@adamsoffer adamsoffer closed this Nov 15, 2017
@lock lock bot locked as resolved and limited conversation to collaborators May 15, 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

Successfully merging this pull request may close these issues.

3 participants