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

refactor: Selection set generation for @defer #3176

Closed
wants to merge 16 commits into from
Closed
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
70 changes: 63 additions & 7 deletions Sources/ApolloCodegenLib/Frontend/CompilationResult.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,18 @@ import JavaScriptCore

/// The output of the frontend compiler.
public class CompilationResult: JavaScriptObject {
private enum Constants {
static let LocalCacheMutationDirectiveName = "apollo_client_ios_localCacheMutation"
static let DeferDirectiveName = "defer"
/// String constants used to match JavaScriptObject instances.
fileprivate enum Constants {
enum DirectiveNames {
static let LocalCacheMutation = "apollo_client_ios_localCacheMutation"
static let Defer = "defer"
}

enum ArgumentLabels {
static let `If` = "if"
}
}

lazy var rootTypes: RootTypeDefinition = self["rootTypes"]

lazy var referencedTypes: [GraphQLNamedType] = self["referencedTypes"]
Expand Down Expand Up @@ -54,7 +62,7 @@ public class CompilationResult: JavaScriptObject {
}

lazy var isLocalCacheMutation: Bool = {
directives?.contains { $0.name == Constants.LocalCacheMutationDirectiveName } ?? false
directives?.contains { $0.name == Constants.DirectiveNames.LocalCacheMutation } ?? false
}()

lazy var nameWithSuffix: String = {
Expand Down Expand Up @@ -117,7 +125,7 @@ public class CompilationResult: JavaScriptObject {
lazy var directives: [Directive]? = self["directives"]

lazy var isLocalCacheMutation: Bool = {
directives?.contains { $0.name == Constants.LocalCacheMutationDirectiveName } ?? false
directives?.contains { $0.name == Constants.DirectiveNames.LocalCacheMutation } ?? false
}()

public override var debugDescription: String {
Expand Down Expand Up @@ -166,13 +174,15 @@ public class CompilationResult: JavaScriptObject {
}
}

public class InlineFragment: JavaScriptObject, Hashable {
public class InlineFragment: JavaScriptObject, Hashable, Deferrable {
lazy var selectionSet: SelectionSet = self["selectionSet"]

lazy var inclusionConditions: [InclusionCondition]? = self["inclusionConditions"]

lazy var directives: [Directive]? = self["directives"]

lazy var isDeferred: IsDeferred = getIsDeferred()

public override var debugDescription: String {
selectionSet.debugDescription
}
Expand All @@ -190,13 +200,15 @@ public class CompilationResult: JavaScriptObject {

/// Represents an individual selection that includes a named fragment in a selection set.
/// (ie. `...FragmentName`)
public class FragmentSpread: JavaScriptObject, Hashable {
public class FragmentSpread: JavaScriptObject, Hashable, Deferrable {
lazy var fragment: FragmentDefinition = self["fragment"]

lazy var inclusionConditions: [InclusionCondition]? = self["inclusionConditions"]

lazy var directives: [Directive]? = self["directives"]

lazy var isDeferred: IsDeferred = getIsDeferred()

var parentType: GraphQLCompositeType { fragment.type }

public func hash(into hasher: inout Hasher) {
Expand Down Expand Up @@ -412,4 +424,48 @@ public class CompilationResult: JavaScriptObject {
}
}

public enum IsDeferred: ExpressibleByBooleanLiteral {
calvincestari marked this conversation as resolved.
Show resolved Hide resolved
case value(Bool)
case variable(String)

public init(booleanLiteral value: BooleanLiteralType) {
self = .value(value)
}

static func `if`(_ variable: String) -> Self {
.variable(variable)
}
}

}

fileprivate protocol Deferrable {
var directives: [CompilationResult.Directive]? { get }
}

fileprivate extension Deferrable where Self: JavaScriptObject {
func getIsDeferred() -> CompilationResult.IsDeferred {
guard let directive = directives?.first(
where: { $0.name == CompilationResult.Constants.DirectiveNames.Defer }
) else {
return false
}

guard let argument = directive.arguments?.first(
where: { $0.name == CompilationResult.Constants.ArgumentLabels.If }
) else {
return true
}

switch (argument.value) {
case let .boolean(value):
return .value(value)

case let .variable(variable):
return .variable(variable)

default:
preconditionFailure("Incompatible argument value. Expected Boolean or Variable, got \(argument.value).")
}
}
}
87 changes: 85 additions & 2 deletions Sources/ApolloCodegenLib/IR/IR+RootFieldBuilder.swift
Original file line number Diff line number Diff line change
Expand Up @@ -230,8 +230,40 @@ extension IR {
}

private func scopeCondition(
for conditionalSelectionSet: ConditionallyIncludable,
for inlineFragment: CompilationResult.InlineFragment,
in parentTypePath: SelectionSet.TypeInfo
) -> ScopeCondition? {
scopeCondition(
for: inlineFragment,
in: parentTypePath,
isDeferred: IsDeferred(compilationResult: inlineFragment.isDeferred)
)
}

private func scopeCondition(
for namedFragment: CompilationResult.FragmentSpread,
in parentTypePath: SelectionSet.TypeInfo
) -> ScopeCondition? {
let matchedParentTypes = (parentTypePath.parentType == namedFragment.parentType)
let matchedInclusionConditions = IR.InclusionConditions.isMatched(
parentTypePath.inclusionConditions,
namedFragment.inclusionConditions
)

let scopedIsDeferred = matchedParentTypes && matchedInclusionConditions
? namedFragment.isDeferred : false

return scopeCondition(
for: namedFragment,
in: parentTypePath,
isDeferred: IR.IsDeferred(compilationResult: scopedIsDeferred)
)
}

private func scopeCondition(
for conditionalSelectionSet: ConditionallyIncludable,
in parentTypePath: SelectionSet.TypeInfo,
isDeferred: IR.IsDeferred
) -> ScopeCondition? {
let inclusionResult = inclusionResult(for: conditionalSelectionSet.inclusionConditions)
guard inclusionResult != .skipped else {
Expand All @@ -241,7 +273,11 @@ extension IR {
let type = parentTypePath.parentType == conditionalSelectionSet.parentType ?
nil : conditionalSelectionSet.parentType

return ScopeCondition(type: type, conditions: inclusionResult.conditions)
return ScopeCondition(
type: type,
conditions: inclusionResult.conditions,
isDeferred: isDeferred
)
Copy link
Contributor

Choose a reason for hiding this comment

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

These changes also should really be tested in tests on the RootFieldBuilder, not just the SelectionSetTemplateTests.

}

private func inclusionResult(
Expand Down Expand Up @@ -387,3 +423,50 @@ extension CompilationResult.InlineFragment: ConditionallyIncludable {
var parentType: GraphQLCompositeType { selectionSet.parentType }
}
extension CompilationResult.FragmentSpread: ConditionallyIncludable {}

fileprivate extension IR.InclusionConditions {
static func isMatched(
_ irConditions: IR.InclusionConditions?,
_ compiledConditions: [CompilationResult.InclusionCondition]?
) -> Bool {
guard
let compiledConditions, !compiledConditions.isEmpty,
let convertedConditions = IR.InclusionConditions.convert(from: compiledConditions)
else {
return irConditions == nil

}

return irConditions == convertedConditions
}

private static func convert(from conditions: [CompilationResult.InclusionCondition]) -> Self? {
var conditions = conditions

guard let irCondition = IR.InclusionCondition(conditions.removeFirst()) else {
return nil
}

var irConditions = IR.InclusionConditions(irCondition)

while conditions.first != nil {
guard let irCondition = IR.InclusionCondition(conditions.removeFirst()) else {
return nil
}

irConditions.append(irCondition)
}

return irConditions
}
}

fileprivate extension IR.InclusionCondition {
init?(_ condition: CompilationResult.InclusionCondition) {
guard case let .variable(variable, inverted) = condition else {
return nil
}

self = .init(variable, isInverted: inverted)
}
}
13 changes: 12 additions & 1 deletion Sources/ApolloCodegenLib/IR/IR.swift
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,22 @@ class IR {
}
}

// TODO: Documentation for this to be completed in issue #3141
/// Represents the `@defer` directive on an inline or named fragment spread. Can be expressed as
/// a Boolean literal or a conditional with a variable.
enum IsDeferred: Hashable, ExpressibleByBooleanLiteral {
case value(Bool)
case `if`(_ variable: String)

init(compilationResult isDeferred: CompilationResult.IsDeferred) {
switch isDeferred {
case let .value(value):
self = .value(value)

case let .variable(variable):
self = .if(variable)
}
}

init(booleanLiteral value: BooleanLiteralType) {
switch value {
case true:
Expand Down
7 changes: 4 additions & 3 deletions Sources/ApolloCodegenLib/IR/ScopeDescriptor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,6 @@ import OrderedCollections

extension IR {

// TODO: Write tests that two inline fragments with same type and inclusion conditions,
Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm thinking we should also be adding tests for this behavior to the IR building tests, not just the SelectionSetTemplateTests. Making sure the actual IR is structured properly is important and having those tests makes it a LOT easier to debug and figure out the cause of bugs than just the template tests.

// but different defer conditions don't merge together.
// To be done in issue #3141
struct ScopeCondition: Hashable, CustomDebugStringConvertible {
let type: GraphQLCompositeType?
let conditions: InclusionConditions?
Expand Down Expand Up @@ -224,6 +221,10 @@ extension IR {
return true
}

var isDeferred: Bool {
scopePath.contains { $0.isDeferred != false }
}

static func == (lhs: ScopeDescriptor, rhs: ScopeDescriptor) -> Bool {
lhs.scopePath == rhs.scopePath &&
lhs.matchingTypes == rhs.matchingTypes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ struct OperationDefinitionTemplate: OperationTemplateRenderer {
accessControlRenderer: { accessControlModifier(for: .member) }()
))

\(section: DeferredProperties(definition))

\(section: VariableProperties(operation.definition.variables))

\(Initializer(operation.definition.variables))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,4 +50,40 @@ extension OperationTemplateRenderer {
"""
}

func DeferredProperties(
calvincestari marked this conversation as resolved.
Show resolved Hide resolved
_ definition: IR.Definition
) -> TemplateString {
func isDeferred(_ selectionSet: IR.SelectionSet) -> Bool {
Copy link
Member Author

Choose a reason for hiding this comment

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

@AnthonyMDev - needing your feedback here whether this is the best way to be determining at an operation-level whether any fragments are deferred; I think it should be since we're only doing direct selections.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this code should be valid, but I'm wondering if we could prevent another iteration through the entire selection set here.

What if we added hasDeferredFragments to the RootFieldBuilder.Result. We could keep track of this as a var on the RootFieldBuilder, and while building, if we encounter a deferred fragment, we just set that to true.

guard !selectionSet.scope.isDeferred else { return true }

if let fields = selectionSet.selections.direct?.fields {
for field in fields.values {
if let entityField = field as? IR.EntityField {
guard !isDeferred(entityField.selectionSet) else { return true }
}
}
}

if let inlineFragments = selectionSet.selections.direct?.inlineFragments {
for fragment in inlineFragments.values {
guard !isDeferred(fragment.selectionSet) else { return true }
}
}

if let namedFragments = selectionSet.selections.direct?.namedFragments {
for fragment in namedFragments.values {
return (fragment.isDeferred != false)
}
}

return false
}

return """
\(if: isDeferred(definition.rootField.selectionSet), """
public static let hasDeferredFragments: Bool = true
""")
"""
}

}
31 changes: 24 additions & 7 deletions Sources/ApolloCodegenLib/Templates/SelectionSetTemplate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ struct SelectionSetTemplate {
_ deprecatedArguments: inout [DeprecatedArgument]?
) -> [TemplateString] {
selections.fields.values.map { FieldSelectionTemplate($0, &deprecatedArguments) } +
selections.inlineFragments.values.map { InlineFragmentSelectionTemplate($0.selectionSet) } +
selections.inlineFragments.values.map { InlineFragmentSelectionTemplate($0) } +
selections.namedFragments.values.map { FragmentSelectionTemplate($0) }
}

Expand Down Expand Up @@ -277,15 +277,15 @@ struct SelectionSetTemplate {
"""
}

private func InlineFragmentSelectionTemplate(_ inlineFragment: IR.SelectionSet) -> TemplateString {
private func InlineFragmentSelectionTemplate(_ inlineFragment: IR.InlineFragmentSpread) -> TemplateString {
"""
.inlineFragment(\(inlineFragment.renderedTypeName).self)
.inlineFragment(\(inlineFragment.selectionSet.renderedTypeName).self\(inlineFragment.isDeferred.rendered))
"""
}

private func FragmentSelectionTemplate(_ fragment: IR.NamedFragmentSpread) -> TemplateString {
private func FragmentSelectionTemplate(_ namedFragment: IR.NamedFragmentSpread) -> TemplateString {
"""
.fragment(\(fragment.definition.name.asFragmentName).self)
.fragment(\(namedFragment.definition.name.asFragmentName).self\(namedFragment.isDeferred.rendered))
"""
}

Expand Down Expand Up @@ -381,8 +381,10 @@ struct SelectionSetTemplate {
let name = fragment.definition.name
let propertyName = name.firstLowercased
let typeName = name.asFragmentName
let isOptional = fragment.inclusionConditions != nil &&
!scope.matches(fragment.inclusionConditions.unsafelyUnwrapped)

let isOptional =
(fragment.inclusionConditions != nil && !scope.matches(fragment.inclusionConditions.unsafelyUnwrapped))
|| fragment.isDeferred != false

return """
\(renderAccessControl())var \(propertyName): \(typeName)\
Expand Down Expand Up @@ -982,3 +984,18 @@ extension IR.SelectionSet.Selections {

}
}

fileprivate extension IR.IsDeferred {
var rendered: String {
switch self {
case .value(true):
return ", deferred: true"

case .value(false):
return ""

case let .if(variable):
return ", deferred: .if(\"\(variable)\")"
}
}
}
Loading