Fix RootEntityType and FulfilledFragment name generation #3045
Conversation
✅ Deploy Preview for apollo-ios-docs canceled.
|
| let type: GraphQLType | ||
|
|
||
| /// Represents the location within a GraphQL definition (operation or fragment) of an `Entity`. | ||
| struct Location: Hashable { |
There was a problem hiding this comment.
In order to generate selection set names properly, an entity needs to be able to indicate where it is located in its definition. This includes needing to know what definition (operation or fragment) it is defined in.
Previously, we were using a hacky solution of adding an initial FieldPath component that included the name of the fragment, or the operation type. This was being used to help properly generate the selection set names, but it was hacky for 2 reasons:
- It was making the "field path" not really accurate. Field path included a first component for the source definition that any code that was consuming the field path just needed to understand was there. Causing tight coupling of implementation code.
- This partially put the responsibility of how we generate names in the templates into the IR. The IR should just represent the shape of the definitions, and the templates consume them to understand how to generate the correct code.
Representing the Location with a SourceDefinition and a FieldPath allows us to better represent the shape of the operations in the IR and allows the templates to do all the logic for formatting names of the Selection Sets.
There was a problem hiding this comment.
Like this change, especially helping separate responsibilities between the IR and Templates
| @@ -44,6 +44,10 @@ public class CompilationResult: JavaScriptObject { | |||
| "\(name) on \(rootType.debugDescription)" | |||
| } | |||
|
|
|||
| public func hash(into hasher: inout Hasher) { | |||
| hasher.combine(name) | |||
There was a problem hiding this comment.
There can never be 2 operations with the same name, as graphql-js would fail validation. So this is safe
| precondition(!array.isEmpty, "Cannot initialize LinkedList with an empty array. LinkedList must have at least one element.") | ||
| var segments = array | ||
| let headNode = HeadNode(value: segments.removeFirst()) | ||
| public init<C: Collection<T>>(_ collection: C) { |
There was a problem hiding this comment.
Made this function take in a generic collection instead of only an array. This allows us to pass in another linked list or an ordered set.
| ) | ||
| } | ||
|
|
||
| private static func generatedSelectionSetName( |
There was a problem hiding this comment.
This is the core of the changes in the template. The rest is mostly updating call sites to use these new functions.
This refactors the selection set name computation to account for the fact that the fieldPath no longer includes a component for the root definition.
| @@ -6037,6 +6037,153 @@ class SelectionSetTemplateTests: XCTestCase { | |||
| expect(actual).to(equalLineByLine(expected, atLine: 1, ignoringExtraLines: true)) | |||
| } | |||
|
|
|||
| func test__render_conditionalFragmentOnQueryRoot__rendersRootEntityType() throws { | |||
There was a problem hiding this comment.
These are the tests for the actual bug fix. The rest of the test changes are just refactoring for other minor API changes.
Additional error noted in closed ticket 2949 #2949 (comment)
Fully qualified names should be generated at all times for
FulfilledFragmentsandRootEntityType. Due to the issue indicated in #2949Fully qualified names were not being calculated properly for fragments on the root of a query. Simplified the calculation of these by adding the fully qualified root type names to the root of the
FieldPathComponents. This fixes #2962.