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

loads called before prepare on GraphQL::Schema::InputObject #4017

Closed
crpahl opened this issue Apr 1, 2022 · 2 comments · Fixed by #4128
Closed

loads called before prepare on GraphQL::Schema::InputObject #4017

crpahl opened this issue Apr 1, 2022 · 2 comments · Fixed by #4128

Comments

@crpahl
Copy link

crpahl commented Apr 1, 2022

Describe the bug

This isn't necessarily a bug, but I'd love to get your thoughts on the issue we're running into here!

For a GraphQL::Schema::Resolver, or GraphQL::Schema::Mutation argument, the proc set for prepare will get called before auto-loading happens when the loads argument is set.

But for a GraphQL::Schema::InputObject argument, the opposite appears to happen and the auto-loading happens first, followed by the the call to the prepare proc. In our case, we always want the prepare proc to be called first. I'll briefly cover why we need this in our case.

The initial build of our GraphQL API didn't return global IDs, but we do currently return global IDs. So apps can be sending us either global, or non-global IDs. When we receive a non-global ID we're actually still able to use the loads mechanism as we've added logic to the prepare proc that is able to convert the id into a global ID (we have meta-data configured on our types that allows us to convert it to the correct global ID format). This doesn't work for something that subclasses GraphQL::Schema::InputObject as the auto-loading happens first (and our proc wasn't able to convert it to global ID format first).

Would it be possible to update GraphQL::Schema::InputObject so the prepare is called before the auto-loading happens (if that is indeed the issue)? Or is there some other approach we should be considering here?

Versions

graphql version: 1.13

@dmcrorieGIT
Copy link

☝️ I've been running into this as well

@rmosolgo
Copy link
Owner

rmosolgo commented Apr 9, 2022

Thanks for the detailed report. Dang, I hadn't noticed it before! But it makes sense to make it work the same way as Resolvers. I'll take a look soon and follow up here.

IIRC this code is a bit of a mess because of how resolvers are initialized: they're initialized during field resolution, but Input objects can be initialized earlier, since they're just a function of static type and AST/variable values. Since the object lifecycles are different, they use different mechanisms for these steps. It's confusing, and, evidently, out-of-order!

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 a pull request may close this issue.

3 participants