Repository: apollographql/apollo-codegen (ts)

Issue:
flow-modern target fails on some multiline comments
Using version 0.18.0 with `--target flow-modern`, I get this error:

```
TypeError: Cannot read property 'trim' of undefined
    at Printer.fixCommas (./node_modules/apollo-codegen/src/javascript/flow/printer.ts:87:24)
    at printQueue.reduce (./node_modules/apollo-codegen/src/javascript/flow/printer.ts:19:36)
```

I added this debugging,
```
                const [contents, comment] = currentLineContents.split('//');
                if (!comment) {
                  console.log({contents, comment, currentLineContents, currentLine, strippedNextLine, commentColumn, documentPart});
```
And that outputs:
```
{ contents: 'TODO make this available under another key with a Date type',
  comment: undefined,
  currentLineContents: 'TODO make this available under another key with a Date type',
  currentLine: 21,
  strippedNextLine: ',',
  commentColumn: -1,
  documentPart: 'export type AdminPeopleTab_organizations_class_owners = {\n  __typename: "ClassMembership",\n  id: number // The membership ID.\n  ,\n  group_id: number // The group ID for this membership.\n  ,\n  can_become_owner: boolean // True if this user may be promoted to an owner.\n  ,\n  can_unsubscribe: boolean // True if you may unsubscribe this user from the class\n  ,\n  can_chat: boolean // True if you may initiate a chat with this user\n  ,\n  chat_eligibility_state: ChatEligibilityState // The chat eligibility state of the user, e.g. eligible, pending, under_13\n  ,\n  role: ClassMembershipRole // The member\'s role in the class\n  ,\n  tag: ?string // An optional tag to show in the member list.\n  ,\n  query_key: string // The string to match client side filtering against.\n  ,\n  created_at: ?string // When the user joined\nTODO make this available under another key with a Date type\n  ,\n  user: AdminPeopleTab_organizations_class_owners_user,\n  org_affiliation_view: ?AdminPeopleTab_organizations_class_owners_org_affiliation_view,\n  permissions: ?AdminPeopleTab_organizations_class_owners_permissions,\n};' }
TypeError: Cannot read property 'trim' of undefined
```

So it appears to be choking on the multiline description, which in our JSON schema is
```json
              "description": "When the user joined\nTODO make this available under another key with a Date type",
```

I see that the repo has an explicit test for multiLine which is passing. My guess as to why this one isn't is that the `AdminPeopleTab` query uses a fragment. Maybe there's some interaction with the `.gql` loader?

Pull request description:
Fix GraphQL multiline comment support for typescript-modern target
Fixes #306 for Typescript
Mirrors #311

Patch:
diff --git a/src/javascript/flow/language.ts b/src/javascript/flow/language.ts
index 369a6e8a..3daae716 100644
--- a/src/javascript/flow/language.ts
+++ b/src/javascript/flow/language.ts
@@ -74,10 +74,12 @@ export default class FlowGenerator {
       keyInheritsNullability: true
     });
 
-    typeAlias.leadingComments = [{
-      type: 'CommentLine',
-      value: ` ${description}`
-    } as t.CommentLine]
+    if (description) {
+      typeAlias.leadingComments = [{
+        type: 'CommentLine',
+        value: ` ${description.replace('\n', ' ')}`
+      } as t.CommentLine]
+    }
 
     return typeAlias;
   }
@@ -104,7 +106,7 @@ export default class FlowGenerator {
         if (description) {
           objectTypeProperty.trailingComments = [{
             type: 'CommentLine',
-            value: ` ${description}`
+            value: ` ${description.replace('\n', ' ')}`
           } as t.CommentLine]
         }
 
diff --git a/src/javascript/flow/printer.ts b/src/javascript/flow/printer.ts
index 0da25775..d52e2739 100644
--- a/src/javascript/flow/printer.ts
+++ b/src/javascript/flow/printer.ts
@@ -78,7 +78,7 @@ export default class Printer {
         const [contents, comment] = currentLineContents.split('//');
         newDocumentParts.push({
           main: contents.replace(/\s+$/g, '') + ',',
-          comment: comment.trim()
+          comment: comment ? comment.trim() : null
         });
         currentLine++;
       } else {
diff --git a/src/javascript/typescript/language.ts b/src/javascript/typescript/language.ts
index 013ea265..85eafd41 100644
--- a/src/javascript/typescript/language.ts
+++ b/src/javascript/typescript/language.ts
@@ -50,10 +50,12 @@ export default class TypescriptGenerator {
       []
     );
 
-    typeAlias.leadingComments = [{
-      type: 'CommentLine',
-      value: ` ${description}`
-    } as t.CommentLine];
+    if (description) {
+      typeAlias.leadingComments = [{
+        type: 'CommentLine',
+        value: ` ${description.replace('\n', ' ')}`
+      } as t.CommentLine];
+    }
 
     return typeAlias;
   }
@@ -106,7 +108,7 @@ export default class TypescriptGenerator {
         if (description) {
           objectTypeProperty.trailingComments = [{
             type: 'CommentLine',
-            value: ` ${description}`
+            value: ` ${description.replace('\n', ' ')}`
           } as t.CommentLine]
         }
 
diff --git a/src/javascript/typescript/printer.ts b/src/javascript/typescript/printer.ts
index 0da25775..ace10db1 100644
--- a/src/javascript/typescript/printer.ts
+++ b/src/javascript/typescript/printer.ts
@@ -78,7 +78,7 @@ export default class Printer {
         const [contents, comment] = currentLineContents.split('//');
         newDocumentParts.push({
           main: contents.replace(/\s+$/g, '') + ',',
-          comment: comment.trim()
+          comment: comment ? comment.trim(): null
         });
         currentLine++;
       } else {


Test patch:
diff --git a/src/javascript/flow/__tests__/__snapshots__/codeGeneration.ts.snap b/src/javascript/flow/__tests__/__snapshots__/codeGeneration.ts.snap
index bd777257..e085e71a 100644
--- a/src/javascript/flow/__tests__/__snapshots__/codeGeneration.ts.snap
+++ b/src/javascript/flow/__tests__/__snapshots__/codeGeneration.ts.snap
@@ -252,6 +252,41 @@ export type Episode = \\"NEWHOPE\\" | \\"EMPIRE\\" | \\"JEDI\\";
 }
 `;
 
+exports[`Flow codeGeneration handles multiline graphql comments 1`] = `
+Object {
+  "__generated__/CustomScalar.js": FlowGeneratedFile {
+    "fileContents": "
+
+/* @flow */
+// This file was automatically generated and should not be edited.
+
+// ====================================================
+// GraphQL query operation: CustomScalar
+// ====================================================
+
+export type CustomScalar_commentTest = {
+  __typename: \\"CommentTest\\",
+  multiLine: ?string, // This is a multi-line comment.
+};
+
+export type CustomScalar = {
+  commentTest: ?CustomScalar_commentTest
+};
+
+//==============================================================
+// START Enums and Input Objects
+// All enums and input objects are included in every output file
+// for now, but this will be changed soon.
+// TODO: Link to issue to fix this.
+//==============================================================
+
+//==============================================================
+// END Enums and Input Objects
+//==============================================================",
+  },
+}
+`;
+
 exports[`Flow codeGeneration inline fragment 1`] = `
 Object {
   "__generated__/HeroInlineFragment.js": FlowGeneratedFile {
diff --git a/src/javascript/flow/__tests__/codeGeneration.ts b/src/javascript/flow/__tests__/codeGeneration.ts
index 2d5a3069..be85a325 100644
--- a/src/javascript/flow/__tests__/codeGeneration.ts
+++ b/src/javascript/flow/__tests__/codeGeneration.ts
@@ -254,4 +254,25 @@ describe('Flow codeGeneration', () => {
     const output = generateSource(context);
     expect(output).toMatchSnapshot();
   });
+
+  test('handles multiline graphql comments', () => {
+    const miscSchema = loadSchema(require.resolve('../../../../test/fixtures/misc/schema.json'));
+
+    const document = parse(`
+      query CustomScalar {
+        commentTest {
+          multiLine
+        }
+      }
+    `);
+
+    const output = generateSource(
+      compileToIR(miscSchema, document, {
+        mergeInFieldsFromFragmentSpreads: true,
+        addTypename: true
+      })
+    );
+
+    expect(output).toMatchSnapshot();
+  });
 });
diff --git a/src/javascript/typescript/__tests__/__snapshots__/codeGeneration.ts.snap b/src/javascript/typescript/__tests__/__snapshots__/codeGeneration.ts.snap
index 8a3e24a7..5761c0d3 100644
--- a/src/javascript/typescript/__tests__/__snapshots__/codeGeneration.ts.snap
+++ b/src/javascript/typescript/__tests__/__snapshots__/codeGeneration.ts.snap
@@ -272,6 +272,41 @@ export enum Episode {
 }
 `;
 
+exports[`Typescript codeGeneration handles multiline graphql comments 1`] = `
+Object {
+  "__generated__/CustomScalar.js": TypescriptGeneratedFile {
+    "fileContents": "
+
+/* tslint:disable */
+// This file was automatically generated and should not be edited.
+
+// ====================================================
+// GraphQL query operation: CustomScalar
+// ====================================================
+
+export type CustomScalar_commentTest = {
+  __typename: \\"CommentTest\\",
+  multiLine: string | undefined | null, // This is a multi-line comment.
+};
+
+export type CustomScalar = {
+  commentTest: CustomScalar_commentTest | undefined | null
+};
+
+//==============================================================
+// START Enums and Input Objects
+// All enums and input objects are included in every output file
+// for now, but this will be changed soon.
+// TODO: Link to issue to fix this.
+//==============================================================
+
+//==============================================================
+// END Enums and Input Objects
+//==============================================================",
+  },
+}
+`;
+
 exports[`Typescript codeGeneration inline fragment 1`] = `
 Object {
   "__generated__/HeroInlineFragment.js": TypescriptGeneratedFile {
@@ -471,8 +506,8 @@ export enum Episode {
 // The input object sent when someone is creating a new review
 type ReviewInput = {
   stars: number,
-  commentary: string | undefined | null,
-  favorite_color: ColorInput | undefined | null,
+  commentary?: string | undefined | null,
+  favorite_color?: ColorInput | undefined | null,
 };
 
 // The input object sent when passing in a color
@@ -528,8 +563,8 @@ export enum Episode {
 // The input object sent when someone is creating a new review
 type ReviewInput = {
   stars: number,
-  commentary: string | undefined | null,
-  favorite_color: ColorInput | undefined | null,
+  commentary?: string | undefined | null,
+  favorite_color?: ColorInput | undefined | null,
 };
 
 // The input object sent when passing in a color
@@ -585,8 +620,8 @@ export enum Episode {
 // The input object sent when someone is creating a new review
 type ReviewInput = {
   stars: number,
-  commentary: string | undefined | null,
-  favorite_color: ColorInput | undefined | null,
+  commentary?: string | undefined | null,
+  favorite_color?: ColorInput | undefined | null,
 };
 
 // The input object sent when passing in a color
@@ -637,8 +672,8 @@ export enum Episode {
 // The input object sent when someone is creating a new review
 type ReviewInput = {
   stars: number,
-  commentary: string | undefined | null,
-  favorite_color: ColorInput | undefined | null,
+  commentary?: string | undefined | null,
+  favorite_color?: ColorInput | undefined | null,
 };
 
 // The input object sent when passing in a color
diff --git a/src/javascript/typescript/__tests__/codeGeneration.ts b/src/javascript/typescript/__tests__/codeGeneration.ts
index 1879d782..1d853ea9 100644
--- a/src/javascript/typescript/__tests__/codeGeneration.ts
+++ b/src/javascript/typescript/__tests__/codeGeneration.ts
@@ -76,7 +76,7 @@ describe('Typescript codeGeneration', () => {
     expect(output).toMatchSnapshot();
   });
 
-  test.only('simple mutation', () => {
+  test('simple mutation', () => {
     const context = compile(`
       mutation ReviewMovie($episode: Episode, $review: ReviewInput) {
         createReview(episode: $episode, review: $review) {
@@ -254,4 +254,25 @@ describe('Typescript codeGeneration', () => {
     const output = generateSource(context);
     expect(output).toMatchSnapshot();
   });
+
+  test('handles multiline graphql comments', () => {
+    const miscSchema = loadSchema(require.resolve('../../../../test/fixtures/misc/schema.json'));
+
+    const document = parse(`
+      query CustomScalar {
+        commentTest {
+          multiLine
+        }
+      }
+    `);
+
+    const output = generateSource(
+      compileToIR(miscSchema, document, {
+        mergeInFieldsFromFragmentSpreads: true,
+        addTypename: true
+      })
+    );
+
+    expect(output).toMatchSnapshot();
+  });
 });