Skip to content

Commit 56d6ff3

Browse files
dimaMachinadimaMachina
authored
[known-fragment-names]: fix when fragment used on interface and union (#991)
* [known-fragment-names]: fix when fragment used on interface and union * fix build
1 parent 3fe3761 commit 56d6ff3

File tree

5 files changed

+118
-48
lines changed

5 files changed

+118
-48
lines changed

‎.changeset/neat-apes-doubt.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': patch
3+
---
4+
5+
[known-fragment-names]: fix when fragment used on interface and union

‎.github/workflows/tests.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ jobs:
6060
${{runner.os}}-16-8-${{matrix.graphql_version}}-node-modules-${{hashFiles('yarn.lock')}}
6161
${{runner.os}}-16-8-${{matrix.graphql_version}}-node-modules-
6262
- name: Use GraphQL v${{matrix.graphql_version}}
63-
run: node ./scripts/match-graphql.js ${{matrix.graphql_version}}
63+
run: node ./scripts/match-graphql.mjs ${{matrix.graphql_version}}
6464
- name: Install Dependencies using Yarn
6565
run: yarn install && git checkout yarn.lock
6666
- name: Build
@@ -102,7 +102,7 @@ jobs:
102102
${{runner.os}}-${{matrix.node_version}}-${{matrix.eslint_version}}-${{matrix.graphql_version}}-node-modules-${{hashFiles('yarn.lock')}}
103103
${{runner.os}}-${{matrix.node_version}}-${{matrix.eslint_version}}-${{matrix.graphql_version}}-node-modules-
104104
- name: Use GraphQL v${{matrix.graphql_version}}
105-
run: node ./scripts/match-graphql.js ${{matrix.graphql_version}}
105+
run: node ./scripts/match-graphql.mjs ${{matrix.graphql_version}}
106106
- name: Use ESLint v${{matrix.eslint_version}}
107107
run: node scripts/match-eslint.mjs ${{matrix.eslint_version}}
108108
- name: Install Dependencies using Yarn

‎packages/plugin/src/rules/graphql-js-validation.ts

Lines changed: 22 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,14 @@ import type { AST } from 'eslint';
22
import type { JSONSchema4 } from 'json-schema';
33
import {
44
Kind,
5-
TypeInfo,
65
DocumentNode,
76
GraphQLSchema,
87
ValidationRule,
98
FragmentDefinitionNode,
109
OperationDefinitionNode,
1110
visit,
1211
validate,
13-
visitWithTypeInfo,
12+
ASTVisitor,
1413
} from 'graphql';
1514
import { validateSDL } from 'graphql/validation/validate';
1615
import type { GraphQLESLintRule, GraphQLESLintRuleContext } from '../types';
@@ -65,59 +64,47 @@ function validateDocument(
6564
}
6665
}
6766

68-
type FragmentInfo = `${string}:${string}`;
67+
type GetFragmentDefsAndFragmentSpreads = {
68+
fragmentDefs: Set<string>;
69+
fragmentSpreads: Set<string>;
70+
};
6971

70-
const getFragmentDefsAndFragmentSpreads = (
71-
schema: GraphQLSchema,
72-
node: DocumentNode
73-
): {
74-
fragmentDefs: Set<FragmentInfo>;
75-
fragmentSpreads: Set<FragmentInfo>;
76-
} => {
77-
const typeInfo = new TypeInfo(schema);
78-
const fragmentDefs = new Set<FragmentInfo>();
79-
const fragmentSpreads = new Set<FragmentInfo>();
72+
const getFragmentDefsAndFragmentSpreads = (node: DocumentNode): GetFragmentDefsAndFragmentSpreads => {
73+
const fragmentDefs = new Set<string>();
74+
const fragmentSpreads = new Set<string>();
8075

81-
const visitor = visitWithTypeInfo(typeInfo, {
76+
const visitor: ASTVisitor = {
8277
FragmentDefinition(node) {
83-
fragmentDefs.add(`${node.name.value}:${node.typeCondition.name.value}`);
78+
fragmentDefs.add(node.name.value);
8479
},
8580
FragmentSpread(node) {
86-
const parentType = typeInfo.getParentType();
87-
if (parentType) {
88-
fragmentSpreads.add(`${node.name.value}:${parentType.name}`);
89-
}
81+
fragmentSpreads.add(node.name.value);
9082
},
91-
});
83+
};
9284

9385
visit(node, visitor);
9486
return { fragmentDefs, fragmentSpreads };
9587
};
9688

97-
const getMissingFragments = (schema: GraphQLSchema, node: DocumentNode): FragmentInfo[] => {
98-
const { fragmentDefs, fragmentSpreads } = getFragmentDefsAndFragmentSpreads(schema, node);
89+
const getMissingFragments = (node: DocumentNode): string[] => {
90+
const { fragmentDefs, fragmentSpreads } = getFragmentDefsAndFragmentSpreads(node);
9991
return [...fragmentSpreads].filter(name => !fragmentDefs.has(name));
10092
};
10193

10294
type GetDocumentNode = (props: {
10395
ruleId: string;
10496
context: GraphQLESLintRuleContext;
105-
schema: GraphQLSchema;
10697
node: DocumentNode;
10798
}) => DocumentNode;
10899

109-
const handleMissingFragments: GetDocumentNode = ({ ruleId, context, schema, node }) => {
110-
const missingFragments = getMissingFragments(schema, node);
100+
const handleMissingFragments: GetDocumentNode = ({ ruleId, context, node }) => {
101+
const missingFragments = getMissingFragments(node);
111102
if (missingFragments.length > 0) {
112103
const siblings = requireSiblingsOperations(ruleId, context);
113104
const fragmentsToAdd: FragmentDefinitionNode[] = [];
114105

115-
for (const missingFragment of missingFragments) {
116-
const [fragmentName, fragmentTypeName] = missingFragment.split(':');
117-
const [foundFragment] = siblings
118-
.getFragment(fragmentName)
119-
.map(source => source.document)
120-
.filter(fragment => fragment.typeCondition.name.value === fragmentTypeName);
106+
for (const fragmentName of missingFragments) {
107+
const [foundFragment] = siblings.getFragment(fragmentName).map(source => source.document);
121108
if (foundFragment) {
122109
fragmentsToAdd.push(foundFragment);
123110
}
@@ -128,7 +115,6 @@ const handleMissingFragments: GetDocumentNode = ({ ruleId, context, schema, node
128115
return handleMissingFragments({
129116
ruleId,
130117
context,
131-
schema,
132118
node: {
133119
kind: Kind.DOCUMENT,
134120
definitions: [...node.definitions, ...fragmentsToAdd],
@@ -182,7 +168,7 @@ const validationToRule = (
182168
const schema = docs.requiresSchema ? requireGraphQLSchemaFromContext(ruleId, context) : null;
183169

184170
const documentNode = getDocumentNode
185-
? getDocumentNode({ ruleId, context, schema, node: node.rawNode() })
171+
? getDocumentNode({ ruleId, context, node: node.rawNode() })
186172
: node.rawNode();
187173

188174
validateDocument(context, schema, documentNode, ruleFn);
@@ -360,7 +346,7 @@ export const GRAPHQL_JS_VALIDATIONS: Record<string, GraphQLESLintRule> = Object.
360346
requiresSchema: true,
361347
requiresSiblings: true,
362348
},
363-
({ ruleId, context, schema, node }) => {
349+
({ ruleId, context, node }) => {
364350
const siblings = requireSiblingsOperations(ruleId, context);
365351
const FilePathToDocumentsMap = [...siblings.getOperations(), ...siblings.getFragments()].reduce<
366352
Record<string, (OperationDefinitionNode | FragmentDefinitionNode)[]>
@@ -371,15 +357,15 @@ export const GRAPHQL_JS_VALIDATIONS: Record<string, GraphQLESLintRule> = Object.
371357
}, Object.create(null));
372358

373359
const getParentNode = (currentFilePath: string, node: DocumentNode): DocumentNode => {
374-
const { fragmentDefs } = getFragmentDefsAndFragmentSpreads(schema, node);
360+
const { fragmentDefs } = getFragmentDefsAndFragmentSpreads(node);
375361
if (fragmentDefs.size === 0) {
376362
return node;
377363
}
378364
// skip iteration over documents for current filepath
379365
delete FilePathToDocumentsMap[currentFilePath];
380366

381367
for (const [filePath, documents] of Object.entries(FilePathToDocumentsMap)) {
382-
const missingFragments = getMissingFragments(schema, {
368+
const missingFragments = getMissingFragments({
383369
kind: Kind.DOCUMENT,
384370
definitions: documents,
385371
});

‎packages/plugin/tests/known-fragment-names.spec.ts

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,89 @@ ruleTester.runGraphQLTests('known-fragment-names', rules['known-fragment-names']
3939
],
4040
},
4141
},
42+
{
43+
name: 'should work when interface implemented',
44+
code: /* GraphQL */ `
45+
fragment Introduction on Introduction {
46+
introText {
47+
...ContentUnit
48+
}
49+
}
50+
`,
51+
parserOptions: {
52+
schema: /* GraphQL */ `
53+
interface ContentUnit {
54+
contentSets: Int
55+
}
56+
57+
type IntroText implements ContentUnit {
58+
contentSets: Int
59+
}
60+
61+
type Introduction {
62+
introText: IntroText
63+
}
64+
65+
type Query {
66+
foo: Int
67+
}
68+
`,
69+
operations: /* GraphQL */ `
70+
fragment ContentUnit on ContentUnit {
71+
contentSets {
72+
id
73+
}
74+
}
75+
`,
76+
},
77+
},
78+
{
79+
name: 'should work when with union',
80+
code: /* GraphQL */ `
81+
query {
82+
animal {
83+
...AnimalFields
84+
}
85+
}
86+
`,
87+
parserOptions: {
88+
schema: /* GraphQL */ `
89+
type Cat {
90+
name: String
91+
}
92+
93+
type Dog {
94+
age: String
95+
}
96+
97+
union AnimalUnion = Cat | Dog
98+
99+
type Animal {
100+
animal: AnimalUnion
101+
}
102+
103+
type Query {
104+
animal: Animal
105+
}
106+
`,
107+
operations: /* GraphQL */ `
108+
fragment CatFields on Cat {
109+
title
110+
}
111+
112+
fragment DogFields on Dog {
113+
url
114+
}
115+
116+
fragment AnimalFields on AnimalUnion {
117+
animal {
118+
...CatFields
119+
...DogFields
120+
}
121+
}
122+
`,
123+
},
124+
},
42125
],
43126
invalid: [
44127
{

‎scripts/match-graphql.js renamed to ‎scripts/match-graphql.mjs

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,14 @@
1-
const { writeFileSync } = require('fs');
2-
const { resolve } = require('path');
3-
const { argv } = require('process');
1+
import { readFileSync, writeFileSync } from 'node:fs';
2+
import { resolve } from 'node:path';
43

5-
const pkgPath = resolve(__dirname, '../package.json');
4+
const pkgPath = resolve(process.cwd(), 'package.json');
5+
const version = process.argv[2];
6+
const pkg = JSON.parse(readFileSync(pkgPath));
67

7-
const pkg = require(pkgPath);
8-
9-
const version = argv[2];
10-
11-
pkg.resolutions = pkg.resolutions || {};
128
if (pkg.resolutions.graphql.startsWith(version)) {
139
// eslint-disable-next-line no-console
1410
console.info(`GraphQL v${version} is match! Skipping.`);
15-
return;
11+
process.exit();
1612
}
1713

1814
const npmVersion = version.includes('-') ? version : `^${version}`;

0 commit comments

Comments
 (0)