graphql / graphql-spec

GraphQL is a query language and execution engine tied to any backend service.
https://spec.graphql.org
14.29k stars 1.12k forks source link

graphql queries with type condition on nested fields [Question] #494

Closed yoduoy closed 6 years ago

yoduoy commented 6 years ago

I am working on the following types, where the "content" of a "Comment" is a union type:

type TextContent {
  text: String
}

type RichContent {
  participants: [String]
  startTime: String
}

union Content = TextContent | RichContent

type Comment {
  id: ID
  sender: String
  content: Content
}

type Review {
  id: ID
  title: String
  lastComment: Comment
}

In my query, I was trying to use conditional fragments on the two Content types:

query listOfReviews {
  reviews {
    ...reviewFields
  }
}

fragment reviewFields on Review {
  id
  title
  lastComment {
    content {
      ... on TextContent {
        text
      }
      ... on RichContent {
        participants
        startTime
      }
    }
  }
}

I received a runtime error where the graphql engine (we are using Apollo) seems trying to access "participants" field of "undefined", where the actual content object is:

{
  __typename: "TextContent:,
  text: "abc"
}

It looks the two types of the union Content are merged together.

My question is: is it allowed to use type conditions on nested fields in graphql queries? Or type conditions have to be used on the top level types returned by the queries? If it's allowed, how should I fix my types/queries?

Thanks a lot!

IvanGoncharov commented 6 years ago

My question is: is it allowed to use type conditions on nested fields in graphql queries?

Yes absolutely, it's actually a primary use case for inline fragments: http://facebook.github.io/graphql/June2018/#example-a6b78

It's an issue with Apollo and you should definetly report it in their repo.

yoduoy commented 6 years ago

Thanks @IvanGoncharov. The example you referenced only has conditional fragments on top-level types (types returned by the query). I searched online but could not find examples with conditional fragments on nested (internal) types (as in my example). Are you aware of any online resources with conditional fragments on nested types?

I've posted the question at apollo-client https://github.com/apollographql/apollo-client/issues/3802.

IvanGoncharov commented 6 years ago

@yoduoy From specification text there is no difference between top-level and other fields, it's working using the same algorithm: http://facebook.github.io/graphql/June2018/#sec-Field-Collection Also here is validation rule for inline fragments: http://facebook.github.io/graphql/June2018/#sec-Fragment-spread-is-possible As you see both algorithms don't restrict inline fragments to the top-level fields.

yoduoy commented 6 years ago

Thanks @IvanGoncharov! I am still not able to figure it out. A little bit more details:

The "cannot read property of undefined" error was thrown at the following places in apollo-link-state/lib/index.js:

43                var resolverMap = resolvers[rootValue.__typename || type];
44                var resolve = resolverMap[fieldName];

Where rootValue.typename is "TextContent" and fieldName is "participants". Since resolvers does not contain "TextContent" (because even if we want to define the resolveType function in the resolvers, it should be defined for the union Content not specific types TextContent/RichContent), resolverMap is undefined, thus causing the "cannot read property participants of undefined" error.

To check why Apollo tried to access the "participant" field of type "TextContent", I debugged into the fragment matching process stopped at the following lines in graphql-anywhere/lib/async.js:

117  if (!execContext.fragmentMatcher(rootValue, typeCondition, contextValue)) return [3, 4];
118     return [4, executeSelectionSet$1(fragment.selectionSet, rootValue, execContext)];

When debugging into fragmentMatcher, we see the following (line 64):

58 function graphql$1(resolver, document, rootValue, contextValue, variableValues, execOptions) {
59     if (execOptions === void 0) { execOptions = {}; }
60     var mainDefinition = apolloUtilities.getMainDefinition(document);
61     var fragments = apolloUtilities.getFragmentDefinitions(document);
62     var fragmentMap = apolloUtilities.createFragmentMap(fragments);
63     var resultMapper = execOptions.resultMapper;
64     var fragmentMatcher = execOptions.fragmentMatcher || (function () { return true; });

We can check that graphql is initialized from graphql-link-state/lib/index.js without passing fragmentMatcher, so the default matcher always returns true, which means the fragmentMatcher check passed and executeSelectionSet will be called recursively.

So it looks to me Apollo was not able to match the fragment for nested union types. Is this a known issue or it is because my usage is incorrect somewhere?

Advice is appreciated!

IvanGoncharov commented 6 years ago

So it looks to me Apollo was not able to match the fragment for nested union types. Is this a known issue or it is because my usage is incorrect somewhere?

@yoduoy Never used Apollo so can't help with this part, but your query is 100% valid against GraphQL Specification. You need to reach out to Apollo community since this repo only dedicated to GraphQL spec.

stubailo commented 6 years ago

@yoduoy please open a new issue here: https://www.github.com/apollographql/apollo-client

yoduoy commented 6 years ago

@const86 helped point out that this is due to this bug: https://github.com/apollographql/apollo-link-state/pull/258. Thanks everyone for helping!

CanRau commented 5 years ago

Just for reference the before mentioned links are not properly redirecting so here they are