required-fields: inline fragments without a field ancestor

[henryqdineen]

We are planning to use the required-fields rule to enforce the id field but we have some code that was causing TypeError: Cannot read property 'selectionSet' of undefined errors.

I tracked down the issue to the case where an inline fragment is missing a required field but does not have a field ancestor. The existing code for the rule will walk up the ancestors until it finds a field, but an inline fragment can be part of any selection set and not only a field (in our case a fragment definition). This example based on the unit tests should demonstrate the issue:

fragment GreetingOrStoryFragment on GreetingOrStory {
  ... on Greetings {
    hello
  }
}

The id field is missing from the inline fragment My fix was just to walk up the ancestors until it fields the first field, operation definition or fragment definition. It basically stops when it finds something that contains a selection set with the exception of an inline fragment, which we intentionally keep walking up.

TODO:

• Make sure all of the significant new logic is covered by tests
• Rebase your changes on master so that they can be merged easily
• Make sure all tests pass
• Update CHANGELOG.md with your change
• If this was a change that affects the external API, update the README

[henryqdineen]

Looks like the build is failing for unrelated reasons. cc @jnwng

[henryqdineen]

Build is now passing. Let me know if there is anything I can do for this PR. Thanks!

[phryneas]

Could this please get merged? We are running into the same problems

[henryqdineen]

I just added some valid cases to the required-fields test suite to ensure there are no regressions from these changes.

One thing I noticed is that query { greetingOrStory { id ... on Greetings { hello } } } is "valid" according to this rule but would actually fail the FieldsOnCorrectType built-in validation: GraphQLError: Cannot query field \"id\" on type \"GreetingOrStory\". Did you mean to use an inline fragment on \"Greetings\" or \"Story\"?", This is true for this branch and also for master so I'm assuming it's either a separate bug or outside of the scope of this rule and should be caught by FieldsOnCorrectType.

[jnwng]

thanks for adding the test cases @henryqdineen!

[jnwng]

live in [email protected]

[henryqdineen]

Thanks Jon!