Skip to content

Commit 67d0527

Browse files
author
dimitri
committed
fix: use skipGraphQLImport for siblings operations
1 parent e276552 commit 67d0527

13 files changed

Lines changed: 164 additions & 58 deletions

‎.changeset/witty-beans-turn.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+
fix unique-fragment-name and no-unused-fragments rule

‎docs/rules/no-unused-fragments.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
- Category: `Validation`
44
- Rule name: `@graphql-eslint/no-unused-fragments`
55
- Requires GraphQL Schema: `true` [ℹ️](../../README.md#extended-linting-rules-with-graphql-schema)
6-
- Requires GraphQL Operations: `false` [ℹ️](../../README.md#extended-linting-rules-with-siblings-operations)
6+
- Requires GraphQL Operations: `true` [ℹ️](../../README.md#extended-linting-rules-with-siblings-operations)
77

88
A GraphQL document is only valid if all fragment definitions are spread within operations, or spread within other fragments spread within operations.
99

‎packages/plugin/package.json‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,22 +16,23 @@
1616
"prepack": "bob prepack"
1717
},
1818
"dependencies": {
19-
"@graphql-tools/utils": "~7.10.0",
20-
"@graphql-tools/load": "~6.2.0",
19+
"@graphql-tools/code-file-loader": "~6.3.0",
2120
"@graphql-tools/graphql-file-loader": "~6.2.0",
21+
"@graphql-tools/graphql-tag-pluck": "~6.5.0",
22+
"@graphql-tools/import": "^6.3.1",
2223
"@graphql-tools/json-file-loader": "~6.2.6",
23-
"@graphql-tools/code-file-loader": "~6.3.0",
24+
"@graphql-tools/load": "~6.2.0",
2425
"@graphql-tools/url-loader": "~6.10.0",
25-
"@graphql-tools/graphql-tag-pluck": "~6.5.0",
26+
"@graphql-tools/utils": "~7.10.0",
2627
"graphql-config": "^3.2.0",
2728
"graphql-depth-limit": "1.1.0"
2829
},
2930
"devDependencies": {
30-
"@types/graphql-depth-limit": "1.1.2",
3131
"@types/eslint": "7.2.13",
32+
"@types/graphql-depth-limit": "1.1.2",
3233
"bob-the-bundler": "1.2.1",
33-
"graphql-config": "3.3.0",
3434
"graphql": "15.5.1",
35+
"graphql-config": "3.3.0",
3536
"typescript": "4.3.5"
3637
},
3738
"peerDependencies": {

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

Lines changed: 52 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
import { validate, GraphQLSchema, DocumentNode, ASTNode, ValidationRule } from 'graphql';
22
import { validateSDL } from 'graphql/validation/validate';
3-
import { GraphQLFileLoader } from '@graphql-tools/graphql-file-loader';
4-
import fs from 'fs';
3+
import { parseImportLine, processImport } from '@graphql-tools/import';
4+
import { existsSync } from 'fs';
5+
import { join, dirname } from 'path';
56
import { GraphQLESLintRule, GraphQLESLintRuleContext } from '../types';
6-
import { requireGraphQLSchemaFromContext } from '../utils';
7+
import { requireGraphQLSchemaFromContext, requireSiblingsOperations } from '../utils';
78
import { GraphQLESTreeNode } from '../estree-parser';
89

910
function extractRuleName(stack: string | undefined): string | null {
@@ -24,7 +25,7 @@ export function validateDoc(
2425
rules: ReadonlyArray<ValidationRule>,
2526
ruleName: string | null = null
2627
): void {
27-
if (documentNode && documentNode.definitions && documentNode.definitions.length > 0) {
28+
if (documentNode?.definitions?.length > 0) {
2829
try {
2930
const validationErrors = schema ? validate(schema, documentNode, rules) : validateSDL(documentNode, null, rules);
3031

@@ -69,19 +70,17 @@ const validationToRule = (
6970
}
7071

7172
const requiresSchema = docs.requiresSchema ?? true;
72-
73+
const requiresSiblings = docs.requiresSiblings ?? false;
7374
return {
7475
[name]: {
7576
meta: {
7677
docs: {
78+
...docs,
7779
category: 'Validation',
7880
requiresSchema,
79-
requiresSiblings: false,
81+
requiresSiblings,
8082
url: `https://github.com/dotansimha/graphql-eslint/blob/master/docs/rules/${name}.md`,
81-
...docs,
82-
description:
83-
docs.description +
84-
`\n\n> This rule is a wrapper around a \`graphql-js\` validation function. [You can find it's source code here](https://github.com/graphql/graphql-js/blob/main/src/validation/rules/${ruleName}Rule.ts).`,
83+
description: `${docs.description}\n\n> This rule is a wrapper around a \`graphql-js\` validation function. [You can find it's source code here](https://github.com/graphql/graphql-js/blob/main/src/validation/rules/${ruleName}Rule.ts).`,
8584
},
8685
},
8786
create(context) {
@@ -98,9 +97,8 @@ const validationToRule = (
9897
const schema = requiresSchema ? requireGraphQLSchemaFromContext(name, context) : null;
9998

10099
let documentNode: DocumentNode;
101-
const filePath = context.getFilename();
102-
const isVirtualFile = !fs.existsSync(filePath);
103-
if (!isVirtualFile && getDocumentNode) {
100+
const isRealFile = existsSync(context.getFilename());
101+
if (isRealFile && getDocumentNode) {
104102
documentNode = getDocumentNode(context);
105103
}
106104
validateDoc(node, context, schema, documentNode || node.rawNode(), [ruleFn], ruleName);
@@ -181,10 +179,8 @@ export const GRAPHQL_JS_VALIDATIONS = Object.assign(
181179
if (!isGraphQLImportFile(code)) {
182180
return null;
183181
}
184-
// Import documents if file contains '#import' comments
185-
const fileLoader = new GraphQLFileLoader();
186-
const graphqlAst = fileLoader.handleFileContent(code, context.getFilename(), { noLocation: true });
187-
return graphqlAst.document;
182+
// Import documents because file contains '#import' comments
183+
return processImport(context.getFilename());
188184
}
189185
),
190186
validationToRule('known-type-names', 'KnownTypeNames', {
@@ -203,9 +199,45 @@ export const GRAPHQL_JS_VALIDATIONS = Object.assign(
203199
validationToRule('no-undefined-variables', 'NoUndefinedVariables', {
204200
description: `A GraphQL operation is only valid if all variables encountered, both directly and via fragment spreads, are defined by that operation.`,
205201
}),
206-
validationToRule('no-unused-fragments', 'NoUnusedFragments', {
207-
description: `A GraphQL document is only valid if all fragment definitions are spread within operations, or spread within other fragments spread within operations.`,
208-
}),
202+
validationToRule(
203+
'no-unused-fragments',
204+
'NoUnusedFragments',
205+
{
206+
description: `A GraphQL document is only valid if all fragment definitions are spread within operations, or spread within other fragments spread within operations.`,
207+
requiresSiblings: true,
208+
},
209+
context => {
210+
const siblings = requireSiblingsOperations('no-unused-fragments', context);
211+
const documents = [...siblings.getOperations(), ...siblings.getFragments()]
212+
.filter(({ document }) => isGraphQLImportFile(document.loc.source.body))
213+
.map(({ filePath, document }) => ({
214+
filePath,
215+
code: document.loc.source.body,
216+
}));
217+
218+
const getParentNode = (filePath: string): DocumentNode | null => {
219+
for (const { filePath: docFilePath, code } of documents) {
220+
const isFileImported = code
221+
.split('\n')
222+
.filter(isGraphQLImportFile)
223+
.map(line => parseImportLine(line.replace('#', '')))
224+
.some(o => filePath === join(dirname(docFilePath), o.from));
225+
226+
if (!isFileImported) {
227+
continue;
228+
}
229+
// Import first file that import this file
230+
const document = processImport(docFilePath);
231+
// Import most top file that import this file
232+
return getParentNode(docFilePath) || document;
233+
}
234+
235+
return null;
236+
};
237+
238+
return getParentNode(context.getFilename());
239+
}
240+
),
209241
validationToRule('no-unused-variables', 'NoUnusedVariables', {
210242
description: `A GraphQL operation is only valid if all variables defined by an operation are used, either directly or within a spread fragment.`,
211243
}),

‎packages/plugin/src/rules/unique-fragment-name.ts‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { relative } from 'path';
12
import { GraphQLESLintRule } from '../types';
23
import { normalizePath, requireSiblingsOperations } from '../utils';
34

@@ -52,19 +53,20 @@ const rule: GraphQLESLintRule<[], false> = {
5253
},
5354
},
5455
create(context) {
56+
const filename = context.getFilename();
5557
return {
56-
FragmentDefinition: node => {
58+
FragmentDefinition(node) {
5759
const fragmentName = node.name?.value;
5860
const siblings = requireSiblingsOperations('unique-fragment-name', context);
5961

6062
if (fragmentName) {
6163
const siblingFragments = siblings.getFragment(fragmentName);
64+
const conflictingFragments = siblingFragments.filter(f => {
65+
const isSameName = f.document.name?.value === fragmentName;
66+
const isSamePath = normalizePath(f.filePath) === normalizePath(filename);
6267

63-
const conflictingFragments = siblingFragments.filter(
64-
f =>
65-
f.document.name?.value === fragmentName &&
66-
normalizePath(f.filePath) !== normalizePath(context.getFilename())
67-
);
68+
return isSameName && !isSamePath;
69+
});
6870

6971
if (conflictingFragments.length > 0) {
7072
context.report({
@@ -81,7 +83,7 @@ const rule: GraphQLESLintRule<[], false> = {
8183
messageId: UNIQUE_FRAGMENT_NAME,
8284
data: {
8385
fragmentName,
84-
fragmentsSummary: conflictingFragments.map(f => `\t${f.filePath}`).join('\n'),
86+
fragmentsSummary: conflictingFragments.map(f => `\t${relative(process.cwd(), f.filePath)}`).join('\n'),
8587
},
8688
});
8789
}

‎packages/plugin/src/sibling-operations.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ function loadSiblings(baseDir: string, loadPaths: string[]): Source[] {
5050
return loadDocumentsSync(loadPaths, {
5151
cwd: baseDir,
5252
loaders: operationsLoaders,
53+
skipGraphQLImport: true,
5354
});
5455
}
5556

@@ -69,8 +70,10 @@ export function getSiblingOperations(options: ParserOptions, gqlConfig: GraphQLC
6970
} else {
7071
const projectForFile = gqlConfig.getProjectForFile(options.filePath);
7172

72-
if (projectForFile) {
73-
siblings = projectForFile.getDocumentsSync();
73+
if (projectForFile && projectForFile.documents.length > 0) {
74+
siblings = projectForFile.loadDocumentsSync(projectForFile.documents, {
75+
skipGraphQLImport: true,
76+
});
7477
operationsCache.set(fileDir, siblings);
7578
}
7679
}

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

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,6 @@ import { join } from 'path';
22
import { GraphQLRuleTester } from '../src';
33
import { GRAPHQL_JS_VALIDATIONS } from '../src/rules/graphql-js-validation';
44

5-
const TEST_SCHEMA = /* GraphQL */ `
6-
type User {
7-
id: ID!
8-
firstName: String!
9-
}
10-
11-
type Query {
12-
user: User
13-
}
14-
`;
15-
165
const ruleTester = new GraphQLRuleTester();
176

187
ruleTester.runGraphQLTests('known-fragment-names', GRAPHQL_JS_VALIDATIONS['known-fragment-names'], {
@@ -21,7 +10,7 @@ ruleTester.runGraphQLTests('known-fragment-names', GRAPHQL_JS_VALIDATIONS['known
2110
filename: join(__dirname, 'mocks/user.graphql'),
2211
code: ruleTester.fromMockFile('user.graphql'),
2312
parserOptions: {
24-
schema: TEST_SCHEMA,
13+
schema: join(__dirname, 'mocks/user-schema.graphql'),
2514
},
2615
},
2716
],

‎packages/plugin/tests/mocks/post-fields.graphql‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
#import './user-fields.graphql'
2+
3+
fragment PostFields on Post {
4+
user {
5+
...UserFields
6+
}
7+
}

‎packages/plugin/tests/mocks/post.graphql‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
#import "./post-fields.graphql"
2+
3+
query Post {
4+
post {
5+
...PostFields
6+
}
7+
}

‎packages/plugin/tests/mocks/user-schema.graphql‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
type User {
2+
id: ID!
3+
firstName: String!
4+
}
5+
6+
type Post {
7+
id: ID!
8+
user: User!
9+
}
10+
11+
type Query {
12+
user: User
13+
post: Post
14+
}

0 commit comments

Comments
 (0)