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

Don't reuse options object for mutate and query #356

Merged

Conversation

Problematic
Copy link
Contributor

Starting in 0.13.1, the options map passed to queries/mutations will be reused in some cases. In the case of queries, this causes errors, as the Apollo client first checks and throws if options.notifyOnNetworkStatusChange is not undefined, and then sets the option to false (https://github.com/apollographql/apollo-client/blob/master/packages/apollo-client/src/core/QueryManager.ts#L640-L645), so that it throws when reused. Simply spreading the options map into a new object resolves the problem.

@apollo-cla
Copy link

@Problematic: 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/

@Problematic
Copy link
Contributor Author

This is a showstopping issue for us; what else remains to be done so that a new release can be cut with this fix?

@irietek
Copy link

irietek commented Sep 20, 2017

I'm stuck with this issue as well. Any chance we can get this fix merged in soon? Please and Thank You!

@arodr967
Copy link

OMG! I've spent so much time trying to figure out what's wrong. I just checked this out and this fixes my issue!!!!

@infinitflow
Copy link

I have the same issue and tested this fix works. Any progress towards merging it would be greatly appreciated. Thank you

@kamilkisiela
Copy link
Owner

@Problematic could you add it also to the CHANGELOG.md?

@Problematic
Copy link
Contributor Author

Done!

@Problematic
Copy link
Contributor Author

Any updates here?

@infinitflow
Copy link

We are getting closer to merge!

@dflor003
Copy link

This is messing me up as well! Is this PR getting merged soon?

@johnrjj
Copy link

johnrjj commented Oct 12, 2017

👍

@kamilkisiela
Copy link
Owner

You can check out the preview of 1.0 at #377

@Problematic
Copy link
Contributor Author

Problematic commented Oct 23, 2017

Two things. First, the 1.0 preview doesn't appear to have this fix in it, and so will potentially have the same problems. And second, with the breaking API changes, I'm not sure we'll be able to consume it immediately.

Can this PR be merged as a patch-level release in the 0.13 line, or is there additional work that needs to be done to make that possible?

@kamilkisiela
Copy link
Owner

@Problematic I will push a new version today. Could you create a test for it? If not, I'll probably do it before releasing v0.13.3.

@Problematic
Copy link
Contributor Author

Yep, I'll start on that now.

@Problematic
Copy link
Contributor Author

@kamilkisiela I'm unfamiliar with netlify; have I broken something?

@kamilkisiela kamilkisiela merged commit a5eabbf into kamilkisiela:master Oct 24, 2017
@kamilkisiela
Copy link
Owner

@Problematic Released as v0.13.3

@Problematic
Copy link
Contributor Author

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants