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

Fix introspection of default input objects #3456

Merged
merged 1 commit into from
May 3, 2021

Conversation

eapache
Copy link
Contributor

@eapache eapache commented Apr 29, 2021

It seems that the intent was to allow default input object values to be
specified in snake_case, but we weren't camelCasing them on output which
was leading to issues (in introspection and also schema printing).

See e.g. string_value which is actually the default for an argument stringValue (because graphql-ruby automatically camelCases argument names at definition time), in

argument :arg_with_default, InspectableInput, required: false, default_value: { string_value: "S" }

Tweak an existing spec to cover this case (as well as the "enums
specifed as values not names" case), then add the missing call to
camelize.

@rmosolgo I'm not actually 100% sure if I'm interpreting the intention here properly. The code is obviously trivial/straightforward but maybe the intention was to force people to write their default values in camelCase and it's the Jazz schema that made a mistake?

This is a second stand-alone extraction from #3448.

cc @benjie

It seems that the intent was to allow default input object values to be
specified in snake_case, but we weren't camelCasing them on output which
was leading to issues.

Tweak an existing spec to cover this case (as well as the "enums
specifed as values not names" case), then add the missing call to
camelize.
@rmosolgo
Copy link
Owner

You got the intention right -- I meant for the default_value: to look like the object that would get passed into resolvers: symbol, snake-cased keys and application values for enums.

@rmosolgo
Copy link
Owner

I'm trying to think, some people might've worked around this issue, will it "break" them if we fix it here? (I think that's ok, but I want to get it right in the changelog.)

@eapache
Copy link
Contributor Author

eapache commented Apr 30, 2021

🤔 I don't think it will break anything unless the user has disabled auto-camelization of argument names in general and is creating snake_case arguments in their actual schema? Otherwise writtenAsCamel default values should continue to pass through, and written_as_snake arguments will start working where they didn't before.

@rmosolgo
Copy link
Owner

rmosolgo commented May 3, 2021

Thanks for helping me think through that -- anyone who worked around this before should still be good to go 👍

@rmosolgo rmosolgo added this to the 1.12.9 milestone May 3, 2021
@rmosolgo rmosolgo merged commit b76d59f into rmosolgo:master May 3, 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.

2 participants