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

Allow to disable Input suffix #2039

Conversation

pdambrauskas
Copy link
Contributor

@pdambrauskas pdambrauskas commented Oct 1, 2024

📝 Description

We are migrating our schema from a legacy Graphql service to a new on, which is based on graphql-kotlin.
We have some input types that do not have Input suffix and want to migrate those types without breaking the schema.

For this to work in a backwards compatible way , I'm suggesting adding new annotation -@GraphQLSkipInputSuffix,
when it is added on input type - schema generator does not add the suffix

@samuelAndalon

@samuelAndalon
Copy link
Contributor

LGTM, i would just rename the field to skipInputSuffix

@pdambrauskas
Copy link
Contributor Author

@samuelAndalon, just a thought - maybe it would make more sense to introduce a separate @GraphQLSkipInputSuffix annotation for this use-case?

Maybe having a property GraphQLName makes no sense, since it can be used on queries and output fields?

@samuelAndalon
Copy link
Contributor

that works as well

@pdambrauskas
Copy link
Contributor Author

Updated. Using dedicated annotation instead of field on name annotation.

@dariuszkuc
Copy link
Collaborator

my 2 cents: if you are going with separate annotations I'd probably just call it @GraphQLInputName (vs existing @GraphQLName).

*also you probably should update PR description

@dariuszkuc
Copy link
Collaborator

I was just thinking -> if using @GraphQLInputName I think the schema generator should also throw an exception if that type is used for output

@pdambrauskas
Copy link
Contributor Author

I've added @GraphQLSkipInputSuffix annotation - It makes more sense, because:

  • You do not have to specify the Type name, as you would with @GraphQLName - class name will be used
  • You can add it only on classes, while @GraphQLName can be added on query fields as well.

@dariuszkuc if we chose to go with @GraphQLInputName, it would look confusing from users perspective, when we also have @GraphQLName - functionality of those two annotations would overlap, since you'd be able to rename the type with both of these.

Alternatively, we could introduce @GraphQLInputType(withSuffix=false), in that case, it would be possible to add input type specific configurations to a single annotation, if it is needed in the future.

@samuelAndalon
Copy link
Contributor

@pdambrauskas PR LGTM, could you please update the 8.x.x docs ? -- as it will be released there first, then we would need to cherry pick it to 7.x.x

@pdambrauskas pdambrauskas force-pushed the allow-to-skip-suffix-on-input-type branch from 3f160e1 to b1787ce Compare October 8, 2024 06:45
@samuelAndalon samuelAndalon merged commit c360573 into ExpediaGroup:master Oct 8, 2024
9 checks passed
@pdambrauskas pdambrauskas deleted the allow-to-skip-suffix-on-input-type branch October 9, 2024 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants