improve optional parameter conversion logic for unions

Author: tylerkrupicka-stripe

src/convert/function-visitor.ts

@@ -134,12 +134,21 @@ export const functionVisitor = <
           }
         }
       }
-      if (param.type === "Identifier") {
-        paramIsOptional =
-          param.optional ||
-          (param.typeAnnotation?.type === "TypeAnnotation" &&
-            param.typeAnnotation?.typeAnnotation.type ===
-              "NullableTypeAnnotation");
+      // This next block is somewhat complicated, but basically inspects the parameters to
+      // determine what combination of optional / null / undefined they should be.
+      // Flow and TypeScript handle optional parameters differently, and in TS we need to make sure
+      // an optional parameter is not followed by a non-optional one since in TS optional can be omitted.
+      if (param.type === "Identifier" && param.typeAnnotation) {
+        const isNullable =
+          param.typeAnnotation.type === "TypeAnnotation" &&
+          param.typeAnnotation.typeAnnotation.type === "NullableTypeAnnotation";
+        const hasVoid =
+          param.typeAnnotation.type === "TypeAnnotation" &&
+          param.typeAnnotation.typeAnnotation.type === "UnionTypeAnnotation" &&
+          param.typeAnnotation.typeAnnotation.types.some(
+            (unionType) => unionType.type === "VoidTypeAnnotation"
+          );
+        paramIsOptional = param.optional || isNullable || hasVoid;
       }
 
       if (!paramIsOptional) {
@@ -177,6 +186,18 @@ export const functionVisitor = <
               t.nullLiteralTypeAnnotation(),
               t.genericTypeAnnotation(t.identifier("undefined")),
             ]);
+          } else if (
+            identifier.typeAnnotation.typeAnnotation.type ===
+            "UnionTypeAnnotation"
+          ) {
+            identifier.typeAnnotation.typeAnnotation.types =
+              identifier.typeAnnotation.typeAnnotation.types.filter(
+                (unionType) => unionType.type !== "VoidTypeAnnotation"
+              );
+            identifier.typeAnnotation.typeAnnotation = t.unionTypeAnnotation([
+              identifier.typeAnnotation.typeAnnotation,
+              t.genericTypeAnnotation(t.identifier("undefined")),
+            ]);
           } else {
             identifier.typeAnnotation.typeAnnotation = t.unionTypeAnnotation([
               identifier.typeAnnotation.typeAnnotation,

src/convert/type-annotations.test.ts

@@ -26,6 +26,42 @@ describe("transform type annotations", () => {
     expect(await transform(src)).toBe(expected);
   });
 
+  it("Avoids marking void parameters if followed by non-optional", async () => {
+    const src = `function f(x: void, y: string){};`;
+    const expected = `function f(x: undefined, y: string){};`;
+    expect(await transform(src)).toBe(expected);
+  });
+
+  it("Marks void as optional if no other parameters", async () => {
+    const src = `function f(x: string | void, y?: string){};`;
+    const expected = `function f(x?: string, y?: string){};`;
+    expect(await transform(src)).toBe(expected);
+  });
+
+  it("Marks multiple void parameters optional with unions", async () => {
+    const src = `function f(x: T | void, y: ?string){};`;
+    const expected = `function f(x?: T, y?: string | null){};`;
+    expect(await transform(src)).toBe(expected);
+  });
+
+  it("Avoids optionals preceding nullables", async () => {
+    const src = `function f(x: ?T, y: string){};`;
+    const expected = `function f(x: T | null | undefined, y: string){};`;
+    expect(await transform(src)).toBe(expected);
+  });
+
+  it("Avoids optionals preceding non-optional unions", async () => {
+    const src = `function f(x: T | void, y: string){};`;
+    const expected = `function f(x: T | undefined, y: string){};`;
+    expect(await transform(src)).toBe(expected);
+  });
+
+  it("Convertsions optional unions preceding optionals", async () => {
+    const src = `function f(x: T | void, y?: string){};`;
+    const expected = `function f(x?: T, y?: string){};`;
+    expect(await transform(src)).toBe(expected);
+  });
+
   it("Converts void types to undefined", async () => {
     const src = dedent`
     const a: string = "";