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

Fix RootEntityType and FulfilledFragment name generation #3045

Merged
merged 8 commits into from
May 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion Sources/ApolloCodegenLib/Frontend/CompilationResult.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public class CompilationResult: JavaScriptObject {
lazy var subscriptionType: GraphQLNamedType? = self["subscriptionType"]
}

public class OperationDefinition: JavaScriptObject, Equatable {
public class OperationDefinition: JavaScriptObject, Hashable {
lazy var name: String = self["name"]

lazy var operationType: OperationType = self["operationType"]
Expand All @@ -44,6 +44,10 @@ public class CompilationResult: JavaScriptObject {
"\(name) on \(rootType.debugDescription)"
}

public func hash(into hasher: inout Hasher) {
hasher.combine(name)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There can never be 2 operations with the same name, as graphql-js would fail validation. So this is safe

}

public static func ==(lhs: OperationDefinition, rhs: OperationDefinition) -> Bool {
return lhs.name == rhs.name
}
Expand Down
20 changes: 16 additions & 4 deletions Sources/ApolloCodegenLib/IR+Formatting.swift
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,19 @@ extension GraphQLType {

extension IR.EntityField {

/// Takes the associated ``IR.EntityField`` and formats it into a selection set name
/// Takes the associated `IR.EntityField` and formats it into a selection set name
func formattedSelectionSetName(
with pluralizer: Pluralizer
) -> String {
IR.Entity.FieldPathComponent(name: responseKey, type: type)
IR.Entity.Location.FieldComponent(name: responseKey, type: type)
.formattedSelectionSetName(with: pluralizer)
}

}

extension IR.Entity.FieldPathComponent {
extension IR.Entity.Location.FieldComponent {

/// Takes the associated ``IR.Entity.FieldPathComponent`` and formats it into a selection set name
/// Takes the associated `IR.Entity.Location.FieldComponent` and formats it into a selection set name
func formattedSelectionSetName(
with pluralizer: Pluralizer
) -> String {
Expand All @@ -38,3 +38,15 @@ extension IR.Entity.FieldPathComponent {
}

}

extension IR.Entity.Location.SourceDefinition {

/// Takes the associated `IR.Entity.Location.SourceDefinition` and formats it into a selection set name
func formattedSelectionSetName() -> String {
switch self {
case .operation: return "Data"
case let .namedFragment(fragment): return fragment.generatedDefinitionName
}
}

}
3 changes: 1 addition & 2 deletions Sources/ApolloCodegenLib/IR/IR+NamedFragmentBuilder.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@ extension IR {
)

let rootEntity = Entity(
rootTypePath: LinkedList(fragmentDefinition.type),
fieldPath: [.init(name: rootField.name, type: rootField.type)]
source: .namedFragment(fragmentDefinition)
)

let result = RootFieldBuilder.buildRootEntityField(
Expand Down
3 changes: 1 addition & 2 deletions Sources/ApolloCodegenLib/IR/IR+OperationBuilder.swift
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@ extension IR {
)

let rootEntity = Entity(
rootTypePath: LinkedList(operationDefinition.rootType),
fieldPath: [.init(name: rootField.name, type: rootField.type)]
source: .operation(operationDefinition)
)

let result = RootFieldBuilder.buildRootEntityField(
Expand Down
34 changes: 18 additions & 16 deletions Sources/ApolloCodegenLib/IR/IR+RootFieldBuilder.swift
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,18 @@ import OrderedCollections
extension IR {

class RootFieldEntityStorage {
private(set) var entitiesForFields: [Entity.FieldPath: IR.Entity] = [:]
private(set) var entitiesForFields: [Entity.Location: IR.Entity] = [:]

init(rootEntity: Entity) {
entitiesForFields[rootEntity.fieldPath] = rootEntity
entitiesForFields[rootEntity.location] = rootEntity
}

func entity(
for field: CompilationResult.Field,
on enclosingEntity: Entity
) -> Entity {
let fieldPath = enclosingEntity
.fieldPath
let location = enclosingEntity
.location
.appending(.init(name: field.responseKey, type: field.type))

var rootTypePath: LinkedList<GraphQLCompositeType> {
Expand All @@ -25,32 +25,34 @@ extension IR {
return enclosingEntity.rootTypePath.appending(fieldType)
}

return entitiesForFields[fieldPath] ??
createEntity(fieldPath: fieldPath, rootTypePath: rootTypePath)
return entitiesForFields[location] ??
createEntity(location: location, rootTypePath: rootTypePath)
}

func entity(
for otherEntity: IR.Entity,
for entityInFragment: IR.Entity,
inFragmentSpreadAtTypePath fragmentSpreadTypeInfo: SelectionSet.TypeInfo
) -> Entity {
let fieldPath = fragmentSpreadTypeInfo.entity.fieldPath +
otherEntity.fieldPath.dropFirst()
var location = fragmentSpreadTypeInfo.entity.location
if let pathInFragment = entityInFragment.location.fieldPath {
location = location.appending(pathInFragment)
}

var rootTypePath: LinkedList<GraphQLCompositeType> {
let otherRootTypePath = otherEntity.rootTypePath.dropFirst()
let otherRootTypePath = entityInFragment.rootTypePath.dropFirst()
return fragmentSpreadTypeInfo.entity.rootTypePath.appending(otherRootTypePath)
}

return entitiesForFields[fieldPath] ??
createEntity(fieldPath: fieldPath, rootTypePath: rootTypePath)
return entitiesForFields[location] ??
createEntity(location: location, rootTypePath: rootTypePath)
}

private func createEntity(
fieldPath: Entity.FieldPath,
location: Entity.Location,
rootTypePath: LinkedList<GraphQLCompositeType>
) -> Entity {
let entity = Entity(rootTypePath: rootTypePath, fieldPath: fieldPath)
entitiesForFields[fieldPath] = entity
let entity = Entity(location: location, rootTypePath: rootTypePath)
entitiesForFields[location] = entity
return entity
}

Expand All @@ -66,7 +68,7 @@ extension IR {
struct Result {
let rootField: IR.EntityField
let referencedFragments: ReferencedFragments
let entities: [Entity.FieldPath: IR.Entity]
let entities: [Entity.Location: IR.Entity]
}

typealias ReferencedFragments = OrderedSet<NamedFragment>
Expand Down
85 changes: 71 additions & 14 deletions Sources/ApolloCodegenLib/IR/IR.swift
Original file line number Diff line number Diff line change
Expand Up @@ -62,30 +62,87 @@ class IR {
/// Multiple `SelectionSet`s may select fields on the same `Entity`. All `SelectionSet`s that will
/// be selected on the same object share the same `Entity`.
class Entity {
struct FieldPathComponent: Hashable {
let name: String
let type: GraphQLType

/// Represents the location within a GraphQL definition (operation or fragment) of an `Entity`.
struct Location: Hashable {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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:

  1. 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.
  2. 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.

Copy link
Member

Choose a reason for hiding this comment

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

Like this change, especially helping separate responsibilities between the IR and Templates

enum SourceDefinition: Hashable {
case operation(CompilationResult.OperationDefinition)
case namedFragment(CompilationResult.FragmentDefinition)

var rootType: GraphQLCompositeType {
switch self {
case let .operation(definition): return definition.rootType
case let .namedFragment(definition): return definition.type
}
}
}

struct FieldComponent: Hashable {
let name: String
let type: GraphQLType
}

typealias FieldPath = LinkedList<FieldComponent>

/// The operation or fragment definition that the entity belongs to.
let source: SourceDefinition

/// The path of fields from the root of the ``source`` definition to the entity.
///
/// Example:
/// For an operation:
/// ```graphql
/// query MyQuery {
/// allAnimals {
/// predators {
/// height {
/// ...
/// }
/// }
/// }
/// }
/// ```
/// The `Height` entity would have a field path of [allAnimals, predators, height].
let fieldPath: FieldPath?

func appending(_ fieldComponent: FieldComponent) -> Location {
let fieldPath = self.fieldPath?.appending(fieldComponent) ?? LinkedList(fieldComponent)
return Location(source: self.source, fieldPath: fieldPath)
}

func appending<C: Collection<FieldComponent>>(_ fieldComponents: C) -> Location {
let fieldPath = self.fieldPath?.appending(fieldComponents) ?? LinkedList(fieldComponents)
return Location(source: self.source, fieldPath: fieldPath)
}

static func +(lhs: IR.Entity.Location, rhs: FieldComponent) -> Location {
lhs.appending(rhs)
}
}
typealias FieldPath = LinkedList<FieldPathComponent>

/// The selections that are selected for the entity across all type scopes in the operation.
/// Represented as a tree.
let selectionTree: EntitySelectionTree

/// A list of path components indicating the path to the field containing the `Entity` in
/// an operation or fragment.
let fieldPath: FieldPath
/// The location within a GraphQL definition (operation or fragment) where the `Entity` is
/// located.
let location: Location

var rootTypePath: LinkedList<GraphQLCompositeType> { selectionTree.rootTypePath }

var rootType: GraphQLCompositeType { rootTypePath.last.value }

init(source: Location.SourceDefinition) {
self.location = .init(source: source, fieldPath: nil)
self.selectionTree = EntitySelectionTree(rootTypePath: LinkedList(source.rootType))
}

init(
rootTypePath: LinkedList<GraphQLCompositeType>,
fieldPath: FieldPath
location: Location,
rootTypePath: LinkedList<GraphQLCompositeType>
) {
self.location = location
self.selectionTree = EntitySelectionTree(rootTypePath: rootTypePath)
self.fieldPath = fieldPath
}
}

Expand Down Expand Up @@ -143,11 +200,11 @@ class IR {
let referencedFragments: OrderedSet<NamedFragment>

/// All of the Entities that exist in the fragment's selection set,
/// keyed by their relative response path within the fragment.
/// keyed by their relative location (ie. path) within the fragment.
///
/// - Note: The FieldPath for an entity within a fragment will begin with a path component
/// with the fragment's name and type.
let entities: [IR.Entity.FieldPath: IR.Entity]
let entities: [IR.Entity.Location: IR.Entity]

var name: String { definition.name }
var type: GraphQLCompositeType { definition.type }
Expand All @@ -156,7 +213,7 @@ class IR {
definition: CompilationResult.FragmentDefinition,
rootField: EntityField,
referencedFragments: OrderedSet<NamedFragment>,
entities: [IR.Entity.FieldPath: IR.Entity]
entities: [IR.Entity.Location: IR.Entity]
) {
self.definition = definition
self.rootField = rootField
Expand Down
17 changes: 11 additions & 6 deletions Sources/ApolloCodegenLib/LinkedList.swift
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ public struct LinkedList<T>: ExpressibleByArrayLiteral {
self.value = value
self.index = index
}

public var isHead: Bool {
index == 0
}
}

final class HeadNode: Node {
Expand Down Expand Up @@ -71,24 +75,25 @@ public struct LinkedList<T>: ExpressibleByArrayLiteral {
self.headNode = head
}

@_disfavoredOverload
public init(_ headValue: T) {
self.init(head: HeadNode(value: headValue))
}

public init(array: [T]) {
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) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

precondition(!collection.isEmpty, "Cannot initialize LinkedList with an empty collection. LinkedList must have at least one element.")
var iterator = collection.makeIterator()
let headNode = HeadNode(value: iterator.next()!)

self.init(head: headNode)

for segment in segments {
while let segment = iterator.next() {
append(segment)
}
}

public init(arrayLiteral segments: T...) {
self.init(array: segments)
self.init(segments)
}

private func copy() -> Self {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,18 +24,30 @@ extension IR.Definition {

}

extension CompilationResult.OperationDefinition {
var generatedDefinitionName: String {
nameWithSuffix.firstUppercased
}
}

extension IR.Operation {

var generatedDefinitionName: String {
definition.nameWithSuffix.firstUppercased
definition.generatedDefinitionName
}

}

extension CompilationResult.FragmentDefinition {
var generatedDefinitionName: String {
name.firstUppercased
}
}

extension IR.NamedFragment {

var generatedDefinitionName: String {
definition.name.firstUppercased
definition.generatedDefinitionName
}

}
Loading