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

Take subtypes into account when matching type conditions to extract representations #804

Merged
merged 3 commits into from
Jun 10, 2021

Conversation

martijnwalraven
Copy link
Contributor

@martijnwalraven martijnwalraven commented Jun 9, 2021

The executeSelectionSet function in executeQueryPlan.ts, which is used to extract field values from a result object to build up representations to send to a subgraph, only looked for exact matches of the __typename to the type condition on an inline fragment. This wasn't an issue before, because the query plans we generated always contained an inline fragment per requested object type, never an abstract type condition.

Recent changes in the way we handle scope surfaced this issue however, because it would sometimes wrap things in an additional inline fragment for a parent interface type (this has since been fixed in #805).

Although that means the issue won't arise for now, we do expect to make other changes that would require support for abstract type conditions (avoiding exploding types when they share an interface with a @key directive).

@martijnwalraven martijnwalraven changed the title Take subtypes into account when matching type conditions Take subtypes into account when matching type conditions on representations Jun 9, 2021
@martijnwalraven martijnwalraven force-pushed the representations-typecondition-fix branch from c6b36c8 to 14acb4c Compare June 10, 2021 11:32
@martijnwalraven martijnwalraven marked this pull request as ready for review June 10, 2021 11:35
Copy link
Contributor

@pcmanus pcmanus left a comment

Choose a reason for hiding this comment

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

The commit message could use some explanatory notes but lgtm otherwise :)

@martijnwalraven
Copy link
Contributor Author

The commit message could use some explanatory notes but lgtm otherwise :)

Just added some notes to the PR description, so I'll use that as the commit message.

@martijnwalraven martijnwalraven changed the title Take subtypes into account when matching type conditions on representations Take subtypes into account when matching type conditions to extract representations Jun 10, 2021
@martijnwalraven martijnwalraven merged commit 60b43d6 into main Jun 10, 2021
@martijnwalraven martijnwalraven deleted the representations-typecondition-fix branch June 10, 2021 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants