From dfdda8628efc2ff8489830fcc774acf8121afbc1 Mon Sep 17 00:00:00 2001 From: Tom Picton Date: Thu, 8 May 2025 10:09:57 -0400 Subject: [PATCH 1/4] Add failing test --- .../rules/require-selections/index.test.ts | 30 ++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/packages/plugin/src/rules/require-selections/index.test.ts b/packages/plugin/src/rules/require-selections/index.test.ts index 91dddb0654d..dbeeda4a4c3 100644 --- a/packages/plugin/src/rules/require-selections/index.test.ts +++ b/packages/plugin/src/rules/require-selections/index.test.ts @@ -55,6 +55,7 @@ const USER_POST_SCHEMA = /* GraphQL */ ` type Post { id: ID title: String + content: String author: [User!]! } @@ -316,6 +317,30 @@ ruleTester.run('require-selections', rule, { } `, }, + { + name: 'should require only extant fields with `requireAllFields` option', + code: /* GraphQL */` + { + user { + id + posts { + id + title + content + } + } + } + `, + options: [{ requireAllFields: true, fieldName: ['id', 'title', 'content'] }], + parserOptions: { + graphQLConfig: { + schema: USER_POST_SCHEMA, + documents: ` + fragment Example on User { id } + ` + }, + }, + }, ], invalid: [ { @@ -523,7 +548,10 @@ ruleTester.run('require-selections', rule, { documents: '{ foo }', }, }, - errors: 1, + errors: [{ + message: "Field `hasId.name` must be selected when it's available on a type.\n" + + 'Include it in your selection set.', + }] }, ], }); From 8e874b59baa2535f7edc5e271fc95f8dbc17e01f Mon Sep 17 00:00:00 2001 From: Tom Picton Date: Thu, 8 May 2025 10:10:20 -0400 Subject: [PATCH 2/4] Implement fix --- packages/plugin/src/rules/require-selections/index.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/plugin/src/rules/require-selections/index.ts b/packages/plugin/src/rules/require-selections/index.ts index 12748fe0c84..ce9c4b285a6 100644 --- a/packages/plugin/src/rules/require-selections/index.ts +++ b/packages/plugin/src/rules/require-selections/index.ts @@ -201,20 +201,20 @@ export const rule: GraphQLESLintRule = { function checkFields(rawType: GraphQLInterfaceType | GraphQLObjectType) { const fields = rawType.getFields(); - const hasIdFieldInType = idNames.some(name => fields[name]); + const idFieldsInType = idNames.filter(name => fields[name]); - if (!hasIdFieldInType) { + if (idFieldsInType.length === 0) { return; } checkFragments(node as GraphQLESTreeNode); if (requireAllFields) { - for (const idName of idNames) { + for (const idName of idFieldsInType) { report([idName]); } } else { - report(idNames); + report(idFieldsInType); } } From 30e5334eb65867331d0256e15010ba0a135a4d22 Mon Sep 17 00:00:00 2001 From: Tom Picton Date: Thu, 8 May 2025 10:13:31 -0400 Subject: [PATCH 3/4] Formatting --- .../rules/require-selections/index.test.ts | 35 ++++++++++--------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/packages/plugin/src/rules/require-selections/index.test.ts b/packages/plugin/src/rules/require-selections/index.test.ts index dbeeda4a4c3..7fbe25b751b 100644 --- a/packages/plugin/src/rules/require-selections/index.test.ts +++ b/packages/plugin/src/rules/require-selections/index.test.ts @@ -319,25 +319,25 @@ ruleTester.run('require-selections', rule, { }, { name: 'should require only extant fields with `requireAllFields` option', - code: /* GraphQL */` - { - user { - id - posts { - id - title - content - } - } + code: /* GraphQL */ ` + { + user { + id + posts { + id + title + content } - `, + } + } + `, options: [{ requireAllFields: true, fieldName: ['id', 'title', 'content'] }], parserOptions: { graphQLConfig: { schema: USER_POST_SCHEMA, documents: ` fragment Example on User { id } - ` + `, }, }, }, @@ -548,10 +548,13 @@ ruleTester.run('require-selections', rule, { documents: '{ foo }', }, }, - errors: [{ - message: "Field `hasId.name` must be selected when it's available on a type.\n" + - 'Include it in your selection set.', - }] + errors: [ + { + message: + "Field `hasId.name` must be selected when it's available on a type.\n" + + 'Include it in your selection set.', + }, + ], }, ], }); From 095aa665ade7ec2fa8a82a85b19c90d21d77ea77 Mon Sep 17 00:00:00 2001 From: Tom Picton Date: Thu, 8 May 2025 10:26:45 -0400 Subject: [PATCH 4/4] Docs --- packages/plugin/src/rules/require-selections/index.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/plugin/src/rules/require-selections/index.ts b/packages/plugin/src/rules/require-selections/index.ts index ce9c4b285a6..a77a352e328 100644 --- a/packages/plugin/src/rules/require-selections/index.ts +++ b/packages/plugin/src/rules/require-selections/index.ts @@ -201,8 +201,11 @@ export const rule: GraphQLESLintRule = { function checkFields(rawType: GraphQLInterfaceType | GraphQLObjectType) { const fields = rawType.getFields(); + + // Only check fields which are actually present on this type const idFieldsInType = idNames.filter(name => fields[name]); + // Type doesn't implement any of the `fieldNames` if (idFieldsInType.length === 0) { return; } @@ -211,9 +214,13 @@ export const rule: GraphQLESLintRule = { if (requireAllFields) { for (const idName of idFieldsInType) { + // For each `fieldName` present on this type, check it's in the + // selection separately report([idName]); } } else { + // Pass the `fieldNames` present on this type together; `report` will + // pass if _any_ of them are selected report(idFieldsInType); } }