Skip to content

Commit c5342de

Browse files
n1ru4ln1ru4l
authored
fix: Loader.canLoad should only handle file paths and glob negation (#3172)
* add failing test * fix: glob negation * uri loader should use canLoad within load
1 parent dae6dc7 commit c5342de

File tree

9 files changed

+79
-74
lines changed

9 files changed

+79
-74
lines changed

‎.changeset/pretty-laws-rest.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
---
2+
'@graphql-tools/code-file-loader': major
3+
'@graphql-tools/git-loader': major
4+
'@graphql-tools/graphql-file-loader': major
5+
'@graphql-tools/load': patch
6+
---
7+
8+
Loader.canLoad and Loader.canLoadSync can only handle file paths not glob patterns

‎packages/load/src/load-typedefs/load-file.ts‎

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,11 @@ export async function loadFile(pointer: string, options: LoadTypedefsOptions): P
1111

1212
for await (const loader of options.loaders) {
1313
try {
14-
const canLoad = await loader.canLoad(pointer, options);
15-
16-
if (canLoad) {
17-
const loadedValue = await loader.load(pointer, options);
18-
if (!isSome(loadedValue) || loadedValue.length === 0) {
19-
continue;
20-
}
21-
return loadedValue;
14+
const loadedValue = await loader.load(pointer, options);
15+
if (!isSome(loadedValue) || loadedValue.length === 0) {
16+
continue;
2217
}
18+
return loadedValue;
2319
} catch (error) {
2420
if (env['DEBUG']) {
2521
console.error(`Failed to find any GraphQL type definitions in: ${pointer} - ${error.message}`);
@@ -40,16 +36,12 @@ export function loadFileSync(pointer: string, options: LoadTypedefsOptions): May
4036

4137
for (const loader of options.loaders) {
4238
try {
43-
const canLoad = loader.canLoadSync && loader.loadSync && loader.canLoadSync(pointer, options);
44-
45-
if (canLoad) {
46-
// We check for the existence so it is okay to force non null
47-
const loadedValue = loader.loadSync!(pointer, options);
48-
if (!isSome(loadedValue) || loadedValue.length === 0) {
49-
continue;
50-
}
51-
return loadedValue;
39+
// We check for the existence so it is okay to force non null
40+
const loadedValue = loader.loadSync!(pointer, options);
41+
if (!isSome(loadedValue) || loadedValue.length === 0) {
42+
continue;
5243
}
44+
return loadedValue;
5345
} catch (error) {
5446
if (env['DEBUG']) {
5547
console.error(`Failed to find any GraphQL type definitions in: ${pointer} - ${error.message}`);

‎packages/load/tests/loaders/documents/documents-from-glob.spec.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ describe('documentsFromGlob', () => {
119119

120120
test(`Should ignore files that is added to ignore glob (using negative glob)`, async () => {
121121
const glob = join(__dirname, './test-files/', '*.graphql');
122-
const ignoreGlob = `!(${join(__dirname, './test-files/', '*.query.graphql')})`;
122+
const ignoreGlob = `!${join(__dirname, './test-files/', '*.query.graphql')}`;
123123
const result = await load([glob, ignoreGlob], {
124124
loaders: [new GraphQLFileLoader()]
125125
});

‎packages/load/tests/loaders/schema/schema-from-typedefs.spec.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,5 +161,17 @@ describe('schema from typedefs', () => {
161161
assertNonMaybe(schemaWithoutSources.extensions)
162162
expect(schemaWithoutSources.extensions['sources']).not.toBeDefined();
163163
});
164+
165+
it('should be able to exclude documents via negative glob', async () => {
166+
const result = await load([
167+
'./tests/loaders/schema/test-files/schema-dir/user.graphql',
168+
'./tests/loaders/schema/test-files/schema-dir/invalid.graphql',
169+
'!./tests/loaders/schema/test-files/schema-dir/i*.graphql',
170+
], {
171+
loaders: [new GraphQLFileLoader()],
172+
includeSources: true,
173+
});
174+
expect(result.getTypeMap()["User"]).toBeDefined()
175+
})
164176
})
165177
});

‎packages/load/tests/loaders/schema/test-files/schema-dir/invalid.graphql‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
type Query {

‎packages/loaders/code-file/src/index.ts‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -76,10 +76,6 @@ export class CodeFileLoader implements Loader<CodeFileLoaderOptions> {
7676

7777
async canLoad(pointer: string, options: CodeFileLoaderOptions): Promise<boolean> {
7878
options = this.getMergedOptions(options);
79-
if (isGlob(pointer)) {
80-
// FIXME: parse to find and check the file extensions?
81-
return true;
82-
}
8379

8480
if (isValidPath(pointer)) {
8581
if (FILE_EXTENSIONS.find(extension => pointer.endsWith(extension))) {
@@ -98,10 +94,6 @@ export class CodeFileLoader implements Loader<CodeFileLoaderOptions> {
9894

9995
canLoadSync(pointer: string, options: CodeFileLoaderOptions): boolean {
10096
options = this.getMergedOptions(options);
101-
if (isGlob(pointer)) {
102-
// FIXME: parse to find and check the file extensions?
103-
return true;
104-
}
10597

10698
if (isValidPath(pointer)) {
10799
if (FILE_EXTENSIONS.find(extension => pointer.endsWith(extension))) {
@@ -116,13 +108,13 @@ export class CodeFileLoader implements Loader<CodeFileLoaderOptions> {
116108
async resolveGlobs(glob: string, options: CodeFileLoaderOptions) {
117109
options = this.getMergedOptions(options);
118110
const ignores = asArray(options.ignore || []);
119-
return globby([glob, ...ignores.map(v => `!(${v})`).map(v => unixify(v))], createGlobbyOptions(options));
111+
return globby([glob, ...ignores.map(v => `!${v}`).map(v => unixify(v))], createGlobbyOptions(options));
120112
}
121113

122114
resolveGlobsSync(glob: string, options: CodeFileLoaderOptions) {
123115
options = this.getMergedOptions(options);
124116
const ignores = asArray(options.ignore || []);
125-
return globby.sync([glob, ...ignores.map(v => `!(${v})`).map(v => unixify(v))], createGlobbyOptions(options));
117+
return globby.sync([glob, ...ignores.map(v => `!${v}`).map(v => unixify(v))], createGlobbyOptions(options));
126118
}
127119

128120
async load(pointer: string, options: CodeFileLoaderOptions): Promise<Source[] | null> {

‎packages/loaders/git/src/index.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ export class GitLoader implements Loader<GitLoaderOptions> {
7575
if (!refsForPaths.has(ref)) {
7676
refsForPaths.set(ref, []);
7777
}
78-
refsForPaths.get(ref).push(`!(${unixify(path)})`);
78+
refsForPaths.get(ref).push(`!${unixify(path)}`);
7979
}
8080

8181
const resolved: string[] = [];
@@ -101,7 +101,7 @@ export class GitLoader implements Loader<GitLoaderOptions> {
101101
if (!refsForPaths.has(ref)) {
102102
refsForPaths.set(ref, []);
103103
}
104-
refsForPaths.get(ref).push(`!(${unixify(path)})`);
104+
refsForPaths.get(ref).push(`!${unixify(path)}`);
105105
}
106106

107107
const resolved: string[] = [];

‎packages/loaders/graphql-file/src/index.ts‎

Lines changed: 27 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import { readFileSync, promises as fsPromises, existsSync } from 'fs';
66
import { cwd as processCwd } from 'process';
77
import { processImport } from '@graphql-tools/import';
88
import globby from 'globby';
9-
import isGlob from 'is-glob';
109
import unixify from 'unixify';
1110

1211
const { readFile, access } = fsPromises;
@@ -61,11 +60,6 @@ export class GraphQLFileLoader implements Loader<GraphQLFileLoaderOptions> {
6160
}
6261

6362
async canLoad(pointer: string, options: GraphQLFileLoaderOptions): Promise<boolean> {
64-
if (isGlob(pointer)) {
65-
// FIXME: parse to find and check the file extensions?
66-
return true;
67-
}
68-
6963
if (isValidPath(pointer)) {
7064
if (FILE_EXTENSIONS.find(extension => pointer.endsWith(extension))) {
7165
const normalizedFilePath = isAbsolute(pointer) ? pointer : resolve(options.cwd || processCwd(), pointer);
@@ -82,66 +76,56 @@ export class GraphQLFileLoader implements Loader<GraphQLFileLoaderOptions> {
8276
}
8377

8478
canLoadSync(pointer: string, options: GraphQLFileLoaderOptions): boolean {
85-
if (isGlob(pointer)) {
86-
// FIXME: parse to find and check the file extensions?
87-
return true;
88-
}
89-
9079
if (isValidPath(pointer)) {
9180
if (FILE_EXTENSIONS.find(extension => pointer.endsWith(extension))) {
9281
const normalizedFilePath = isAbsolute(pointer) ? pointer : resolve(options.cwd || processCwd(), pointer);
9382
return existsSync(normalizedFilePath);
9483
}
9584
}
96-
9785
return false;
9886
}
9987

10088
async resolveGlobs(glob: string, options: GraphQLFileLoaderOptions) {
10189
const ignores = asArray(options.ignore || []);
102-
return globby([glob, ...ignores.map(v => `!(${v})`).map(v => unixify(v))], createGlobbyOptions(options));
90+
const target = [glob, ...ignores.map(v => `!${v}`).map(v => unixify(v))];
91+
const result = await globby(target, createGlobbyOptions(options));
92+
return result;
10393
}
10494

10595
resolveGlobsSync(glob: string, options: GraphQLFileLoaderOptions) {
10696
const ignores = asArray(options.ignore || []);
107-
return globby.sync([glob, ...ignores.map(v => `!(${v})`).map(v => unixify(v))], createGlobbyOptions(options));
97+
const target = [glob, ...ignores.map(v => `!${v}`).map(v => unixify(v))];
98+
const result = globby.sync(target, createGlobbyOptions(options));
99+
return result;
108100
}
109101

110102
async load(pointer: string, options: GraphQLFileLoaderOptions): Promise<Source[]> {
111-
if (isGlob(pointer)) {
112-
const resolvedPaths = await this.resolveGlobs(pointer, options);
113-
const finalResult: Source[] = [];
114-
await Promise.all(
115-
resolvedPaths.map(async path => {
116-
if (await this.canLoad(path, options)) {
117-
const result = await this.load(path, options);
118-
result?.forEach(result => finalResult.push(result));
119-
}
120-
})
121-
);
122-
return finalResult;
123-
}
124-
const normalizedFilePath = isAbsolute(pointer) ? pointer : resolve(options.cwd || processCwd(), pointer);
125-
const rawSDL: string = await readFile(normalizedFilePath, { encoding: 'utf8' });
126-
127-
return [this.handleFileContent(rawSDL, normalizedFilePath, options)];
103+
const resolvedPaths = await this.resolveGlobs(pointer, options);
104+
const finalResult: Source[] = [];
105+
106+
await Promise.all(
107+
resolvedPaths.map(async path => {
108+
if (await this.canLoad(path, options)) {
109+
const normalizedFilePath = isAbsolute(path) ? path : resolve(options.cwd || processCwd(), path);
110+
const rawSDL: string = await readFile(normalizedFilePath, { encoding: 'utf8' });
111+
finalResult.push(this.handleFileContent(rawSDL, normalizedFilePath, options));
112+
}
113+
})
114+
);
115+
return finalResult;
128116
}
129117

130118
loadSync(pointer: string, options: GraphQLFileLoaderOptions): Source[] {
131-
if (isGlob(pointer)) {
132-
const resolvedPaths = this.resolveGlobsSync(pointer, options);
133-
const finalResult: Source[] = [];
134-
for (const path of resolvedPaths) {
135-
if (this.canLoadSync(path, options)) {
136-
const result = this.loadSync(path, options);
137-
result?.forEach(result => finalResult.push(result));
138-
}
119+
const resolvedPaths = this.resolveGlobsSync(pointer, options);
120+
const finalResult: Source[] = [];
121+
for (const path of resolvedPaths) {
122+
if (this.canLoadSync(path, options)) {
123+
const normalizedFilePath = isAbsolute(path) ? path : resolve(options.cwd || processCwd(), path);
124+
const rawSDL = readFileSync(normalizedFilePath, { encoding: 'utf8' });
125+
finalResult.push(this.handleFileContent(rawSDL, normalizedFilePath, options));
139126
}
140-
return finalResult;
141127
}
142-
const normalizedFilePath = isAbsolute(pointer) ? pointer : resolve(options.cwd || processCwd(), pointer);
143-
const rawSDL = readFileSync(normalizedFilePath, { encoding: 'utf8' });
144-
return [this.handleFileContent(rawSDL, normalizedFilePath, options)];
128+
return finalResult;
145129
}
146130

147131
handleFileContent(rawSDL: string, pointer: string, options: GraphQLFileLoaderOptions) {

‎packages/loaders/url/src/index.ts‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,15 @@ export interface LoadFromUrlOptions extends BaseLoaderOptions, Partial<Introspec
141141
subscriptionsProtocol?: SubscriptionProtocol;
142142
}
143143

144+
const isCompatibleUri = (uri: string): boolean => {
145+
if (isWebUri(uri)) {
146+
return true;
147+
}
148+
// we just replace the url part, the remaining validation is the same
149+
const wsUri = uri.replace('wss://', 'http://').replace('ws://', 'http://');
150+
return !!isWebUri(wsUri);
151+
};
152+
144153
/**
145154
* This loader loads a schema from a URL. The loaded schema is a fully-executable,
146155
* remote schema since it's created using [@graphql-tools/wrap](/docs/remote-schemas).
@@ -163,7 +172,7 @@ export class UrlLoader implements Loader<LoadFromUrlOptions> {
163172
}
164173

165174
canLoadSync(pointer: string, _options: LoadFromUrlOptions): boolean {
166-
return !!isWebUri(pointer);
175+
return isCompatibleUri(pointer);
167176
}
168177

169178
createFormDataFromVariables<TVariables>({
@@ -642,6 +651,9 @@ export class UrlLoader implements Loader<LoadFromUrlOptions> {
642651
}
643652

644653
async load(pointer: string, options: LoadFromUrlOptions): Promise<Source[]> {
654+
if (!(await this.canLoad(pointer, options))) {
655+
return [];
656+
}
645657
let source: Source = {
646658
location: pointer,
647659
};
@@ -680,6 +692,10 @@ export class UrlLoader implements Loader<LoadFromUrlOptions> {
680692
}
681693

682694
loadSync(pointer: string, options: LoadFromUrlOptions): Source[] {
695+
if (!this.canLoad(pointer, options)) {
696+
return [];
697+
}
698+
683699
let source: Source = {
684700
location: pointer,
685701
};

0 commit comments

Comments
 (0)