Skip to content

Commit cc9a561

Browse files
dimaMachinadimaMachina
authored
Ignore fragment definition in require id when available rule (#852)
1 parent 530a459 commit cc9a561

File tree

5 files changed

+126
-79
lines changed

5 files changed

+126
-79
lines changed

‎.changeset/shy-mangos-grin.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@graphql-eslint/eslint-plugin': minor
3+
---
4+
5+
feat: ignore fragments in `require-id-when-available` rule

‎docs/rules/require-id-when-available.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ type User {
2323
}
2424

2525
# Query
26-
query user {
26+
query {
2727
user {
2828
name
2929
}
@@ -42,7 +42,7 @@ type User {
4242
}
4343

4444
# Query
45-
query user {
45+
query {
4646
user {
4747
id
4848
name

‎packages/plugin/src/rules/require-id-when-available.ts‎

Lines changed: 40 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,22 @@
11
import { getLocation, requireGraphQLSchemaFromContext, requireSiblingsOperations } from '../utils';
22
import { GraphQLESLintRule } from '../types';
3-
import { GraphQLInterfaceType, GraphQLObjectType, Kind, SelectionNode } from 'graphql';
3+
import { GraphQLInterfaceType, GraphQLObjectType, Kind, SelectionNode, SelectionSetNode } from 'graphql';
4+
import { asArray } from '@graphql-tools/utils';
45
import { getBaseType, GraphQLESTreeNode } from '../estree-parser';
56

6-
const REQUIRE_ID_WHEN_AVAILABLE = 'REQUIRE_ID_WHEN_AVAILABLE';
7-
const DEFAULT_ID_FIELD_NAME = 'id';
7+
export type RequireIdWhenAvailableRuleConfig = { fieldName: string | string[] };
88

9-
type RequireIdWhenAvailableRuleConfig = { fieldName: string };
9+
const RULE_ID = 'require-id-when-available';
10+
const MESSAGE_ID = 'REQUIRE_ID_WHEN_AVAILABLE';
11+
const DEFAULT_ID_FIELD_NAME = 'id';
1012

1113
const rule: GraphQLESLintRule<[RequireIdWhenAvailableRuleConfig], true> = {
1214
meta: {
1315
type: 'suggestion',
1416
docs: {
1517
category: 'Operations',
1618
description: 'Enforce selecting specific fields when they are available on the GraphQL type.',
17-
url: 'https://github.com/dotansimha/graphql-eslint/blob/master/docs/rules/require-id-when-available.md',
19+
url: `https://github.com/dotansimha/graphql-eslint/blob/master/docs/rules/${RULE_ID}.md`,
1820
requiresSchema: true,
1921
requiresSiblings: true,
2022
examples: [
@@ -28,7 +30,7 @@ const rule: GraphQLESLintRule<[RequireIdWhenAvailableRuleConfig], true> = {
2830
}
2931
3032
# Query
31-
query user {
33+
query {
3234
user {
3335
name
3436
}
@@ -45,7 +47,7 @@ const rule: GraphQLESLintRule<[RequireIdWhenAvailableRuleConfig], true> = {
4547
}
4648
4749
# Query
48-
query user {
50+
query {
4951
user {
5052
id
5153
name
@@ -57,7 +59,7 @@ const rule: GraphQLESLintRule<[RequireIdWhenAvailableRuleConfig], true> = {
5759
recommended: true,
5860
},
5961
messages: {
60-
[REQUIRE_ID_WHEN_AVAILABLE]: [
62+
[MESSAGE_ID]: [
6163
`Field {{ fieldName }} must be selected when it's available on a type. Please make sure to include it in your selection set!`,
6264
`If you are using fragments, make sure that all used fragments {{ checkedFragments }}specifies the field {{ fieldName }}.`,
6365
].join('\n'),
@@ -88,21 +90,23 @@ const rule: GraphQLESLintRule<[RequireIdWhenAvailableRuleConfig], true> = {
8890
},
8991
},
9092
create(context) {
91-
requireGraphQLSchemaFromContext('require-id-when-available', context);
92-
const siblings = requireSiblingsOperations('require-id-when-available', context);
93+
requireGraphQLSchemaFromContext(RULE_ID, context);
94+
const siblings = requireSiblingsOperations(RULE_ID, context);
9395
const { fieldName = DEFAULT_ID_FIELD_NAME } = context.options[0] || {};
94-
const idNames = Array.isArray(fieldName) ? fieldName : [fieldName];
96+
const idNames = asArray(fieldName);
9597

9698
const isFound = (s: GraphQLESTreeNode<SelectionNode> | SelectionNode) =>
9799
s.kind === Kind.FIELD && idNames.includes(s.name.value);
98100

101+
// Skip check selections in FragmentDefinition
102+
const selector = 'OperationDefinition SelectionSet[parent.kind!=OperationDefinition]';
103+
99104
return {
100-
SelectionSet(node) {
105+
[selector](node: GraphQLESTreeNode<SelectionSetNode, true>) {
101106
const typeInfo = node.typeInfo();
102107
if (!typeInfo.gqlType) {
103108
return;
104109
}
105-
106110
const rawType = getBaseType(typeInfo.gqlType);
107111
const isObjectType = rawType instanceof GraphQLObjectType;
108112
const isInterfaceType = rawType instanceof GraphQLInterfaceType;
@@ -115,47 +119,43 @@ const rule: GraphQLESLintRule<[RequireIdWhenAvailableRuleConfig], true> = {
115119
if (!hasIdFieldInType) {
116120
return;
117121
}
118-
119122
const checkedFragmentSpreads = new Set<string>();
120-
let found = false;
121123

122124
for (const selection of node.selections) {
123125
if (isFound(selection)) {
124-
found = true;
125-
} else if (selection.kind === Kind.INLINE_FRAGMENT) {
126-
found = selection.selectionSet?.selections.some(s => isFound(s));
127-
} else if (selection.kind === Kind.FRAGMENT_SPREAD) {
126+
return;
127+
}
128+
if (selection.kind === Kind.INLINE_FRAGMENT && selection.selectionSet.selections.some(isFound)) {
129+
return;
130+
}
131+
if (selection.kind === Kind.FRAGMENT_SPREAD) {
128132
const [foundSpread] = siblings.getFragment(selection.name.value);
129-
130133
if (foundSpread) {
131134
checkedFragmentSpreads.add(foundSpread.document.name.value);
132-
found = foundSpread.document.selectionSet?.selections.some(s => isFound(s));
135+
if (foundSpread.document.selectionSet.selections.some(isFound)) {
136+
return;
137+
}
133138
}
134139
}
135-
136-
if (found) {
137-
break;
138-
}
139140
}
140141

141142
const { parent } = node as any;
142143
const hasIdFieldInInterfaceSelectionSet =
143-
parent &&
144-
parent.kind === Kind.INLINE_FRAGMENT &&
145-
parent.parent &&
146-
parent.parent.kind === Kind.SELECTION_SET &&
147-
parent.parent.selections.some(s => isFound(s));
148-
149-
if (!found && !hasIdFieldInInterfaceSelectionSet) {
150-
context.report({
151-
loc: getLocation(node.loc),
152-
messageId: REQUIRE_ID_WHEN_AVAILABLE,
153-
data: {
154-
checkedFragments: checkedFragmentSpreads.size === 0 ? '' : `(${[...checkedFragmentSpreads].join(', ')}) `,
155-
fieldName: idNames.map(name => `"${name}"`).join(' or '),
156-
},
157-
});
144+
parent?.kind === Kind.INLINE_FRAGMENT &&
145+
parent.parent?.kind === Kind.SELECTION_SET &&
146+
parent.parent.selections.some(isFound);
147+
if (hasIdFieldInInterfaceSelectionSet) {
148+
return;
158149
}
150+
151+
context.report({
152+
loc: getLocation(node.loc),
153+
messageId: MESSAGE_ID,
154+
data: {
155+
checkedFragments: checkedFragmentSpreads.size === 0 ? '' : `(${[...checkedFragmentSpreads].join(', ')}) `,
156+
fieldName: idNames.map(name => `"${name}"`).join(' or '),
157+
},
158+
});
159159
},
160160
};
161161
},

‎packages/plugin/tests/__snapshots__/require-id-when-available.spec.ts.snap‎

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,19 @@
11
// Jest Snapshot v1, https://goo.gl/fbAQLP
22

33
exports[` 1`] = `
4-
> 1 | query { hasId { name } }
5-
| ^ Field "id" must be selected when it's available on a type. Please make sure to include it in your selection set!
4+
> 1 | { hasId { name } }
5+
| ^ Field "id" must be selected when it's available on a type. Please make sure to include it in your selection set!
66
If you are using fragments, make sure that all used fragments specifies the field "id".
77
`;
88

99
exports[` 2`] = `
10-
> 1 | query { hasId { id } }
11-
| ^ Field "name" must be selected when it's available on a type. Please make sure to include it in your selection set!
10+
> 1 | { hasId { id } }
11+
| ^ Field "name" must be selected when it's available on a type. Please make sure to include it in your selection set!
1212
If you are using fragments, make sure that all used fragments specifies the field "name".
1313
`;
1414

1515
exports[` 3`] = `
16-
1 |
17-
2 | query {
18-
> 3 | hasId {
19-
| ^ Field "id" or "_id" must be selected when it's available on a type. Please make sure to include it in your selection set!
16+
> 1 | { hasId { name } }
17+
| ^ Field "id" or "_id" must be selected when it's available on a type. Please make sure to include it in your selection set!
2018
If you are using fragments, make sure that all used fragments specifies the field "id" or "_id".
21-
4 | name
22-
5 | }
23-
6 | }
24-
7 |
2519
`;

‎packages/plugin/tests/require-id-when-available.spec.ts‎

Lines changed: 73 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { GraphQLRuleTester, ParserOptions } from '../src';
2-
import rule from '../src/rules/require-id-when-available';
2+
import rule, { RequireIdWhenAvailableRuleConfig } from '../src/rules/require-id-when-available';
33

44
const TEST_SCHEMA = /* GraphQL */ `
55
type Query {
@@ -38,64 +38,112 @@ const TEST_SCHEMA = /* GraphQL */ `
3838
}
3939
`;
4040

41+
const USER_POST_SCHEMA = /* GraphQL */ `
42+
type User {
43+
id: ID
44+
name: String
45+
posts: [Post]
46+
}
47+
48+
type Post {
49+
id: ID
50+
title: String
51+
}
52+
53+
type Query {
54+
user: User
55+
}
56+
`;
57+
4158
const WITH_SCHEMA = {
4259
parserOptions: <ParserOptions>{
4360
schema: TEST_SCHEMA,
44-
operations: /* GraphQL */ `
45-
fragment HasIdFields on HasId {
46-
id
47-
}
48-
`,
61+
operations: '{ foo }',
4962
},
5063
};
64+
5165
const ruleTester = new GraphQLRuleTester();
5266

53-
ruleTester.runGraphQLTests('require-id-when-available', rule, {
67+
ruleTester.runGraphQLTests<[RequireIdWhenAvailableRuleConfig]>('require-id-when-available', rule, {
5468
valid: [
55-
{ ...WITH_SCHEMA, code: `query { noId { name } }` },
56-
{ ...WITH_SCHEMA, code: `query { hasId { id name } }` },
57-
{ ...WITH_SCHEMA, code: `query { hasId { ...HasIdFields } }` },
58-
{ ...WITH_SCHEMA, code: `query { vehicles { id ...on Car { id mileage } } }` },
59-
{ ...WITH_SCHEMA, code: `query { vehicles { ...on Car { id mileage } } }` },
60-
{ ...WITH_SCHEMA, code: `query { flying { ...on Bird { id } } }` },
69+
{
70+
name: 'should completely ignore FragmentDefinition',
71+
code: /* GraphQL */ `
72+
fragment UserFields on User {
73+
name
74+
posts {
75+
title
76+
}
77+
}
78+
`,
79+
parserOptions: {
80+
schema: USER_POST_SCHEMA,
81+
operations: '{ foo }',
82+
},
83+
},
84+
{
85+
name: "should ignore checking selections on OperationDefinition as it's redundant check",
86+
code: '{ foo }',
87+
parserOptions: {
88+
schema: 'type Query { id: ID }',
89+
operations: '{ foo }',
90+
},
91+
},
92+
{ ...WITH_SCHEMA, code: '{ noId { name } }' },
93+
{ ...WITH_SCHEMA, code: '{ hasId { id name } }' },
94+
{
95+
name: 'should find selection in fragment',
96+
code: '{ hasId { ...HasIdFields } }',
97+
parserOptions: {
98+
schema: TEST_SCHEMA,
99+
operations: 'fragment HasIdFields on HasId { id }',
100+
},
101+
},
102+
{ ...WITH_SCHEMA, code: '{ vehicles { id ...on Car { id mileage } } }' },
103+
{ ...WITH_SCHEMA, code: '{ vehicles { ...on Car { id mileage } } }' },
104+
{ ...WITH_SCHEMA, code: '{ flying { ...on Bird { id } } }' },
61105
{
62106
...WITH_SCHEMA,
63-
code: `query { hasId { name } }`,
107+
code: '{ hasId { name } }',
64108
options: [{ fieldName: 'name' }],
65109
},
66110
{
67111
...WITH_SCHEMA,
68-
code: `query { vehicles { id ...on Car { mileage } } }`,
112+
code: '{ vehicles { id ...on Car { mileage } } }',
69113
},
70114
{
71115
...WITH_SCHEMA,
72116
name: 'support multiple id field names',
73-
code: `query { hasId { _id } }`,
117+
code: '{ hasId { _id } }',
74118
options: [{ fieldName: ['id', '_id'] }],
75119
},
76120
],
77121
invalid: [
122+
// TODO: Improve this
123+
// {
124+
// name: 'should report an error about missing "posts.id" selection',
125+
// code: '{ user { id ...UserFields } }',
126+
// errors: [{ message: 'REQUIRE_ID_WHEN_AVAILABLE' }],
127+
// parserOptions: {
128+
// schema: USER_POST_SCHEMA,
129+
// operations: 'fragment UserFields on User { posts { title } }'
130+
// },
131+
// },
78132
{
79133
...WITH_SCHEMA,
80-
code: `query { hasId { name } }`,
134+
code: '{ hasId { name } }',
81135
errors: [{ messageId: 'REQUIRE_ID_WHEN_AVAILABLE' }],
82136
},
83137
{
84138
...WITH_SCHEMA,
85-
code: `query { hasId { id } }`,
139+
code: '{ hasId { id } }',
86140
options: [{ fieldName: 'name' }],
87141
errors: [{ messageId: 'REQUIRE_ID_WHEN_AVAILABLE' }],
88142
},
89143
{
90144
...WITH_SCHEMA,
91145
name: 'support multiple id field names',
92-
code: /* GraphQL */ `
93-
query {
94-
hasId {
95-
name
96-
}
97-
}
98-
`,
146+
code: '{ hasId { name } }',
99147
options: [{ fieldName: ['id', '_id'] }],
100148
errors: [{ messageId: 'REQUIRE_ID_WHEN_AVAILABLE' }],
101149
},

0 commit comments

Comments
 (0)