-
Notifications
You must be signed in to change notification settings - Fork 277
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
Normalize subgraph names when generating expanded enum values #5814
Conversation
CI performance tests
|
"Subgraph name couldn't be transformed into valid GraphQL name", | ||
)); | ||
match EnumValue::new(&subgraph.name) { | ||
Ok(enum_value) => subgraphs_and_enum_values.push((subgraph, enum_value)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need to worry about collisions resulting from the normalization? For example, if two names were the same except for characters that were all converted to "_"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is only being done for our auto-transformed names, so hopefully that's really unlikely. It'd need to be two subgraphs which have nearly identical names and also have a @connect
on the same path.
Long term, though, for sure we should also port this
This almost completely implements the JS equivalent, but I intentionally left out adding the
_
suffix, since it'll update all our snapshots and be messy. It's not obvious to me why that rule was in place initially, and clearly this pattern has been working for us so far, so 🤷.Implemented with a new type, just so I could easily see all the things this impacts. I reduced/moved a couple clones while updating type signatures, since I was looking.