Fix some issues with .d.ts generation

.changeset/weak-bees-rule.md

@@ -0,0 +1,23 @@
+---
+"@preconstruct/cli": patch
+---
+
+Fixed generating declaration maps with versions of TypeScript 4.3 and above.
+
+Errors are now also emitted when TypeScript fails to generate declarations because it needs to reference a type that isn't exported. Previously Preconstruct silently generated a broken declaration file when encountering inputs like the one shown below where TypeScript needs to be able to name the type `X` when generating the `d.ts` file for `index.ts` but it isn't exported, now it will emit an error instead. To fix the error, you need to export the type.
+
+```ts
+// @filename: index.ts
+import { getX } from "./x";
+
+export const x = getX();
+
+// @filename: x.ts
+type X = {
+  x?: X;
+};
+
+export const getX = (): X => ({});
+```
+
+Note that Preconstruct still does not run TypeScript's type checking, you should still do that in addition to running Preconstruct, Preconstruct will only emit these specific errors.

package.json

@@ -69,7 +69,7 @@
     "normalize-path": "^3.0.0",
     "outdent": "^0.7.1",
     "prettier": "^2.1.2",
-    "typescript": "^4.0.3"
+    "typescript": "^4.4.2"
   },
   "jest": {
     "reporters": [

packages/cli/src/__tests__/dev.ts

@@ -281,7 +281,7 @@ test("typescript", async () => {
   ).toMatchSnapshot();
 });
 
-test("typescript", async () => {
+test("typescript with typeScriptProxyFileWithImportEqualsRequireAndExportEquals", async () => {
   let tmpPath = await testdir({
     ...typescriptFixture,
     "package.json": JSON.stringify({

packages/cli/src/__tests__/fix.ts

@@ -391,7 +391,7 @@ test("unexpected experimental flag throws, not removes", async () => {
   });
 
   await expect(fix(tmpPath)).rejects.toMatchInlineSnapshot(
-    `[Error: 🎁  pkg-a The experimental flag "thisDoesNotExist" in your config does not exist]`
+    `[Error: 🎁 pkg-a The experimental flag "thisDoesNotExist" in your config does not exist]`
   );
 });
 

packages/cli/src/__tests__/validate.ts

@@ -586,7 +586,7 @@ test("non-existant entrypoint", async () => {
   });
 
   await expect(validate(tmpPath)).rejects.toMatchInlineSnapshot(
-    `[Error: 🎁  pkg-a specifies a entrypoint "index.js" but the file does not exist, please create it or fix the config]`
+    `[Error: 🎁 pkg-a specifies a entrypoint "index.js" but the file does not exist, please create it or fix the config]`
   );
 });
 
@@ -603,6 +603,6 @@ test("negated entrypoint", async () => {
   });
 
   await expect(validate(tmpPath)).rejects.toMatchInlineSnapshot(
-    `[Error: 🎁  pkg-a specifies a entrypoint "index.js" but it is negated in the same config so it should be removed or the config should be fixed]`
+    `[Error: 🎁 pkg-a specifies a entrypoint "index.js" but it is negated in the same config so it should be removed or the config should be fixed]`
   );
 });

packages/cli/src/build/__tests__/basic.ts

@@ -11,6 +11,8 @@ import {
   repoNodeModules,
   js,
 } from "../../../test-utils";
+import { BatchError } from "../../errors";
+import stripAnsi from "strip-ansi";
 
 const f = fixturez(__dirname);
 
@@ -975,3 +977,64 @@ test("UMD build with process.env.NODE_ENV and typeof document", async () => {
           {"version":3,"file":"scope-test.umd.min.js","sources":["../src/index.js"],"sourcesContent":["let x = typeof document;\\n\\nconst thing = () => {\\n  console.log(process.env.NODE_ENV);\\n};\\n\\nexport default thing;"],"names":["console","log"],"mappings":"wOAEc,KACZA,QAAQC"}
         `);
 });
+
+test("typescript declaration emit with unreferencable types emits diagnostic", async () => {
+  let dir = await testdir({
+    "package.json": JSON.stringify({
+      name: "@scope/test",
+      main: "dist/scope-test.cjs.js",
+    }),
+    ".babelrc": JSON.stringify({
+      presets: [require.resolve("@babel/preset-typescript")],
+    }),
+    node_modules: {
+      kind: "symlink",
+      path: repoNodeModules,
+    },
+    "tsconfig.json": JSON.stringify(
+      {
+        compilerOptions: {
+          target: "esnext",
+          module: "esnext",
+          jsx: "react",
+          isolatedModules: true,
+          strict: true,
+          moduleResolution: "node",
+          esModuleInterop: true,
+          noEmit: true,
+        },
+      },
+      null,
+      2
+    ),
+    "src/index.ts": ts`
+                      import { x } from "./x";
+
+                      export const thing = x();
+                    `,
+    "src/x.ts": ts`
+                  class A {
+                    private a?: string;
+                  }
+
+                  export const x = () => new A();
+                `,
+  });
+  const error = await build(dir).catch((x) => x);
+  expect(error).toBeInstanceOf(BatchError);
+  expect(
+    stripAnsi(
+      error.message.replace(
+        /external module "[^"]+" but cannot be named/,
+        'external module "path-to-module-with-a" but cannot be named'
+      )
+    )
+  ).toMatchInlineSnapshot(`
+    "🎁 Generating TypeScript declarations for src/index.ts failed:
+    🎁 src/index.ts:3:14 - error TS4023: Exported variable 'thing' has or is using name 'A' from external module \\"path-to-module-with-a\\" but cannot be named.
+    🎁
+    🎁 3 export const thing = x();
+    🎁                ~~~~~
+    🎁"
+  `);
+});

packages/cli/src/build/__tests__/other.ts

@@ -400,7 +400,7 @@ test("package resolvable but not in deps", async () => {
     await build(tmpPath);
   } catch (err) {
     expect(err.message).toMatchInlineSnapshot(
-      `"🎁  package-resolvable-but-not-in-deps \\"react\\" is imported by \\"src/index.js\\" but the package is not specified in dependencies or peerDependencies"`
+      `"🎁 package-resolvable-but-not-in-deps \\"react\\" is imported by \\"src/index.js\\" but the package is not specified in dependencies or peerDependencies"`
     );
     return;
   }
@@ -468,7 +468,7 @@ test("module imported outside package directory", async () => {
     await build(tmpPath);
   } catch (err) {
     expect(err.message).toMatchInlineSnapshot(
-      `"🎁  @imports-outside-pkg-dir/pkg-a all relative imports in a package should only import modules inside of their package directory but \\"src/index.js\\" is importing \\"../../some-file\\""`
+      `"🎁 @imports-outside-pkg-dir/pkg-a all relative imports in a package should only import modules inside of their package directory but \\"src/index.js\\" is importing \\"../../some-file\\""`
     );
     return;
   }
@@ -585,10 +585,10 @@ test("batches build errors", async () => {
     error = err;
   }
   expect(error).toMatchInlineSnapshot(`
-    [Error: 🎁  @errors/package-one "something-2" is imported by "src/index.js" but the package is not specified in dependencies or peerDependencies
-    🎁  @errors/package-one "something" is imported by "src/index.js" but the package is not specified in dependencies or peerDependencies
-    🎁  @errors/package-two "something-2" is imported by "src/index.js" but the package is not specified in dependencies or peerDependencies
-    🎁  @errors/package-two "something" is imported by "src/index.js" but the package is not specified in dependencies or peerDependencies]
+    [Error: 🎁 @errors/package-one "something-2" is imported by "src/index.js" but the package is not specified in dependencies or peerDependencies
+    🎁 @errors/package-one "something" is imported by "src/index.js" but the package is not specified in dependencies or peerDependencies
+    🎁 @errors/package-two "something-2" is imported by "src/index.js" but the package is not specified in dependencies or peerDependencies
+    🎁 @errors/package-two "something" is imported by "src/index.js" but the package is not specified in dependencies or peerDependencies]
   `);
 });
 
@@ -717,11 +717,11 @@ test("fails for source files containing top-level this", async () => {
     await build(dir);
   } catch (err) {
     expect(err.message).toMatchInlineSnapshot(`
-      "🎁  pkg \\"src/index.js\\" used \`this\` keyword at the top level of an ES module. You can read more about this at https://rollupjs.org/guide/en/#error-this-is-undefined and fix this issue that has happened here:
-      🎁  pkg
-      🎁  pkg 1: export default this;
-      🎁  pkg                   ^
-      🎁  pkg"
+      "🎁 pkg \\"src/index.js\\" used \`this\` keyword at the top level of an ES module. You can read more about this at https://rollupjs.org/guide/en/#error-this-is-undefined and fix this issue that has happened here:
+      🎁 pkg
+      🎁 pkg 1: export default this;
+      🎁 pkg                   ^
+      🎁 pkg"
     `);
     return;
   }

packages/cli/src/errors.ts

@@ -14,7 +14,7 @@ export class BatchError extends Error {
     super(
       errors
         .map((x) => {
-          return format([x.message], "none", x.scope);
+          return format(x.message, "none", x.scope);
         })
         .join("\n")
     );

packages/cli/src/logger.ts

@@ -1,42 +1,39 @@
 import chalk from "chalk";
-import util from "util";
 
 export function format(
-  args: Array<any>,
+  message: string,
   messageType: "error" | "success" | "info" | "none",
   scope?: string
 ) {
   let prefix = {
-    error: chalk.red("error"),
-    success: chalk.green("success"),
-    info: chalk.cyan("info"),
+    error: " " + chalk.red("error"),
+    success: " " + chalk.green("success"),
+    info: " " + chalk.cyan("info"),
     none: "",
   }[messageType];
-  let fullPrefix =
-    "🎁 " + prefix + (scope === undefined ? "" : " " + chalk.cyan(scope));
-  return (
-    fullPrefix +
-    util
-      .format("", ...args)
-      .split("\n")
-      .reduce((str, line) => {
-        const prefixed = `${str}\n${fullPrefix}`;
-        return line ? `${prefixed} ${line}` : prefixed;
-      })
-  );
+  let fullPrefix = "🎁" + prefix + (scope ? " " + chalk.cyan(scope) : "");
+  return message
+    .split("\n")
+    .map((line) => {
+      if (!line.trim()) {
+        return fullPrefix;
+      }
+      return `${fullPrefix} ${line}`;
+    })
+    .join("\n");
 }
 export function error(message: string, scope?: string) {
-  console.error(format([message], "error", scope));
+  console.error(format(message, "error", scope));
 }
 
 export function success(message: string, scope?: string) {
-  console.log(format([message], "success", scope));
+  console.log(format(message, "success", scope));
 }
 
 export function info(message: string, scope?: string) {
-  console.log(format([message], "info", scope));
+  console.log(format(message, "info", scope));
 }
 
 export function log(message: string) {
-  console.log(format([message], "none"));
+  console.log(format(message, "none"));
 }