## Summary: We typically name our types `OperationFieldTypeFieldType`, but if a type's name matches the preceding field-name, we omit the type-name. In #71 I changed the behavior such that we no longer do that in the case where the type's name matches some suffix of the name-so-far that's longer than just the leaf field-name. This was semi-intentional; I assumed it didn't matter and would be more predictable this way. But it turns out that was a feature, both in the sense that almost any change to the type-name-generator is breaking, and in the sense that it made the names uglier. Plus, now that we have better conflict-detection (#94), the possibility that some tricksy type-names could cause problems is no longer as much of an issue, so we can be a little less careful here. (Although I think this is no less safe than before; the field-names are the important part.) So in this commit I revert the change. Specifically, this comes up a lot at Khan where we do ``` mutation ForcePhantom { forcePhantom { # type: ForcePhantom error { ... } # type: ForcePhantomError } } ``` Before #71, and again after this change, we'll generate `ForcePhantomForcePhantomError` for `error`; before we'd generate `ForcePhantomForcePhantomErrorForcePhantomError`. Issue: https://github.com/Khan/genqlient/issues/109 ## Test plan: make tesc Author: benjaminjkraft Reviewers: csilvers, aberkan, dnerdy, jvoll, mahtabsabet, MiguelCastillo, StevenACoffman Required Reviewers: Approved By: csilvers Checks: ✅ Test (1.17), ✅ Test (1.16), ✅ Test (1.15), ✅ Test (1.14), ✅ Lint, ✅ Test (1.17), ✅ Test (1.16), ✅ Test (1.15), ✅ Test (1.14), ✅ Lint Pull Request URL: https://github.com/Khan/genqlient/pull/110
This commit is contained in:
@@ -26,6 +26,8 @@ When releasing a new version:
|
|||||||
|
|
||||||
### Bug fixes:
|
### Bug fixes:
|
||||||
|
|
||||||
|
- Generated type-names now abbreviate across multiple components; for example if the path to a type is `(MyOperation, Outer, Outer, Inner, OuterInner)`, it will again be called `MyOperationOuterInner`. (This regressed in a pre-v0.1.0 refactor.) (#109)
|
||||||
|
|
||||||
## v0.1.0
|
## v0.1.0
|
||||||
|
|
||||||
First open-sourced version.
|
First open-sourced version.
|
||||||
|
|||||||
+2
-2
@@ -143,9 +143,9 @@ func typeNameParts(prefix *prefixList, typeName string) *prefixList {
|
|||||||
// If the prefix has just one part, that's the operation-name. There's no
|
// If the prefix has just one part, that's the operation-name. There's no
|
||||||
// need to add "Query" or "Mutation". (Zero should never happen.)
|
// need to add "Query" or "Mutation". (Zero should never happen.)
|
||||||
if prefix == nil || prefix.tail == nil ||
|
if prefix == nil || prefix.tail == nil ||
|
||||||
// If the last prefix field ends with this type's name, omit the
|
// If the name-so-far ends with this type's name, omit the
|
||||||
// type-name (see the "shortened" case in the top-of-file comment).
|
// type-name (see the "shortened" case in the top-of-file comment).
|
||||||
strings.HasSuffix(prefix.head, typeName) {
|
strings.HasSuffix(joinPrefixList(prefix), typeName) {
|
||||||
return prefix
|
return prefix
|
||||||
}
|
}
|
||||||
return &prefixList{typeName, prefix}
|
return &prefixList{typeName, prefix}
|
||||||
|
|||||||
@@ -33,8 +33,8 @@ func TestTypeNames(t *testing.T) {
|
|||||||
[]*ast.Field{fakeField("Query", "operationUser")},
|
[]*ast.Field{fakeField("Query", "operationUser")},
|
||||||
"User",
|
"User",
|
||||||
}, {
|
}, {
|
||||||
// We don't shorten across multiple prefixes.
|
// We do shorten across multiple prefixes.
|
||||||
"OperationUserOperationUser",
|
"OperationUser",
|
||||||
[]*ast.Field{fakeField("Query", "user")},
|
[]*ast.Field{fakeField("Query", "user")},
|
||||||
"OperationUser",
|
"OperationUser",
|
||||||
}, {
|
}, {
|
||||||
|
|||||||
Reference in New Issue
Block a user