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

[Failing Test] Preserve resolvers of types that are waiting on asynchronous operations #108

Closed
wants to merge 3 commits into from

Conversation

lemonsrc
Copy link

This PR is in reference to this issue:
#105

Mocked types are sent to that types resolvers before RootQuery async operations complete causing resolvers to not be preserved.

It is expected to receive the resolved type from the RootQuery in that types resolvers; however, according to this failing test, a mocked version of that type is received first when the RootQuery returns a Promise and other asynchronous operations like async / await.

@apollo-cla
Copy link

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

@helfer
Copy link
Contributor

helfer commented Aug 19, 2016

@lemonsrc thanks for the PR. It would be great if someone could pick this up and submit a PR with a fix. I'd be happy to give pointers. Any volunteers?

@helfer helfer added the help wanted Extra attention is needed label Aug 19, 2016
@helfer
Copy link
Contributor

helfer commented Aug 28, 2016

@lemonsrc On closer inspection, I think this has nothing to do with the fact that the resolver returns a promise. Rather I think it's because it's a root type, whose fields get handled separately from non-root types. I think it should be a pretty quick fix, if you want to give it a try!

https://github.com/apollostack/graphql-tools/blob/master/src/mock.js#L186

@sebastienbarre
Copy link
Contributor

sebastienbarre commented Aug 30, 2016

@lemonsrc I think I may have introduced that bug a few days before you reported it. I fixed a few things since then, and added some tests, see recent commits in #115, let me know if that works for you.

@helfer
Copy link
Contributor

helfer commented Aug 30, 2016

So it turns out I was wrong and this could indeed have been related to promises. Let's make sure we fix both of those potential issues! @lemonsrc if you don't mind, we'll close this PR and pick things up in #115?

@helfer
Copy link
Contributor

helfer commented Aug 31, 2016

@lemonsrc can you confirm that this works now on master, and if it does close this PR? Thanks!

@helfer helfer closed this Sep 6, 2016
@helfer helfer removed the help wanted Extra attention is needed label Sep 6, 2016
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.

4 participants