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

feat(code-first): Use capitalized case for enums in code-first #1852

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

kasvith
Copy link

@kasvith kasvith commented Nov 4, 2021

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Other... Please describe:

What is the current behavior?

Issue Number: #1832

What is the new behavior?

registerEnumType now contains an optional parameter called mapToUppercase(defaults to false to avoid breaking changes). This option automatically converts enum to CONST_CASE in compliance with GraphQL best practices.
It means we can use PascalCase in typescript while using CONST_CASE in GraphQL.

export enum CatType {
  PersianCat = 'persian-cat',
  MaineCoon = 'maine-coon',
  Ragdoll = 'ragdoll',
}

registerEnumType(CatType, {
  name: 'CatType',
  description: 'Distinguish cats',
  mapToUppercase: true, 
});

Would produce the following enum type in schema

"""Distinguish cats"""
enum CatType {
  PERSIAN_CAT
  MAINE_COON
  RAGDOLL
}

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

Since this is a norm in GraphQL world, from the next release I consider we should make mapToUppercase to true by default. So people can use natural PascalCase enum values in typescript and use CONST_CASE by default in GraphQL

@kasvith
Copy link
Author

kasvith commented Nov 4, 2021

@kamilmysliwiec would you please take a look at this

@kasvith
Copy link
Author

kasvith commented Nov 5, 2021

I added docs in JSDoc format for the added option. But I would also add this to nest docs once its merged

@kasvith kasvith changed the title feat(): Use capitalized case for enums in code-first feat(code-first): Use capitalized case for enums in code-first Nov 8, 2021
@kasvith
Copy link
Author

kasvith commented Jan 24, 2022

I've resolved the merge conflict. Also addressed comments by @micalevisk.
Can you take a look @kamilmysliwiec ? 😃

@kasvith
Copy link
Author

kasvith commented Feb 10, 2022

I added more test cases

@kasvith
Copy link
Author

kasvith commented Sep 30, 2022

Hi @kamilmysliwiec would you be able to review this PR and give me a comment about the future of it :) thanks

@arnold-almeida
Copy link

This is a subtle but important feature. Agree that by default it should align with GraphQL enum best practices.

@peshkov3
Copy link

peshkov3 commented Mar 8, 2023

Waiting this PR

@kasvith
Copy link
Author

kasvith commented Mar 11, 2023

Im ready to resolve conflicts once it gets proper attention

@kasvith
Copy link
Author

kasvith commented May 24, 2023

@kamilmysliwiec would you be able to review this and get the approval

i can resolve the merge conflict afterwards

@kasvith
Copy link
Author

kasvith commented Feb 29, 2024

Hi @kamilmysliwiec any news about this?

@@ -12,4 +12,5 @@ export interface EnumMetadata<T extends object = any> {
name: string;
description?: string;
valuesMap?: EnumMetadataValuesMap<T>;
mapToUppercase?: boolean;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kasvith this option seems to be a single purpose one. Would it be better to take a key transform function so that people can use any function they want for the enum keys? toUppercase could then become the default and the feature would become a lot more useful.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @mareksuscak

By convention GraphQL enums are UPPER_CASE literals, i dont see a usecase to have a transformer function and make this much more complicated than it is now

https://www.apollographql.com/tutorials/side-quest-intermediate-schema-design/02-the-enum-type

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants