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

Serialize enum argument values in AddArgumentsAsVariables #1075

Closed
wants to merge 1 commit into from

Conversation

stefanprobst
Copy link

@stefanprobst stefanprobst commented Mar 10, 2019

The AddArgumentsAsVariables transform currently takes the variable values directly from args. This PR serializes all GraphQLEnumType argument values before, so that the variable value contains the enum key, not the internal enum value. This avoids errors with merged schemas.

Not sure if the same thing should also be done with custom scalars(???) I left those alone since I got a bit confused there - the serialize/parseValue/parseLiteral methods don't seem to be carried over in schema merging (because keepResolvers will always be false in recreateType???)

EDIT (07/18): This has been merged into graphql-tools-fork with additional fixes for custom scalars by @yaacovCR - thanks!

@apollo-cla
Copy link

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

@yaacovCR
Copy link
Collaborator

I think this fixes #1033.

@perrin4869
Copy link

Would be amazing to have this merged asap!

@kommander
Copy link

Any update on this?

@miguelocarvajal
Copy link

I tested this locally to stitch a couple of schemas, one with enum as arguments and it worked successfully.

What is needed to merge this?

@yaacovCR
Copy link
Collaborator

This is fixed in graphql-tools-fork v6.0.

@miguelocarvajal
Copy link

@yaacovCR Works like a charm! Thank you!

@bkstorm
Copy link

bkstorm commented Jul 5, 2019

I hope this pull request will be merged asap. Without it, I can't use schema stitching.

@stefanprobst
Copy link
Author

superseded by #1206

@stefanprobst stefanprobst deleted the serialize-args branch November 27, 2019 20:20
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

7 participants