From 43bb587d9edf77790ef6d2ba28650241c0c8b72e Mon Sep 17 00:00:00 2001 From: Roger Goldfinger Date: Mon, 1 May 2017 15:37:36 -0700 Subject: [PATCH] Lint for required fields [WIP] (#50) * lint for required fields --- CHANGELOG.md | 1 + README.md | 85 ++++++++++++++++++++++++++++++++++++++++++++- src/index.js | 44 +++++++++++++++++++++-- src/rules.js | 30 ++++++++++++++++ test/makeRule.js | 34 ++++++++++++++++++ test/schema.graphql | 2 ++ 6 files changed, 193 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eae5b13..35baf16 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ### vNEXT - Updated `graphql` dependency to resolve test failures ([wording change](https://github.com/graphql/graphql-js/commit/ba401e154461bca5323ca9121c6dacaee10ebe88), no API change) [jnwng](https://github.com/jnwng) +- Add lint rule to enforce that required fields are specified. [rgoldfinger](https://github.com/rgoldfinger) in [#47](https://github.com/apollographql/eslint-plugin-graphql/pull/50) ### v0.7.0 - Add lint rule to enforce that operations have names [gauravmk](https://github.com/gauravmk) in [#47](https://github.com/apollographql/eslint-plugin-graphql/pull/47) diff --git a/README.md b/README.md index 7c17ba8..8768c19 100644 --- a/README.md +++ b/README.md @@ -263,7 +263,7 @@ query { } ``` -The rule is defined as `graphql/named-operations` and requires a `schema` and optional `tagName` +The rule is defined as `graphql/named-operations` and requires a `schema` and optional `tagName`. ```js // In a file called .eslintrc.js @@ -283,3 +283,86 @@ module.exports = { ] } ``` +### Required Fields Validation Rule + +The Required Fields rule validates that any specified required field is part of the query, but only if that field is available in schema. This is useful to ensure that query results are cached properly in the client. + +**Pass** +``` +// 'uuid' required + +schema { + query { + viewer { + uuid + name + } + } +} + +query ViewerName { + viewer { + name + uuid + } +} +``` + +**Pass** +``` +// 'uuid' required + +schema { + query { + viewer { + name + } + } +} + +query ViewerName { + viewer { + name + } +} +``` + +**Fail** +``` +// 'uuid' required + +schema { + query { + viewer { + uuid + name + } + } +} + +query ViewerName { + viewer { + name + } +} +``` + +The rule is defined as `graphql/required-fields` and requires a `schema` and `requiredFields`, with an optional `tagName`. + +```js +// In a file called .eslintrc.js +module.exports = { + rules: { + 'graphql/required-fields': [ + 'error', + { + schemaJsonFilepath: require('./schema.json'), + requiredFields: ['uuid'], + }, + ], + }, + plugins: [ + 'graphql' + ] +} +``` diff --git a/src/index.js b/src/index.js index c9e3233..336d39f 100644 --- a/src/index.js +++ b/src/index.js @@ -59,11 +59,12 @@ function createRule(context, optionParser) { const tagRules = []; for (const optionGroup of context.options) { const {schema, env, tagName, validators} = optionParser(optionGroup); + const boundValidators = validators.map(v => (ctx) => v(ctx, optionGroup)); if (tagNames.has(tagName)) { throw new Error('Multiple options for GraphQL tag ' + tagName); } tagNames.add(tagName); - tagRules.push({schema, env, tagName, validators}); + tagRules.push({schema, env, tagName, validators: boundValidators}); } return { TaggedTemplateExpression(node) { @@ -145,7 +146,46 @@ const rules = { ...optionGroup, }));; }, - } + }, + 'required-fields': { + meta: { + schema: { + type: 'array', + minLength: 1, + items: { + additionalProperties: false, + properties: { + ...defaultRuleProperties, + requiredFields: { + type: 'array', + items: { + type: 'string', + }, + }, + }, + oneOf: [ + { + required: ['schemaJson'], + not: { required: ['schemaJsonFilepath'] }, + }, + { + required: ['schemaJsonFilepath'], + not: { required: ['schemaJson'] }, + }, + ], + }, + }, + }, + create: context => { + return createRule(context, optionGroup => + parseOptions({ + validators: ['RequiredFields'], + options: { requiredFields: optionGroup.requiredFields }, + ...optionGroup, + }) + ); + }, + }, }; function parseOptions(optionGroup) { diff --git a/src/rules.js b/src/rules.js index 353bf1d..ad06ae5 100644 --- a/src/rules.js +++ b/src/rules.js @@ -12,3 +12,33 @@ export function OperationsMustHaveNames(context) { }; } +export function RequiredFields(context, options) { + return { + Field(node) { + const def = context.getFieldDef(); + const { requiredFields } = options; + requiredFields.forEach(field => { + const fieldAvaliableOnType = def.type && def.type._fields && def.type._fields[field]; + + function recursivelyCheckOnType(ofType, field) { + return (ofType._fields && ofType._fields[field]) || (ofType.ofType && recursivelyCheckOnType(ofType.ofType, field)); + } + + let fieldAvaliableOnOfType = false; + if (def.type && def.type.ofType) { + fieldAvaliableOnOfType = recursivelyCheckOnType(def.type.ofType, field); + } + if (fieldAvaliableOnType || fieldAvaliableOnOfType) { + const fieldWasRequested = !!node.selectionSet.selections.find( + n => (n.name.value === field || n.kind === 'FragmentSpread') + ); + if (!fieldWasRequested) { + context.reportError( + new GraphQLError(`'${field}' field required on '${node.name.value}'`, [node]) + ); + } + } + }); + }, + }; +} diff --git a/test/makeRule.js b/test/makeRule.js index f879de9..1903aac 100644 --- a/test/makeRule.js +++ b/test/makeRule.js @@ -793,6 +793,29 @@ const namedOperationsValidatorCases = { }, }; +const requiredFieldsTestCases = { + pass: [ + 'const x = gql`query { allFilms { films { title } } }`', + 'const x = gql`query { stories { id comments { text } } }`' + ], + fail: [ + { + code: 'const x = gql`query { stories { comments { text } } }`', + errors: [{ + message: `'id' field required on 'stories'`, + type: 'TaggedTemplateExpression', + }], + }, + { + code: 'const x = gql`query { greetings { hello } }`', + errors: [{ + message: `'id' field required on 'greetings'`, + type: 'TaggedTemplateExpression', + }], + }, + ], +}; + { let options = [{ schemaJson, tagName: 'gql', @@ -846,4 +869,15 @@ const namedOperationsValidatorCases = { valid: Object.values(namedOperationsValidatorCases).map(({pass: code}) => ({options, parserOptions, code})), invalid: Object.values(namedOperationsValidatorCases).map(({fail: code, errors}) => ({options, parserOptions, code, errors})), }); + + // Validate the required-fields rule + options = [{ + schemaJson, + tagName: 'gql', + requiredFields: ['id'], + }]; + ruleTester.run('testing required-fields rule', rules['required-fields'], { + valid: requiredFieldsTestCases.pass.map((code) => ({options, parserOptions, code})), + invalid: requiredFieldsTestCases.fail.map(({code, errors}) => ({options, parserOptions, code, errors})), + }); } diff --git a/test/schema.graphql b/test/schema.graphql index 87afc69..5082b44 100644 --- a/test/schema.graphql +++ b/test/schema.graphql @@ -29,6 +29,7 @@ type RootQuery { allFilms: AllFilmsObj sum(a: Int!, b: Int!): Int! greetings: Greetings + stories: [Story!]! } type AllFilmsObj { @@ -42,6 +43,7 @@ type Film { } type Greetings { + id: ID hello: String }