Skip to content

Commit 3b35bae

Browse files
yoavain-sundayskyyoavaindimaMachina
authored
Fix Issue #2276 (#2277)
* Fix Issue #2276 * Convert `source.filePath` to its absolute path to be comparable to importPath/filePath on Windows * Fix Issue #2276 * Convert `source.filePath` to its absolute path to be comparable to importPath/filePath on Windows * Add require-import-fragment test for relative path on Windows with both delimiter types in input * upd snapshot * try to see if tests fails * more * rm * prettier * more * more * --allowOnly for now * try * try * try * try * more * more * try * try * use always forward for suggested import path * remove unneeded test * fix * Update .changeset/empty-horses-relate.md --------- Co-authored-by: Yoav Vainrich <yoavain@gmail.com> Co-authored-by: Dimitri POSTOLOV <dmytropostolov@gmail.com>
1 parent adc31fe commit 3b35bae

File tree

12 files changed

+51
-37
lines changed

12 files changed

+51
-37
lines changed

.changeset/empty-horses-relate.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@graphql-eslint/eslint-plugin': patch
3+
---
4+
5+
fix false positive cases for `require-import-fragment` on Windows, when `graphql-config`'s `documents` key contained glob pattern => source file path of document contained always forward slashes

packages/plugin/__tests__/mocks/import-fragments/invalid-query-default.gql

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
#import 'bar-fragment.gql'
1+
#import './fragments/bar-fragment.gql'
22
query {
33
foo {
44
...FooFields

packages/plugin/__tests__/mocks/import-fragments/invalid-query.gql

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
#import FooFields from "bar-fragment.gql"
1+
#import FooFields from "./fragments/bar-fragment.gql"
22
query {
33
foo {
44
...FooFields

packages/plugin/__tests__/mocks/import-fragments/valid-query-default.gql

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# Imports could have extra whitespace and double/single quotes
22

3-
# import 'foo-fragment.gql'
3+
# import './fragments/foo-fragment.gql'
44

55
query {
66
foo {

packages/plugin/__tests__/mocks/import-fragments/valid-query.gql

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# Imports could have extra whitespace and double/single quotes
22

3-
# import FooFields from "foo-fragment.gql"
3+
# import FooFields from "./fragments/foo-fragment.gql"
44

55
query {
66
foo {

packages/plugin/src/documents.ts

+8-3
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { resolve } from 'node:path';
1+
import path from 'node:path';
22
import debugFactory from 'debug';
33
import fg from 'fast-glob';
44
import { GraphQLProjectConfig } from 'graphql-config';
@@ -15,13 +15,18 @@ const handleVirtualPath = (documents: Source[]): Source[] => {
1515
return documents.map(source => {
1616
const location = source.location!;
1717
if (['.gql', '.graphql'].some(extension => location.endsWith(extension))) {
18-
return source;
18+
return {
19+
...source,
20+
// When using glob pattern e.g. `**/*.gql` location contains always forward slashes even on
21+
// Windows
22+
location: path.resolve(location),
23+
};
1924
}
2025
filepathMap[location] ??= -1;
2126
const index = (filepathMap[location] += 1);
2227
return {
2328
...source,
24-
location: resolve(location, `${index}_document.graphql`),
29+
location: path.resolve(location, `${index}_document.graphql`),
2530
};
2631
});
2732
};

packages/plugin/src/rules/require-import-fragment/index.test.ts

+20-15
Original file line numberDiff line numberDiff line change
@@ -1,55 +1,60 @@
1-
import { join } from 'node:path';
2-
import { CWD } from '@/utils.js';
1+
import path from 'node:path';
32
import { ParserOptionsForTests, ruleTester } from '../../../__tests__/test-utils.js';
43
import { rule } from './index.js';
54

6-
function withMocks({ name, filename, errors }: { name: string; filename: string; errors?: any }) {
5+
function withMocks({
6+
name,
7+
filename,
8+
errors,
9+
only = false,
10+
}: {
11+
name: string;
12+
filename: string;
13+
errors?: any;
14+
only?: boolean;
15+
}) {
716
return {
817
name,
918
filename,
1019
code: ruleTester.fromMockFile(filename.split('mocks')[1]),
1120
parserOptions: {
1221
graphQLConfig: {
13-
documents: [
14-
filename,
15-
join(CWD, '__tests__', 'mocks', 'import-fragments', 'foo-fragment.gql'),
16-
join(CWD, '__tests__', 'mocks', 'import-fragments', 'bar-fragment.gql'),
17-
],
22+
documents: [filename, './__tests__/mocks/import-fragments/fragments/**/*.gql'],
1823
},
1924
} satisfies ParserOptionsForTests,
2025
errors,
26+
only,
2127
};
2228
}
23-
2429
ruleTester.run('require-import-fragment', rule, {
2530
valid: [
2631
withMocks({
2732
name: 'should not report with named import',
28-
filename: join(CWD, '__tests__', 'mocks', 'import-fragments', 'valid-query.gql'),
33+
filename: path.resolve('__tests__', 'mocks', 'import-fragments', 'valid-query.gql'),
2934
}),
3035
withMocks({
3136
name: 'should not report with default import',
32-
filename: join(CWD, '__tests__', 'mocks', 'import-fragments', 'valid-query-default.gql'),
37+
filename: path.resolve('__tests__', 'mocks', 'import-fragments', 'valid-query-default.gql'),
3338
}),
3439
withMocks({
3540
name: 'should not report fragments from the same file',
36-
filename: join(CWD, '__tests__', 'mocks', 'import-fragments', 'same-file.gql'),
41+
filename: path.resolve('__tests__', 'mocks', 'import-fragments', 'same-file.gql'),
3742
}),
3843
],
3944
invalid: [
4045
withMocks({
4146
name: 'should report with named import',
42-
filename: join(CWD, '__tests__', 'mocks', 'import-fragments', 'invalid-query.gql'),
47+
filename: path.resolve('__tests__', 'mocks', 'import-fragments', 'invalid-query.gql'),
4348
errors: [{ message: 'Expected "FooFields" fragment to be imported.' }],
4449
}),
4550
withMocks({
4651
name: 'should report with default import',
47-
filename: join(CWD, '__tests__', 'mocks', 'import-fragments', 'invalid-query-default.gql'),
52+
filename: path.resolve('__tests__', 'mocks', 'import-fragments', 'invalid-query-default.gql'),
4853
errors: [{ message: 'Expected "FooFields" fragment to be imported.' }],
4954
}),
5055
withMocks({
5156
name: 'should report fragments when there are no import expressions',
52-
filename: join(CWD, '__tests__', 'mocks', 'import-fragments', 'missing-import.gql'),
57+
filename: path.resolve('__tests__', 'mocks', 'import-fragments', 'missing-import.gql'),
5358
errors: [{ message: 'Expected "FooFields" fragment to be imported.' }],
5459
}),
5560
],

packages/plugin/src/rules/require-import-fragment/index.ts

+6-6
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import path from 'node:path';
22
import { NameNode } from 'graphql';
33
import { GraphQLESTreeNode } from '../../estree-converter/index.js';
44
import { GraphQLESLintRule } from '../../types.js';
5-
import { requireSiblingsOperations } from '../../utils.js';
5+
import { requireSiblingsOperations, slash } from '../../utils.js';
66

77
const RULE_ID = 'require-import-fragment';
88
const SUGGESTION_ID = 'add-import-expression';
@@ -89,21 +89,21 @@ export const rule: GraphQLESLintRule = {
8989

9090
const extractedImportPath = comment.value.match(/(["'])((?:\1|.)*?)\1/)?.[2];
9191
if (!extractedImportPath) continue;
92-
93-
const importPath = path.join(path.dirname(filePath), extractedImportPath);
92+
const importPath = path.join(filePath, '..', extractedImportPath);
9493
const hasInSiblings = fragmentsFromSiblings.some(
9594
source => source.filePath === importPath,
9695
);
9796
if (hasInSiblings) return;
9897
}
99-
10098
const fragmentInSameFile = fragmentsFromSiblings.some(
10199
source => source.filePath === filePath,
102100
);
103101
if (fragmentInSameFile) return;
104-
105102
const suggestedFilePaths = fragmentsFromSiblings.length
106-
? fragmentsFromSiblings.map(o => path.relative(path.dirname(filePath), o.filePath))
103+
? fragmentsFromSiblings.map(o =>
104+
// Use always forward slash for suggested import path
105+
slash(path.relative(path.dirname(filePath), o.filePath)),
106+
)
107107
: ['CHANGE_ME.graphql'];
108108

109109
context.report({

packages/plugin/src/rules/require-import-fragment/snapshot.md

+7-7
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ exports[`require-import-fragment > invalid > should report fragments when there
1818

1919
#### 💡 Suggestion: Add import expression for "FooFields".
2020

21-
1 | # import FooFields from 'foo-fragment.gql'
21+
1 | # import FooFields from 'fragments/foo-fragment.gql'
2222
2 | {
2323
3 | foo {
2424
4 | ...FooFields
@@ -29,7 +29,7 @@ exports[`require-import-fragment > invalid > should report fragments when there
2929
exports[`require-import-fragment > invalid > should report with default import 1`] = `
3030
#### ⌨️ Code
3131

32-
1 | #import 'bar-fragment.gql'
32+
1 | #import './fragments/bar-fragment.gql'
3333
2 | query {
3434
3 | foo {
3535
4 | ...FooFields
@@ -45,8 +45,8 @@ exports[`require-import-fragment > invalid > should report with default import 1
4545

4646
#### 💡 Suggestion: Add import expression for "FooFields".
4747

48-
1 | # import FooFields from 'foo-fragment.gql'
49-
2 | #import 'bar-fragment.gql'
48+
1 | # import FooFields from 'fragments/foo-fragment.gql'
49+
2 | #import './fragments/bar-fragment.gql'
5050
3 | query {
5151
4 | foo {
5252
5 | ...FooFields
@@ -57,7 +57,7 @@ exports[`require-import-fragment > invalid > should report with default import 1
5757
exports[`require-import-fragment > invalid > should report with named import 1`] = `
5858
#### ⌨️ Code
5959

60-
1 | #import FooFields from "bar-fragment.gql"
60+
1 | #import FooFields from "./fragments/bar-fragment.gql"
6161
2 | query {
6262
3 | foo {
6363
4 | ...FooFields
@@ -73,8 +73,8 @@ exports[`require-import-fragment > invalid > should report with named import 1`]
7373

7474
#### 💡 Suggestion: Add import expression for "FooFields".
7575

76-
1 | # import FooFields from 'foo-fragment.gql'
77-
2 | #import FooFields from "bar-fragment.gql"
76+
1 | # import FooFields from 'fragments/foo-fragment.gql'
77+
2 | #import FooFields from "./fragments/bar-fragment.gql"
7878
3 | query {
7979
4 | foo {
8080
5 | ...FooFields

packages/plugin/src/utils.ts

+1-2
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,7 @@ export const logger = {
4949
export const slash = (path: string): string => path.replaceAll('\\', '/');
5050

5151
// Match slash or backslash for Windows
52-
// eslint-disable-next-line no-useless-escape
53-
export const VIRTUAL_DOCUMENT_REGEX = /[\/\\]\d+_document.graphql$/;
52+
export const VIRTUAL_DOCUMENT_REGEX = /[/\\]\d+_document.graphql$/;
5453

5554
export const CWD = process.cwd();
5655

0 commit comments

Comments
 (0)