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
ardatan · Jul 12, 2021 · c5342de · c5342de
1 parent dae6dc7
commit c5342de
Show file tree

Hide file tree

Showing 9 changed files with 79 additions and 74 deletions.
diff --git a/.changeset/pretty-laws-rest.md b/.changeset/pretty-laws-rest.md
@@ -0,0 +1,8 @@
+---
+'@graphql-tools/code-file-loader': major
+'@graphql-tools/git-loader': major
+'@graphql-tools/graphql-file-loader': major
+'@graphql-tools/load': patch
+---
+
+Loader.canLoad and Loader.canLoadSync can only handle file paths not glob patterns
diff --git a/packages/load/src/load-typedefs/load-file.ts b/packages/load/src/load-typedefs/load-file.ts
@@ -11,15 +11,11 @@ export async function loadFile(pointer: string, options: LoadTypedefsOptions): P
 
   for await (const loader of options.loaders) {
     try {
-      const canLoad = await loader.canLoad(pointer, options);
-
-      if (canLoad) {
-        const loadedValue = await loader.load(pointer, options);
-        if (!isSome(loadedValue) || loadedValue.length === 0) {
-          continue;
-        }
-        return loadedValue;
+      const loadedValue = await loader.load(pointer, options);
+      if (!isSome(loadedValue) || loadedValue.length === 0) {
+        continue;
       }
+      return loadedValue;
     } catch (error) {
       if (env['DEBUG']) {
         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
 
   for (const loader of options.loaders) {
     try {
-      const canLoad = loader.canLoadSync && loader.loadSync && loader.canLoadSync(pointer, options);
-
-      if (canLoad) {
-        // We check for the existence so it is okay to force non null
-        const loadedValue = loader.loadSync!(pointer, options);
-        if (!isSome(loadedValue) || loadedValue.length === 0) {
-          continue;
-        }
-        return loadedValue;
+      // We check for the existence so it is okay to force non null
+      const loadedValue = loader.loadSync!(pointer, options);
+      if (!isSome(loadedValue) || loadedValue.length === 0) {
+        continue;
       }
+      return loadedValue;
     } catch (error) {
       if (env['DEBUG']) {
         console.error(`Failed to find any GraphQL type definitions in: ${pointer} - ${error.message}`);

diff --git a/packages/load/tests/loaders/documents/documents-from-glob.spec.ts b/packages/load/tests/loaders/documents/documents-from-glob.spec.ts
@@ -119,7 +119,7 @@ describe('documentsFromGlob', () => {
 
     test(`Should ignore files that is added to ignore glob (using negative glob)`, async () => {
       const glob = join(__dirname, './test-files/', '*.graphql');
-      const ignoreGlob = `!(${join(__dirname, './test-files/', '*.query.graphql')})`;
+      const ignoreGlob = `!${join(__dirname, './test-files/', '*.query.graphql')}`;
       const result = await load([glob, ignoreGlob], {
         loaders: [new GraphQLFileLoader()]
       });

diff --git a/packages/load/tests/loaders/schema/schema-from-typedefs.spec.ts b/packages/load/tests/loaders/schema/schema-from-typedefs.spec.ts
@@ -161,5 +161,17 @@ describe('schema from typedefs', () => {
       assertNonMaybe(schemaWithoutSources.extensions)
       expect(schemaWithoutSources.extensions['sources']).not.toBeDefined();
     });
+
+    it('should be able to exclude documents via negative glob', async () => {
+      const result = await load([
+        './tests/loaders/schema/test-files/schema-dir/user.graphql',
+        './tests/loaders/schema/test-files/schema-dir/invalid.graphql',
+        '!./tests/loaders/schema/test-files/schema-dir/i*.graphql',
+      ], {
+        loaders: [new GraphQLFileLoader()],
+        includeSources: true,
+      });
+      expect(result.getTypeMap()["User"]).toBeDefined()
+    })
   })
 });
diff --git a/packages/load/tests/loaders/schema/test-files/schema-dir/invalid.graphql b/packages/load/tests/loaders/schema/test-files/schema-dir/invalid.graphql
@@ -0,0 +1 @@
+type Query {
diff --git a/packages/loaders/code-file/src/index.ts b/packages/loaders/code-file/src/index.ts
@@ -76,10 +76,6 @@ export class CodeFileLoader implements Loader<CodeFileLoaderOptions> {
 
   async canLoad(pointer: string, options: CodeFileLoaderOptions): Promise<boolean> {
     options = this.getMergedOptions(options);
-    if (isGlob(pointer)) {
-      // FIXME: parse to find and check the file extensions?
-      return true;
-    }
 
     if (isValidPath(pointer)) {
       if (FILE_EXTENSIONS.find(extension => pointer.endsWith(extension))) {
@@ -98,10 +94,6 @@ export class CodeFileLoader implements Loader<CodeFileLoaderOptions> {
 
   canLoadSync(pointer: string, options: CodeFileLoaderOptions): boolean {
     options = this.getMergedOptions(options);
-    if (isGlob(pointer)) {
-      // FIXME: parse to find and check the file extensions?
-      return true;
-    }
 
     if (isValidPath(pointer)) {
       if (FILE_EXTENSIONS.find(extension => pointer.endsWith(extension))) {
@@ -116,13 +108,13 @@ export class CodeFileLoader implements Loader<CodeFileLoaderOptions> {
   async resolveGlobs(glob: string, options: CodeFileLoaderOptions) {
     options = this.getMergedOptions(options);
     const ignores = asArray(options.ignore || []);
-    return globby([glob, ...ignores.map(v => `!(${v})`).map(v => unixify(v))], createGlobbyOptions(options));
+    return globby([glob, ...ignores.map(v => `!${v}`).map(v => unixify(v))], createGlobbyOptions(options));
   }
 
   resolveGlobsSync(glob: string, options: CodeFileLoaderOptions) {
     options = this.getMergedOptions(options);
     const ignores = asArray(options.ignore || []);
-    return globby.sync([glob, ...ignores.map(v => `!(${v})`).map(v => unixify(v))], createGlobbyOptions(options));
+    return globby.sync([glob, ...ignores.map(v => `!${v}`).map(v => unixify(v))], createGlobbyOptions(options));
   }
 
   async load(pointer: string, options: CodeFileLoaderOptions): Promise<Source[] | null> {

diff --git a/packages/loaders/git/src/index.ts b/packages/loaders/git/src/index.ts
@@ -75,7 +75,7 @@ export class GitLoader implements Loader<GitLoaderOptions> {
       if (!refsForPaths.has(ref)) {
         refsForPaths.set(ref, []);
       }
-      refsForPaths.get(ref).push(`!(${unixify(path)})`);
+      refsForPaths.get(ref).push(`!${unixify(path)}`);
     }
 
     const resolved: string[] = [];
@@ -101,7 +101,7 @@ export class GitLoader implements Loader<GitLoaderOptions> {
       if (!refsForPaths.has(ref)) {
         refsForPaths.set(ref, []);
       }
-      refsForPaths.get(ref).push(`!(${unixify(path)})`);
+      refsForPaths.get(ref).push(`!${unixify(path)}`);
     }
 
     const resolved: string[] = [];

diff --git a/packages/loaders/graphql-file/src/index.ts b/packages/loaders/graphql-file/src/index.ts
@@ -6,7 +6,6 @@ import { readFileSync, promises as fsPromises, existsSync } from 'fs';
 import { cwd as processCwd } from 'process';
 import { processImport } from '@graphql-tools/import';
 import globby from 'globby';
-import isGlob from 'is-glob';
 import unixify from 'unixify';
 
 const { readFile, access } = fsPromises;
@@ -61,11 +60,6 @@ export class GraphQLFileLoader implements Loader<GraphQLFileLoaderOptions> {
   }
 
   async canLoad(pointer: string, options: GraphQLFileLoaderOptions): Promise<boolean> {
-    if (isGlob(pointer)) {
-      // FIXME: parse to find and check the file extensions?
-      return true;
-    }
-
     if (isValidPath(pointer)) {
       if (FILE_EXTENSIONS.find(extension => pointer.endsWith(extension))) {
         const normalizedFilePath = isAbsolute(pointer) ? pointer : resolve(options.cwd || processCwd(), pointer);
@@ -82,66 +76,56 @@ export class GraphQLFileLoader implements Loader<GraphQLFileLoaderOptions> {
   }
 
   canLoadSync(pointer: string, options: GraphQLFileLoaderOptions): boolean {
-    if (isGlob(pointer)) {
-      // FIXME: parse to find and check the file extensions?
-      return true;
-    }
-
     if (isValidPath(pointer)) {
       if (FILE_EXTENSIONS.find(extension => pointer.endsWith(extension))) {
         const normalizedFilePath = isAbsolute(pointer) ? pointer : resolve(options.cwd || processCwd(), pointer);
         return existsSync(normalizedFilePath);
       }
     }
-
     return false;
   }
 
   async resolveGlobs(glob: string, options: GraphQLFileLoaderOptions) {
     const ignores = asArray(options.ignore || []);
-    return globby([glob, ...ignores.map(v => `!(${v})`).map(v => unixify(v))], createGlobbyOptions(options));
+    const target = [glob, ...ignores.map(v => `!${v}`).map(v => unixify(v))];
+    const result = await globby(target, createGlobbyOptions(options));
+    return result;
   }
 
   resolveGlobsSync(glob: string, options: GraphQLFileLoaderOptions) {
     const ignores = asArray(options.ignore || []);
-    return globby.sync([glob, ...ignores.map(v => `!(${v})`).map(v => unixify(v))], createGlobbyOptions(options));
+    const target = [glob, ...ignores.map(v => `!${v}`).map(v => unixify(v))];
+    const result = globby.sync(target, createGlobbyOptions(options));
+    return result;
   }
 
   async load(pointer: string, options: GraphQLFileLoaderOptions): Promise<Source[]> {
-    if (isGlob(pointer)) {
-      const resolvedPaths = await this.resolveGlobs(pointer, options);
-      const finalResult: Source[] = [];
-      await Promise.all(
-        resolvedPaths.map(async path => {
-          if (await this.canLoad(path, options)) {
-            const result = await this.load(path, options);
-            result?.forEach(result => finalResult.push(result));
-          }
-        })
-      );
-      return finalResult;
-    }
-    const normalizedFilePath = isAbsolute(pointer) ? pointer : resolve(options.cwd || processCwd(), pointer);
-    const rawSDL: string = await readFile(normalizedFilePath, { encoding: 'utf8' });
-
-    return [this.handleFileContent(rawSDL, normalizedFilePath, options)];
+    const resolvedPaths = await this.resolveGlobs(pointer, options);
+    const finalResult: Source[] = [];
+
+    await Promise.all(
+      resolvedPaths.map(async path => {
+        if (await this.canLoad(path, options)) {
+          const normalizedFilePath = isAbsolute(path) ? path : resolve(options.cwd || processCwd(), path);
+          const rawSDL: string = await readFile(normalizedFilePath, { encoding: 'utf8' });
+          finalResult.push(this.handleFileContent(rawSDL, normalizedFilePath, options));
+        }
+      })
+    );
+    return finalResult;
   }
 
   loadSync(pointer: string, options: GraphQLFileLoaderOptions): Source[] {
-    if (isGlob(pointer)) {
-      const resolvedPaths = this.resolveGlobsSync(pointer, options);
-      const finalResult: Source[] = [];
-      for (const path of resolvedPaths) {
-        if (this.canLoadSync(path, options)) {
-          const result = this.loadSync(path, options);
-          result?.forEach(result => finalResult.push(result));
-        }
+    const resolvedPaths = this.resolveGlobsSync(pointer, options);
+    const finalResult: Source[] = [];
+    for (const path of resolvedPaths) {
+      if (this.canLoadSync(path, options)) {
+        const normalizedFilePath = isAbsolute(path) ? path : resolve(options.cwd || processCwd(), path);
+        const rawSDL = readFileSync(normalizedFilePath, { encoding: 'utf8' });
+        finalResult.push(this.handleFileContent(rawSDL, normalizedFilePath, options));
       }
-      return finalResult;
     }
-    const normalizedFilePath = isAbsolute(pointer) ? pointer : resolve(options.cwd || processCwd(), pointer);
-    const rawSDL = readFileSync(normalizedFilePath, { encoding: 'utf8' });
-    return [this.handleFileContent(rawSDL, normalizedFilePath, options)];
+    return finalResult;
   }
 
   handleFileContent(rawSDL: string, pointer: string, options: GraphQLFileLoaderOptions) {

diff --git a/packages/loaders/url/src/index.ts b/packages/loaders/url/src/index.ts
@@ -141,6 +141,15 @@ export interface LoadFromUrlOptions extends BaseLoaderOptions, Partial<Introspec
   subscriptionsProtocol?: SubscriptionProtocol;
 }
 
+const isCompatibleUri = (uri: string): boolean => {
+  if (isWebUri(uri)) {
+    return true;
+  }
+  // we just replace the url part, the remaining validation is the same
+  const wsUri = uri.replace('wss://', 'http://').replace('ws://', 'http://');
+  return !!isWebUri(wsUri);
+};
+
 /**
  * This loader loads a schema from a URL. The loaded schema is a fully-executable,
  * remote schema since it's created using [@graphql-tools/wrap](/docs/remote-schemas).
@@ -163,7 +172,7 @@ export class UrlLoader implements Loader<LoadFromUrlOptions> {
   }
 
   canLoadSync(pointer: string, _options: LoadFromUrlOptions): boolean {
-    return !!isWebUri(pointer);
+    return isCompatibleUri(pointer);
   }
 
   createFormDataFromVariables<TVariables>({
@@ -642,6 +651,9 @@ export class UrlLoader implements Loader<LoadFromUrlOptions> {
   }
 
   async load(pointer: string, options: LoadFromUrlOptions): Promise<Source[]> {
+    if (!(await this.canLoad(pointer, options))) {
+      return [];
+    }
     let source: Source = {
       location: pointer,
     };
@@ -680,6 +692,10 @@ export class UrlLoader implements Loader<LoadFromUrlOptions> {
   }
 
   loadSync(pointer: string, options: LoadFromUrlOptions): Source[] {
+    if (!this.canLoad(pointer, options)) {
+      return [];
+    }
+
     let source: Source = {
       location: pointer,
     };