-
Notifications
You must be signed in to change notification settings - Fork 117
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
Handling enums with strings that have variations of character casing #265
Comments
Oof, that's an annoying schema. (I assume they must have done it for backcompat reasons? Especially since it's only some of the enums.) Nonetheless, I guess we have to at least have a way to handle it. Thanks for reporting! Do you know if you will want to use all of the values, or only some non-conflicting subset? Thinking out loud about a few options:
I don't love any of these. Option 1 means that a backwards-compatible change in the schema won't be backwards-compatible in the generated code (since a new enum member would affect the mapping of existing ones; at least it's a compile-time error). Doing 2 everywhere seems kinda overkill, and even for enums seems a bit annoying and also inconsistent. I guess it would be ok if we let you do it just for a specific type? Then at least it's explicit. Meanwhile 3 will be very annoying to set up since there will be dozens of values you have to map, and 4 seems a bit incomplete unless in conjunction with the other solutions. And 5 seems like too much complexity to get into. Note that in principle at least the same problem can happen with fields and types. In practice that's maybe less likely, although I could see it for similar backcompat reasons. For enums we are pretty safe on Go's privacy rules since they're all prefixed with the type name; for fields we do need to keep an uppercase first letter (and for types the user might prefer the same, if their generated code is in its own package). With that exception, I think any of these approaches could work for fields and types, and 3 we sort of already have for types (albeit only locally in a directive, whereas this would ~need to be global in I don't know that I care about handling this in an ideal way since it's hopefully a rare case, but we should at least try not to paint ourselves into a corner. I'm thinking 2 is probably the way to go? It would be nice to do things automatically but 1 seems a bit too magical and we can always give a clearer error message that points you to the option. And it seems extensible enough if we do want this for fields/types/etc. in the future, and could pair with 3 or 4. Something like: casing:
enums:
CreditCardBrandCode: raw
# perhaps in the future:
casing:
enums:
CreditCardBrandCode.american_express: raw
fields:
MyType: raw
MyType.myField: raw
types:
myWeird__type: raw Or alternately (arguably overloading bindings a bit but it might be nice to keep all per-type config together, and we could rename it eventually if needed): bindings:
CreditCardBrandCode:
value_casing: raw
# perhaps in the future:
bindings:
CreditCardBrandCode:
values:
american_express:
casing: raw
visa:
mapping: "Visa®"
diners:
omit: true
MyType:
field_casing: raw
fields:
myField:
casing: raw
myWeird__type:
casing: raw |
Hello! Thanks for the thorough and quick reply. I would like to be able to use all values just in case. I personally prefer to not do any manual mapping and prefer magic in instance, but having the ability to override with a manual mapping when needed/desired would be nice. |
There are a bunch of places in genqlient where we just kind of hope you don't have ridiculous casing conflicts in your schema. Apparently with enum values there are actual schemas that have this problem! Now we have an option to disable. I ended up putting it all under `casing` instead of in `bindings` so we can clearly document the list of algorithms we support, and so we can have an `all_enums` value if you're working with a schema with a lot of this; in the future we may want to add similar behavior for types/fields, add more possible algorithms (e.g. to make things unexported), etc. I added tests for the new feature, although the way the tests are set up it wasn't convenient to do so for a schema where this is actually required. I also added a check for case conflicts that points you to this option. (I don't want to do it automatically for reasons described in the issue; mainly it just seemed a bit too magical.) Fixes #265.
There are a bunch of places in genqlient where we just kind of hope you don't have ridiculous casing conflicts in your schema. Apparently with enum values there are actual schemas that have this problem! Now we have an option to disable. I ended up putting it all under `casing` instead of in `bindings` so we can clearly document the list of algorithms we support, and so we can have an `all_enums` value if you're working with a schema with a lot of this; in the future we may want to add similar behavior for types/fields, add more possible algorithms (e.g. to make things unexported), etc. I added tests for the new feature, although the way the tests are set up it wasn't convenient to do so for a schema where this is actually required. I also added a check for case conflicts that points you to this option. (I don't want to do it automatically for reasons described in the issue; mainly it just seemed a bit too magical.) Fixes #265.
Describe the bug
genqlient doesn't handle enums well when a string has a capital case & lower case version of it.
To Reproduce
Braintree's schema.graphql has a section that looks like the following and I get a repeating version of error further below when trying to compile my project.
Expected behavior
I suck at Go. Maybe create
CreditCardBrandCodeAMERICANEXPRESS
andCreditCardBrandCodeAmericanExpress
?genqlient version
Latest version, I'm running
go run github.com/Khan/genqlient
Additional context
Add any other context about the problem here.
The text was updated successfully, but these errors were encountered: