fix: skip bad statement source on type generation and exclude type file (#738)

Author: Dane Pilcher

packages/amplify-codegen/src/commands/add.js

@@ -133,7 +133,7 @@ async function add(context, apiId = null, region = 'us-east-1') {
   const newProject = {
     projectName: withoutInit ? 'Codegen Project' : apiDetails.name,
     includes: answer.includePattern,
-    excludes: answer.excludePattern,
+    excludes: [...answer.excludePattern, answer.generatedFileName],
     schema,
     amplifyExtension: {
       codeGenTarget: answer.target || '',

packages/amplify-codegen/src/commands/types.js

@@ -62,18 +62,29 @@ async function generateTypes(context, forceDownloadSchema, withoutInit = false,
           cwd: projectPath,
           absolute: true,
         });
-        const queries = queryFilePaths.map(queryFilePath => {
-          const fileContents = fs.readFileSync(queryFilePath, 'utf8');
-          if (
-            queryFilePath.endsWith('.jsx') ||
-            queryFilePath.endsWith('.js') ||
-            queryFilePath.endsWith('.tsx') ||
-            queryFilePath.endsWith('.ts')
-          ) {
-            return extractDocumentFromJavascript(fileContents, '');
-          }
-          return new Source(fileContents, queryFilePath);
-        });
+        const queries = queryFilePaths
+          .map(queryFilePath => {
+            const fileContents = fs.readFileSync(queryFilePath, 'utf8');
+            if (
+              queryFilePath.endsWith('.jsx') ||
+              queryFilePath.endsWith('.js') ||
+              queryFilePath.endsWith('.tsx') ||
+              queryFilePath.endsWith('.ts')
+            ) {
+              return [queryFilePath, extractDocumentFromJavascript(fileContents, '')];
+            }
+            return [queryFilePath, new Source(fileContents, queryFilePath)];
+          })
+          .filter(([queryFilePath, source]) => {
+            if (!source) {
+              context.print.warning(
+                `Unable to extract GraphQL queries from ${queryFilePath}. Skipping source. This source matched the includes target in .grapqhlconfig.yml. Modify the includes or excludes target if this file should not be included.`,
+              );
+              return false;
+            }
+            return true;
+          })
+          .map(([, source]) => source);
         if (queries.length === 0) {
           throw new Error("No queries found to generate types for, you may need to run 'codegen statements' first");
         }

packages/amplify-codegen/src/walkthrough/configure.js

@@ -50,12 +50,14 @@ async function configureProjectWalkThrough(context, amplifyConfig, withoutInit =
   selectedProjectConfig.includes = await askCodeGeneQueryFilePattern(includePattern);
 
   if (!(frontend === 'android' || targetLanguage === 'javascript')) {
-    amplifyExtension.generatedFileName = await askTargetFileName(amplifyExtension.generatedFileName || 'API', targetLanguage);
+    const generatedFileName = await askTargetFileName(amplifyExtension.generatedFileName || 'API', targetLanguage);
+    amplifyExtension.generatedFileName = generatedFileName;
+    selectedProjectConfig.excludes = Array.from(new Set(selectedProjectConfig.excludes || []).add(generatedFileName));
   } else {
     amplifyExtension.generatedFileName = '';
   }
   amplifyExtension.codeGenTarget = targetLanguage;
-  amplifyExtension.docsFilePath = getGraphQLDocPath(frontend, includePatternDefault.graphQLDirectory, selectedProjectConfig.includes)
+  amplifyExtension.docsFilePath = getGraphQLDocPath(frontend, includePatternDefault.graphQLDirectory, selectedProjectConfig.includes);
   amplifyExtension.maxDepth = await askMaxDepth(amplifyExtension.maxDepth);
 
   return selectedProjectConfig;

packages/amplify-codegen/tests/commands/__snapshots__/add.test.js.snap

@@ -11,7 +11,10 @@ Object {
     "generatedFileName": "API.TS",
     "region": "us-east-1",
   },
-  "excludes": "MOCK_EXCLUDE",
+  "excludes": Array [
+    "MOCK_EXCLUDE",
+    "API.TS",
+  ],
   "includes": "MOCK_INCLUDE",
   "projectName": "Codegen Project",
   "schema": "/user/foo/project/schema.json",
@@ -29,7 +32,10 @@ Object {
     "generatedFileName": "API.TS",
     "region": "us-east-1",
   },
-  "excludes": "MOCK_EXCLUDE",
+  "excludes": Array [
+    "MOCK_EXCLUDE",
+    "API.TS",
+  ],
   "includes": "MOCK_INCLUDE",
   "projectName": "Codegen Project",
   "schema": "/user/foo/project/schema.json",
@@ -47,7 +53,10 @@ Object {
     "generatedFileName": "API.TS",
     "region": "us-east-1",
   },
-  "excludes": "MOCK_EXCLUDE",
+  "excludes": Array [
+    "MOCK_EXCLUDE",
+    "API.TS",
+  ],
   "includes": "MOCK_INCLUDE",
   "projectName": "Codegen Project",
   "schema": "/user/foo/project/schema.graphql",
@@ -65,7 +74,10 @@ Object {
     "generatedFileName": "API.TS",
     "region": "us-west-2",
   },
-  "excludes": "MOCK_EXCLUDE",
+  "excludes": Array [
+    "MOCK_EXCLUDE",
+    "API.TS",
+  ],
   "includes": "MOCK_INCLUDE",
   "projectName": "Codegen Project",
   "schema": "/user/foo/project/schema.json",

packages/amplify-codegen/tests/commands/add.test.js

@@ -45,7 +45,7 @@ jest.mock('process', () => ({
 }));
 
 const MOCK_INCLUDE_PATTERN = 'MOCK_INCLUDE';
-const MOCK_EXCLUDE_PATTERN = 'MOCK_EXCLUDE';
+const MOCK_EXCLUDE_PATTERN = ['MOCK_EXCLUDE'];
 const MOCK_SCHEMA_LOCATION = 'INTROSPECTION_SCHEMA.JSON';
 const MOCK_TARGET = 'TYPE_SCRIPT_OR_FLOW_OR_ANY_OTHER_LANGUAGE';
 const MOCK_GENERATED_FILE_NAME = 'API.TS';
@@ -100,7 +100,7 @@ describe('command - add', () => {
     const newProjectConfig = LOAD_CONFIG_METHODS.addProject.mock.calls[0][0];
     expect(newProjectConfig.projectName).toEqual(MOCK_API_NAME);
     expect(newProjectConfig.includes).toEqual(MOCK_INCLUDE_PATTERN);
-    expect(newProjectConfig.excludes).toEqual(MOCK_EXCLUDE_PATTERN);
+    expect(newProjectConfig.excludes).toEqual([...MOCK_EXCLUDE_PATTERN, MOCK_GENERATED_FILE_NAME]);
     expect(newProjectConfig.schema).toEqual(MOCK_SCHEMA_FILE_LOCATION);
     expect(newProjectConfig.amplifyExtension.codeGenTarget).toEqual(MOCK_TARGET);
     expect(newProjectConfig.amplifyExtension.generatedFileName).toEqual(MOCK_GENERATED_FILE_NAME);

packages/amplify-codegen/tests/commands/mock-fs-setup.js

@@ -36,7 +36,7 @@ function setupMocks(mockFs, loadConfig, apiId, frontend, target, generatedFileNa
       {
         schema: schemaFilePath,
         includes: [path.join(docsFilePath[frontend], '*')],
-        excludes: ['./amplify/**'],
+        excludes: ['./amplify/**', './src/graphql/excluded.ts'],
         amplifyExtension: {
           codeGenTarget: target,
           generatedFileName,

packages/amplify-codegen/tests/commands/types-mock-fs.test.js

@@ -34,6 +34,7 @@ const MOCK_APIS = [
 const MOCK_CONTEXT = {
   print: {
     info: jest.fn(),
+    warning: jest.fn(),
   },
   amplify: {
     getProjectMeta: jest.fn(() => ({ api: { id: MOCK_API_ID } })),
@@ -121,4 +122,25 @@ describe('command - types (mock fs)', () => {
     await generateTypes(MOCK_CONTEXT, false);
     expect(fs.existsSync(generatedFileName)).toBeFalsy();
   });
+
+  it('should skip invalid sources', async () => {
+    const { generatedFileName } = setupMocks(mockFs, loadConfig, MOCK_API_ID, 'javascript', 'typescript', 'src/graphql/API.ts', {
+      'src/graphql/excluded.ts': '',
+      'src/graphql/foo.ts': '',
+    });
+
+    await generateStatements(MOCK_CONTEXT, false);
+    await generateTypes(MOCK_CONTEXT, false);
+    expect(MOCK_CONTEXT.print.warning).toHaveBeenCalledWith(
+      expect.stringMatching(
+        'Unable to extract GraphQL queries from .*/src/graphql/foo.ts. Skipping source. This source matched the includes target in .grapqhlconfig.yml. Modify the includes or excludes target if this file should not be included.',
+      ),
+    );
+    expect(MOCK_CONTEXT.print.warning).not.toHaveBeenCalledWith(
+      expect.stringMatching(
+        'Unable to extract GraphQL queries from .*/src/graphql/excluded.ts. Skipping source. This source matched the includes target in .grapqhlconfig.yml. Modify the includes or excludes target if this file should not be included.',
+      ),
+    );
+    expect(fs.existsSync(generatedFileName)).toBeTruthy();
+  });
 });

packages/amplify-codegen/tests/commands/types.test.js

@@ -80,7 +80,10 @@ describe('command - types', () => {
     await generateTypes(MOCK_CONTEXT, forceDownload);
     expect(getFrontEndHandler).toHaveBeenCalledWith(MOCK_CONTEXT);
     expect(loadConfig).toHaveBeenCalledWith(MOCK_CONTEXT, false);
-    expect(sync).toHaveBeenCalledWith([MOCK_INCLUDE_PATH, `!${MOCK_EXCLUDE_PATH}`], { cwd: MOCK_PROJECT_ROOT, absolute: true });
+    expect(sync).toHaveBeenCalledWith([MOCK_INCLUDE_PATH, `!${MOCK_EXCLUDE_PATH}`], {
+      cwd: MOCK_PROJECT_ROOT,
+      absolute: true,
+    });
     expect(generateTypesHelper).toHaveBeenCalledWith({
       queries: [new Source('query 1', 'q1.gql'), new Source('query 2', 'q2.gql')],
       schema: 'schema',

packages/amplify-codegen/tests/walkthrough/configure.test.js

@@ -24,10 +24,14 @@ describe('configure walk-through', () => {
   const mockGraphQLExtension = 'MOCK_GQL_EXTENSION';
   const MOCK_MAX_DEPTH = 'MOCK_MAX_DEPTH';
 
+  const projectOneExcludes = ['one/excluded/*.gql', 'one/excluded/*.graphql'];
+  const projectTwoExcludes = ['two/excluded/*.gql', 'two/excluded/*.graphql'];
+
   const mockConfigs = [
     {
       projectName: 'One',
-      includes: ['one/**/*.gql', 'one/**/*.graohql'],
+      includes: ['one/**/*.gql', 'one/**/*.graphql'],
+      excludes: projectOneExcludes,
       amplifyExtension: {
         graphQLApiId: 'one',
         generatedFileName: 'one-1.ts',
@@ -37,7 +41,8 @@ describe('configure walk-through', () => {
     },
     {
       projectName: 'Two',
-      includes: ['two/**/*.gql', 'two/**/*.graohql'],
+      includes: ['two/**/*.gql', 'two/**/*.graphql'],
+      excludes: projectTwoExcludes,
       amplifyExtension: {
         graphQLApiId: 'two',
         maxDepth: 10,
@@ -78,13 +83,40 @@ describe('configure walk-through', () => {
       mockConfigs[1].amplifyExtension.codeGenTarget,
       false,
       undefined,
-      undefined
+      undefined,
     );
     expect(askCodegneQueryFilePattern).toHaveBeenCalledWith([join(mockGraphQLDirectory, '**', mockGraphQLExtension)]);
     expect(askGeneratedFileName).toHaveBeenCalledWith(mockConfigs[1].amplifyExtension.generatedFileName, mockTargetLanguage);
     expect(askMaxDepth).toHaveBeenCalledWith(10);
     expect(results).toEqual({
       projectName: mockConfigs[1].projectName,
+      excludes: [...projectTwoExcludes, mockGeneratedFileName],
+      includes: mockIncludes,
+      amplifyExtension: {
+        graphQLApiId: mockConfigs[1].amplifyExtension.graphQLApiId,
+        generatedFileName: mockGeneratedFileName,
+        codeGenTarget: mockTargetLanguage,
+        maxDepth: MOCK_MAX_DEPTH,
+      },
+    });
+  });
+
+  it('should not add generated types file to excludes twice', async () => {
+    const result = await configure(mockContext, mockConfigs);
+    expect(result).toEqual({
+      projectName: mockConfigs[1].projectName,
+      excludes: [...projectTwoExcludes, mockGeneratedFileName],
+      includes: mockIncludes,
+      amplifyExtension: {
+        graphQLApiId: mockConfigs[1].amplifyExtension.graphQLApiId,
+        generatedFileName: mockGeneratedFileName,
+        codeGenTarget: mockTargetLanguage,
+        maxDepth: MOCK_MAX_DEPTH,
+      },
+    });
+    expect(await configure(mockContext, [result])).toEqual({
+      projectName: mockConfigs[1].projectName,
+      excludes: [...projectTwoExcludes, mockGeneratedFileName],
       includes: mockIncludes,
       amplifyExtension: {
         graphQLApiId: mockConfigs[1].amplifyExtension.graphQLApiId,