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

Use getApolloState instead relying on reduxRootKey #226

Merged
merged 1 commit into from Sep 27, 2016

Conversation

Projects
None yet
4 participants
@sheerun
Copy link
Contributor

sheerun commented Sep 22, 2016

I hope I got it well. It may or may not require major bump.

I'd be really glad if you released it asap, as I use my own redux store.

@meteor-bot

This comment has been minimized.

Copy link

meteor-bot commented Sep 22, 2016

@sheerun: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/

@sheerun sheerun force-pushed the sheerun:redux-selector branch 4 times, most recently from 5318cd9 to 0540e54 Sep 22, 2016

@sheerun

This comment has been minimized.

Copy link
Contributor Author

sheerun commented Sep 22, 2016

This is simpler and more accurate implementation of #223, should resolve #221

@sheerun sheerun force-pushed the sheerun:redux-selector branch from 0540e54 to 547d921 Sep 22, 2016

@sheerun sheerun force-pushed the sheerun:redux-selector branch from 547d921 to c1c04af Sep 22, 2016

@sheerun

This comment has been minimized.

Copy link
Contributor Author

sheerun commented Sep 22, 2016

Actually it shoudn't require major bump as queryManager.getApolloState() was available in previous versions.

@stubailo

This comment has been minimized.

Copy link
Member

stubailo commented Sep 22, 2016

I'm a little uncomfortable because this isn't strictly a "public API" but it definitely works for now! We'll just need to be careful to not remove that function, or rename queryManager.

@sheerun

This comment has been minimized.

Copy link
Contributor Author

sheerun commented Sep 22, 2016

CI fails for non-related reasons.. I guess if you figure out public API for getting apollo state, you can change it. At least it's less hacky than manually fetching it by reduxRootSelector (that can be both string or function)... and backward compatible.

@stubailo

This comment has been minimized.

Copy link
Member

stubailo commented Sep 22, 2016

Yeah I think this is a great solution right now, let's do it!

@sheerun

This comment has been minimized.

Copy link
Contributor Author

sheerun commented Sep 26, 2016

@stubailo Any chance to release it soon? :)

@jbaxleyiii

This comment has been minimized.

Copy link
Member

jbaxleyiii commented Sep 27, 2016

@sheerun I'll get this out today!

@jbaxleyiii jbaxleyiii merged commit 4baa0f4 into apollographql:master Sep 27, 2016

2 of 3 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
CLA Author has signed the Meteor CLA.
Details
coverage/coveralls Coverage decreased (-0.02%) to 96.593%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment