graphql/required-fields rule ignores presence of required field in spread fragment

[fnune]

trafficstars

My configuration:

    'graphql/required-fields': [
      'error',
      {
        env: 'apollo',
        schemaJson: require('./schema.json'),
        requiredFields: ['id'],
      },
      {
        env: 'literal',
        schemaJson: require('./schema.json'),
        requiredFields: ['id'],
      },
    ],

My query:

query Main {
  nestedField {
    ...NestedField
  }
}

fragment NestedField on SomeType {
  id
  someField
}

Expected result

No linter error.

Actual result

/path/MyQuery.graphql
  2:3  error  'id' field required on 'nestedField'  graphql/required-fields

Apparently the required-fields rule doesn't account for fields included in a fragment.

Update

It seems like field selection merging is part of the GraphQL specification: https://graphql.github.io/graphql-spec/draft/#sec-Field-Selection-Merging

So this is not really a bug per se but more a feature request to be more exhaustive in checking for the inclusion of required fiels in fragments.

[jameswritescode]

Similar issue, using graphql-tag/loader with with a literal importing a fragment like this results in a required-fields error if requiredFields: ['id']:

#import "./MessageFragment.graphql"

mutation createMessage($input: MessageInput!) {
  createMessage(input: $input) {
    ...MessageFragment
  }
} 

fragment MessageFragment on Message {
  id
  snippedOtherFields
}

[moimael]

Seems like it is the intended behaviour if I understand this comment properly: https://github.com/apollographql/eslint-plugin-graphql/blob/d9aa7a479bf8cc5063f416b3d1ac235d22cb92ab/src/customGraphQLValidationRules.js#L106

[fnune]

I don't completely understand the comment, though. What does it mean for a fragment to cover all of the possible type options?

[moimael]

I guess when you have something like this in your fragment:

... on FirstType {
  id
}

But the parent supports:

... on FirstType {
  id
}

... on SecondType {
  id
}

or

parent {
  id
  ... on FirstType {
	...
  }

  ... on SecondType {
    ...
  }
}

At least this is how I understand it.

[fnune]

I suppose a decision was taken not to implement checking fragments exhaustively to see if they include a required field.

[moimael]

Yeah I guess so, not sure how hard it would be to implement that though

[fnune]

It seems like field selection merging is part of the GraphQL specification: https://graphql.github.io/graphql-spec/draft/#sec-Field-Selection-Merging

So this is not really a bug per se but more a feature request to be more exhaustive in checking for the inclusion of required fiels in fragments.