fix: add error boundaries around typescript path alias resolution (#70)

* fix: add error boundaries around typescript path alias resolution * docs: add changeset
antoine-coulon · Jul 20, 2023 · 812d2a5 · 812d2a5
1 parent 46b09ba
commit 812d2a5
Show file tree

Hide file tree

Showing 7 changed files with 102 additions and 36 deletions.
diff --git a/.changeset/purple-pianos-play.md b/.changeset/purple-pianos-play.md
@@ -0,0 +1,5 @@
+---
+"skott": patch
+---
+
+add error boundaries around opaque TypeScript path aliases resolution issues
diff --git a/packages/skott/src/modules/resolvers/ecmascript/resolver.ts b/packages/skott/src/modules/resolvers/ecmascript/resolver.ts
@@ -16,6 +16,8 @@ import {
   kExpectedModuleExtensions
 } from "../base-resolver.js";
 
+import * as Option from "@effect/data/Option";
+
 const NODE_PROTOCOL = "node:";
 
 export function isBuiltinModule(module: string): boolean {
@@ -195,16 +197,17 @@ export class EcmaScriptDependencyResolver implements DependencyResolver {
       const resolvedModulePath = resolvePathAlias(
         moduleDeclaration,
         path.dirname(config.tsConfigPath),
-        workspaceConfiguration.pathAliases
+        workspaceConfiguration.pathAliases,
+        logger
       );
 
-      if (resolvedModulePath) {
+      if (Option.isSome(resolvedModulePath)) {
         logger.success(
           highlightResolved(moduleDeclaration, "TypeScript path alias")
         );
 
         await followModuleDeclaration({
-          moduleDeclaration: resolvedModulePath,
+          moduleDeclaration: resolvedModulePath.value,
           rootPath: resolvedNodePath,
           isPathAliasDeclaration: true
         });

diff --git a/packages/skott/src/modules/walkers/ecmascript/typescript/path-alias.ts b/packages/skott/src/modules/walkers/ecmascript/typescript/path-alias.ts
@@ -4,6 +4,9 @@ import path from "node:path";
 import JSON5 from "json5";
 import type { CompilerOptions } from "typescript";
 
+import * as Option from "@effect/data/Option";
+import { pipe } from "@effect/data/Function";
+
 import type { FileReader } from "../../../../filesystem/file-reader.js";
 import { Logger } from "../../../../logger.js";
 
@@ -19,22 +22,41 @@ interface SupportedTSConfig {
 function resolveAliasToRelativePath(
   moduleDeclaration: string,
   baseAlias: string,
-  baseAliasDirname: string
-): string {
+  baseAliasDirname: string,
+  logger: Logger
+): Option.Option<string> {
   /**
    * When having a path alias like "foo/*": ["core/lib/*"], we want to map
    * any segment such as "foo/lib/baz" to "core/lib/baz".
    */
-  const modulePathWithoutAliasBaseDirname = moduleDeclaration.split(
-    path.join(baseAliasDirname, path.sep)
-  )[1];
+  try {
+    logger.info(
+      `Extracting "${moduleDeclaration}" base path using path alias base directory "${baseAliasDirname}"`
+    );
 
-  const realPathWithAlias = path.join(
-    baseAlias,
-    modulePathWithoutAliasBaseDirname
-  );
+    const modulePathWithoutAliasBaseDirname = moduleDeclaration.split(
+      path.join(baseAliasDirname, path.sep)
+    )[1];
+
+    logger.info(
+      `Attempt to map alias path to real file-system path.` +
+        ` Join "${baseAlias}" -> "${modulePathWithoutAliasBaseDirname}"`
+    );
 
-  return realPathWithAlias;
+    const realPathWithAlias = path.join(
+      baseAlias,
+      modulePathWithoutAliasBaseDirname
+    );
+
+    return Option.some(realPathWithAlias);
+  } catch {
+    logger.failure(
+      `Could not resolve ${moduleDeclaration} alias path to real file-system path` +
+        ` using ${baseAlias} and ${baseAliasDirname}.`
+    );
+
+    return Option.none();
+  }
 }
 
 function isNotBasePathSegment(segment: string): boolean {
@@ -104,17 +126,24 @@ export async function buildPathAliases(
       logger.info(`Extracting path aliases from tsconfig: ${tsConfigPath}`);
 
       for (const [alias, aliasedPath] of Object.entries(paths)) {
+        logger.info(`Found path alias "${alias}": ["${aliasedPath}"]`);
         /**
          * When the path alias is like "foo/*": ["foo/lib/*"], we must be sure
          * to only use known segments of the glob so that we can map easily
          * once glob segment "foo/baz/bar" to "foo/lib/baz/bar".
          */
-        const aliasWithoutGlob = alias.split("/*")[0];
-        const realPathWithoutGlob = aliasedPath[0].split("/*")[0];
+        const aliasWithoutWildcard = alias.split("/*")[0];
+        const realPathWithoutWildcard = aliasedPath[0].split("/*")[0];
 
-        aliasLinks.set(
-          aliasWithoutGlob,
-          path.join(baseUrl, realPathWithoutGlob)
+        const realPathAliasLocation = path.join(
+          baseUrl,
+          realPathWithoutWildcard
+        );
+
+        aliasLinks.set(aliasWithoutWildcard, realPathAliasLocation);
+
+        logger.info(
+          `Registering path alias without wildcards "${aliasWithoutWildcard}": ["${realPathAliasLocation}"]`
         );
       }
     }
@@ -152,21 +181,39 @@ export async function buildPathAliases(
 export function resolvePathAlias(
   moduleDeclaration: string,
   baseDir: string,
-  aliasLinks: Map<string, string>
-): string | undefined {
+  aliasLinks: Map<string, string>,
+  logger: Logger
+): Option.Option<string> {
   const aliasWithoutGlob = aliasLinks.get(moduleDeclaration);
 
   if (aliasWithoutGlob) {
-    return path.join(baseDir, aliasWithoutGlob);
+    return Option.some(path.join(baseDir, aliasWithoutGlob));
   }
 
   let baseAliasDirname = path.dirname(moduleDeclaration);
   let baseAlias = aliasLinks.get(baseAliasDirname);
 
   if (baseAlias) {
-    return path.join(
-      baseDir,
-      resolveAliasToRelativePath(moduleDeclaration, baseAlias, baseAliasDirname)
+    const aliasPath = resolveAliasToRelativePath(
+      moduleDeclaration,
+      baseAlias,
+      baseAliasDirname,
+      logger
+    );
+
+    if (Option.isNone(aliasPath)) {
+      logger.failure(
+        `${moduleDeclaration} path alias could not be resolved using base path ${baseAlias}`
+      );
+    }
+
+    return pipe(
+      aliasPath,
+      Option.map((resolvedPath) => path.join(baseDir, resolvedPath))
+    );
+  } else {
+    logger.failure(
+      `No match found for ${moduleDeclaration} using base ${baseAlias} path alias in the registered entries`
     );
   }
 
@@ -182,21 +229,28 @@ export function resolvePathAlias(
     const deepBaseAlias = aliasLinks.get(baseAliasDirname);
 
     if (deepBaseAlias) {
-      baseAlias = resolveAliasToRelativePath(
-        moduleDeclaration,
-        deepBaseAlias,
-        baseAliasDirname
+      baseAlias = Option.getOrUndefined(
+        resolveAliasToRelativePath(
+          moduleDeclaration,
+          deepBaseAlias,
+          baseAliasDirname,
+          logger
+        )
       );
       pathDepthAttempts = 0;
       break;
     }
   }
 
   if (!baseAlias) {
-    return undefined;
+    logger.failure(
+      `No match found for ${moduleDeclaration} and corresponding segments of ${baseAlias}` +
+        ` path alias in the registered entries. Path depth attempts: ${pathDepthAttempts}`
+    );
+    return Option.none();
   }
 
-  return path.join(baseDir, baseAlias);
+  return Option.some(path.join(baseDir, baseAlias));
 }
 
 export function isTypeScriptRelativePathWithNoLeadingIdentifier(

diff --git a/packages/skott/test/unit/ecmascript/graph.spec.ts b/packages/skott/test/unit/ecmascript/graph.spec.ts
@@ -10,7 +10,7 @@ import { Skott, SkottStructure } from "../../../src/skott.js";
 import {
   buildSkottProjectUsingInMemoryFileExplorer,
   mountFakeFileSystem
-} from "../shared";
+} from "../shared.js";
 import { CollectLevel } from "../../../src/graph/traversal.js";
 import { SkottNode } from "../../../src/graph/node.js";
 

diff --git a/packages/skott/test/unit/ecmascript/javascript.spec.ts b/packages/skott/test/unit/ecmascript/javascript.spec.ts
@@ -6,9 +6,9 @@ import {
   buildSkottProjectUsingInMemoryFileExplorer,
   fakeNodeBody,
   mountFakeFileSystem
-} from "../shared";
+} from "../shared.js";
 
-import { makeTestSuiteForJsxOrTsx as makeTestSuiteForJsx } from "./jsx-and-tsx";
+import { makeTestSuiteForJsxOrTsx as makeTestSuiteForJsx } from "./jsx-and-tsx.js";
 
 describe("When traversing a JavaScript/Node.js project", () => {
   describe("When the project uses ECMAScript modules", () => {

diff --git a/packages/skott/test/unit/ecmascript/jsx-and-tsx.ts b/packages/skott/test/unit/ecmascript/jsx-and-tsx.ts
@@ -4,7 +4,7 @@ import {
   buildSkottProjectUsingInMemoryFileExplorer,
   fakeNodeBody,
   mountFakeFileSystem
-} from "../shared";
+} from "../shared.js";
 
 export function makeTestSuiteForJsxOrTsx(rawLanguage: "ts" | "js"): void {
   const suiteLanguageLabel = rawLanguage === "ts" ? "TSX" : "JSX";

diff --git a/packages/skott/test/unit/ecmascript/typescript.spec.ts b/packages/skott/test/unit/ecmascript/typescript.spec.ts
@@ -4,9 +4,9 @@ import {
   buildSkottProjectUsingInMemoryFileExplorer,
   fakeNodeBody,
   mountFakeFileSystem
-} from "../shared";
+} from "../shared.js";
 
-import { makeTestSuiteForJsxOrTsx as makeTestSuiteForTsx } from "./jsx-and-tsx";
+import { makeTestSuiteForJsxOrTsx as makeTestSuiteForTsx } from "./jsx-and-tsx.js";
 
 describe("When traversing a TypeScript project", () => {
   describe("When the file does not have any module declarations", () => {
@@ -428,10 +428,14 @@ describe("When traversing a TypeScript project", () => {
               "index.ts": `
                 import { script } from "@typescript-eslint/estree-parser";
                 import { foo } from "@lib";
+                import "@lib/should-not-be-resolved";
               `,
               "lib/index.ts": `
                 export function foo(): string {}
               `,
+              "lib/should-not-be-resolved.ts": `
+                export function foo(): string {}
+              `,
               "tsconfig.json": JSON.stringify(tsConfig)
             });