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

validation: allow Ints passed to Float arguments #1149

Closed
wants to merge 2 commits into from

Conversation

darl
Copy link
Contributor

@darl darl commented Nov 17, 2021

Maybe translation similar to #1136 would be better.
But I don't know reasons behind #1136 implementation.

@ghostdogpr
Copy link
Owner

The reason behind the transformation of StringValue into EnumValue is so that when you use Field metadata, you get a value of a type that matches the schema. Basically it's coercing "early" so that you don't even need to do it in the ArgBuilder. I guess we should apply the same logic for this case. @frekw right? What do you think?

@ghostdogpr
Copy link
Owner

@darl I opened #1150

@frekw
Copy link
Collaborator

frekw commented Nov 17, 2021

@ghostdogpr yeah I think doing it like in #1150 is the only way to do it since we're basically paving over limitations in JSON. If I read the spec correctly, coercion should only be applied when we're dealing with variables so doing it like this probably runs the risk of accepting invalid queries for Content-Type: application/graphql.

@ghostdogpr
Copy link
Owner

@frekw shouldn't it be applied to arguments too? http://spec.graphql.org/June2018/#sec-Coercing-Field-Arguments

@ghostdogpr
Copy link
Owner

Answering myself: not for enums, but for float yes. Will handle it in my other PR.

@ghostdogpr ghostdogpr closed this Nov 17, 2021
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.

3 participants