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

[BUG: mocks] Default resolvers are called incorrectly #1807

Closed
deftomat opened this issue Jul 20, 2020 · 6 comments
Closed

[BUG: mocks] Default resolvers are called incorrectly #1807

deftomat opened this issue Jul 20, 2020 · 6 comments

Comments

@deftomat
Copy link

deftomat commented Jul 20, 2020

Looks like mock is using the incorrect parameters when it is calling the default resolver.

result = root[fieldName](root, args, context, info);

When the object already contains the field and if that field is a function, then it should be called without the root parameter.
graphql-js is already doing that as described here.

Also, as apollo-server is using this package (a slightly outdated), enabling mocks will broke these "inline" subresolvers. See apollographql/apollo-server#4398 to see it in action.

@yaacovCR
Copy link
Collaborator

I definitely see your point!

As this bug is quite old, I am not sure if this unusual behavior is relied upon by the remainder of the mock code.

Crazy busy at the moment, but a PR that shows no other failing tests would be greatly appreciated.

@deftomat
Copy link
Author

Ok, I will try to create PR.

@yaacovCR yaacovCR mentioned this issue Oct 6, 2020
Merged
@yaacovCR yaacovCR added the waiting-for-release Fixed/resolved, and waiting for the next stable release label Oct 6, 2020
@yaacovCR
Copy link
Collaborator

yaacovCR commented Oct 6, 2020

fixed in v7

yaacovCR added a commit that referenced this issue Oct 6, 2020
resolvers that return 'subresolvers' or fields that are set to functions for later use as resolvers were behaving not according to upstream graphql-js convention, these functions take 3 arguments, not 4, with parent available as this.

See #1807
See https://github.com/graphql/graphql-js/blob/7e79bbe5f2b0e971b5e5f6fe3e7b19c43dea9f35/src/execution/execute.js#L1210-L1212
@ardatan ardatan added the bug label Oct 6, 2020
yaacovCR added a commit that referenced this issue Oct 6, 2020
resolvers that return 'subresolvers' or fields that are set to functions for later use as resolvers were behaving not according to upstream graphql-js convention, these functions take 3 arguments, not 4, with parent available as this.

See #1807
See https://github.com/graphql/graphql-js/blob/7e79bbe5f2b0e971b5e5f6fe3e7b19c43dea9f35/src/execution/execute.js#L1210-L1212
@deftomat
Copy link
Author

deftomat commented Oct 7, 2020

Awesome 🥳

Thanks.

@deftomat deftomat closed this as completed Oct 7, 2020
@yaacovCR
Copy link
Collaborator

yaacovCR commented Oct 7, 2020

Will leave open until release. Thanks for reporting.

@yaacovCR yaacovCR reopened this Oct 7, 2020
yaacovCR added a commit that referenced this issue Oct 23, 2020
resolvers that return 'subresolvers' or fields that are set to functions for later use as resolvers were behaving not according to upstream graphql-js convention, these functions take 3 arguments, not 4, with parent available as this.

See #1807
See https://github.com/graphql/graphql-js/blob/7e79bbe5f2b0e971b5e5f6fe3e7b19c43dea9f35/src/execution/execute.js#L1210-L1212
@yaacovCR
Copy link
Collaborator

Released in v7

@yaacovCR yaacovCR removed bug waiting-for-release Fixed/resolved, and waiting for the next stable release labels Oct 25, 2020
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

No branches or pull requests

3 participants