Fix Generated Error Types in JSDoc (#299)

Change documented error types in JSDoc to reference the actual generated error types by Conjure
palantir · Dec 18, 2024 · 44c799d · 44c799d
1 parent f066e46
commit 44c799d
Show file tree

Hide file tree

Showing 6 changed files with 129 additions and 9 deletions.
diff --git a/changelog/@unreleased/pr-299.v2.yml b/changelog/@unreleased/pr-299.v2.yml
@@ -0,0 +1,5 @@
+type: improvement
+improvement:
+  description: Change documented error types in JSDoc to reference the actual generated error types by Conjure
+  links:
+  - https://github.com/palantir/conjure-typescript/pull/299
diff --git a/scripts/run-integration-tests.sh b/scripts/run-integration-tests.sh
@@ -5,4 +5,8 @@ set -euo pipefail
 ./build/downloads/bin/conjure-verification-server ./build/resources/verification-server-test-cases.json ./build/resources/verification-server-api.conjure.json &
 SERVER_PID=$!
 yarn karma start --single-run --browsers ChromeHeadless karma.conf.js
-kill -kill ${SERVER_PID}
+
+if ps -p ${SERVER_PID} > /dev/null
+then
+    kill -kill ${SERVER_PID}
+fi
diff --git a/src/commands/generate/__tests__/serviceGeneratorTest.ts b/src/commands/generate/__tests__/serviceGeneratorTest.ts
@@ -588,7 +588,22 @@ export interface IMyService {
                 ],
                 serviceName: { name: "MyService", package: "com.palantir.services" },
             },
-            new Map(),
+            new Map([
+                [
+                    createHashableTypeName({ name: "MyError1", package: "com.palantir.services" }),
+                    {
+                        type: "object",
+                        object: { typeName: { name: "MyError1", package: "com.palantir.services" }, fields: [] },
+                    },
+                ],
+                [
+                    createHashableTypeName({ name: "MyError2", package: "com.palantir.services" }),
+                    {
+                        type: "object",
+                        object: { typeName: { name: "MyError2", package: "com.palantir.services" }, fields: [] },
+                    },
+                ],
+            ]),
             simpleAst,
             DEFAULT_TYPE_GENERATION_FLAGS,
         );
@@ -599,8 +614,8 @@ export interface IMyService {
 export interface IMyService {
     /**
      * endpoint level docs
-     * @throws {MyError1} MyError1 documentation
-     * @throws {MyError2} MyError2 documentation
+     * @throws {IMyError1} MyError1 documentation
+     * @throws {IMyError2} MyError2 documentation
      */
     foo(): Promise<void>;
 }
@@ -631,7 +646,15 @@ export interface IMyService {
                 ],
                 serviceName: { name: "MyService", package: "com.palantir.services" },
             },
-            new Map(),
+            new Map([
+                [
+                    createHashableTypeName({ name: "MyError", package: "com.palantir.services" }),
+                    {
+                        type: "object",
+                        object: { typeName: { name: "MyError", package: "com.palantir.services" }, fields: [] },
+                    },
+                ],
+            ]),
             simpleAst,
             DEFAULT_TYPE_GENERATION_FLAGS,
         );
@@ -643,7 +666,7 @@ export interface IMyService {
     /**
      * endpoint level docs
      * @incubating
-     * @throws {MyError} MyError documentation
+     * @throws {IMyError} MyError documentation
      */
     foo(): Promise<void>;
 }
@@ -734,4 +757,60 @@ export interface IMyService {
         );
         assertOutputAndExpectedAreEqual(outDir, expectedDir, "services/optionalService.ts");
     });
+
+    it("emits service with no duplicate error imports", async () => {
+        await generateService(
+            {
+                endpoints: [
+                    {
+                        args: [],
+                        endpointName: "foo",
+                        httpMethod: HttpMethod.GET,
+                        httpPath: "/foo",
+                        markers: [],
+                        tags: [],
+                        errors: [
+                            {
+                                error: { name: "MyError", namespace: "MyNamespace", package: "com.palantir.errors" },
+                            },
+                        ],
+                    },
+                    {
+                        args: [],
+                        endpointName: "bar",
+                        httpMethod: HttpMethod.GET,
+                        httpPath: "/bar",
+                        markers: [],
+                        tags: [],
+                        errors: [
+                            {
+                                error: { name: "MyError", namespace: "MyNamespace", package: "com.palantir.errors" },
+                            },
+                        ],
+                    },
+                ],
+                serviceName: { name: "MyService", package: "com.palantir.services" },
+            },
+            new Map([
+                [
+                    createHashableTypeName({ name: "MyError", package: "com.palantir.errors" }),
+                    {
+                        type: "object",
+                        object: { typeName: { name: "MyError", package: "com.palantir.errors" }, fields: [] },
+                    },
+                ],
+            ]),
+            simpleAst,
+            DEFAULT_TYPE_GENERATION_FLAGS,
+        );
+        const outFile = path.join(outDir, "services/myService.ts");
+        const contents = fs.readFileSync(outFile, "utf8");
+
+        expect(contents).toContain(`import type { IMyError } from "../errors/myError";
+import { IHttpApiBridge } from "conjure-client";
+
+/** Constant reference to \`undefined\` that we expect to get minified and therefore reduce total code size */
+const __undefined: undefined = undefined;
+`);
+    });
 });
diff --git a/src/commands/generate/__tests__/simpleAstTest.ts b/src/commands/generate/__tests__/simpleAstTest.ts
@@ -23,6 +23,7 @@ import { generateError } from "../errorGenerator";
 import { generateService } from "../serviceGenerator";
 import { SimpleAst } from "../simpleAst";
 import { generateEnum } from "../typeGenerator";
+import { createHashableTypeName } from "../utils";
 import { DEFAULT_TYPE_GENERATION_FLAGS } from "./resources/constants";
 
 describe("simpleAst", () => {
@@ -62,15 +63,31 @@ describe("simpleAst", () => {
                             type: "primitive",
                         },
                         tags: [],
-                        errors: [],
+                        errors: [
+                            {
+                                error: {
+                                    name: "MyError",
+                                    package: "com.palantir.package1",
+                                    namespace: "Metadata",
+                                },
+                            },
+                        ],
                     },
                 ],
                 serviceName: {
                     name: "MyService",
                     package: "com.palantir.package2",
                 },
             },
-            new Map(),
+            new Map([
+                [
+                    createHashableTypeName({ name: "MyError", package: "com.palantir.package1" }),
+                    {
+                        type: "object",
+                        object: { typeName: { name: "MyError", package: "com.palantir.package1" }, fields: [] },
+                    },
+                ],
+            ]),
             simpleAst,
             DEFAULT_TYPE_GENERATION_FLAGS,
         );

diff --git a/src/commands/generate/serviceGenerator.ts b/src/commands/generate/serviceGenerator.ts
@@ -137,6 +137,21 @@ export function generateService(
             scope: Scope.Public,
             docs: docs != null ? [docs] : undefined,
         });
+
+        endpointDefinition.errors?.forEach(error => {
+            // TODO: Get rid of the visitor, because the type (reference) is already known
+            const errorImports: ImportDeclarationStructure[] = IType.visit(
+                {
+                    reference: {
+                        name: `${error.error.name}`,
+                        package: error.error.package,
+                    },
+                    type: "reference",
+                },
+                importsVisitor,
+            ).map(i => ({ ...i, isTypeOnly: true }));
+            imports.push(...errorImports);
+        });
     });
 
     if (imports.length !== 0) {

diff --git a/src/commands/generate/utils.ts b/src/commands/generate/utils.ts
@@ -160,7 +160,7 @@ export function isFlavorizable(type: IType, flavorizedAliases: boolean): boolean
 }
 
 function formattedEndpointError(errorDefinition: IEndpointError): string {
-    let formattedString = `{${errorDefinition.error.name}}`;
+    let formattedString = `{I${errorDefinition.error.name}}`;
     if (errorDefinition.docs != null && errorDefinition.docs != null) {
         formattedString += ` ${errorDefinition.docs}`;
     }