Skip to content

RFC Allow recursion within ResolveAbstractType #960

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

Closed
wants to merge 2 commits into from
Closed
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
16 changes: 12 additions & 4 deletions spec/Section 6 -- Execution.md
Original file line number Diff line number Diff line change
Expand Up @@ -722,18 +722,26 @@ and output of {CoerceResult()} must not be {null}.

When completing a field with an abstract return type, that is an Interface or
Union return type, first the abstract type must be resolved to a relevant Object
type. This determination is made by the internal system using whatever means
appropriate.
type. The internal system must provided a method for doing so using whatever
means appropriate.

Note: In cases where interfaces may implement interfaces, creating an abstract
type hierarchy, the provided internal method may find it convenient on some
systems to return an intermediate Interface type rather than the runtime Object
type, requiring recursion, as detailed below.

Note: A common method of determining the Object type for an {objectValue} in
object-oriented environments, such as Java or C#, is to use the class name of
the {objectValue}.

ResolveAbstractType(abstractType, objectValue):

- Return the result of calling the internal method provided by the type system
for determining the Object type of {abstractType} given the value
- Let {possibleRuntimeType} be the result of calling the internal method
provided by the type system, given Abstract type {abstractType} and value
{objectValue}.
- If {possibleRuntimeType} is an Interface type:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Following up on discussion - this probably needs an assertion that {possibleRuntimeType} (maybe {resolvedType}?) implements {abstractType}?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@leebyron

Hmmm. This PR allows the internal type system method to resolve to either an object or to a transitive interface. Everything should be fine as long as the internal type system method returns a type that implements the original abstract type, so an assertion is required.

However, the existing behavior also requires a similar assertion, as well as requiring an assertion that the internal method returned an object type. Perhaps to better clarify the proposed change, we should have a separate PR that adds the current assertions/behavior to the spec, and this PR can then modify/loosen those assertions/behavior as desired.

See: #973

- Return ResolveAbstractTypeImpl(possibleRuntimeType, objectValue).
- Return {possibleRuntimeType}.

**Merging Selection Sets**

Expand Down