Merge pull request #131 from benyap/fix/non-idempotent-behaviour-for-wildcard-paths

fix: do not replace existing relative paths

Author: benyap

README.md

@@ -16,10 +16,6 @@ Use this package after `tsc` builds your code to replace any path aliases with
 relative paths - this means that you can develop using path aliases whilst still
 being able to ship working JavaScript code.
 
-_Yes, there are plugins that can handle this when you use bundlers such as
-Webpack or Rollup. But if you don't want to use a bundler, this package is a
-convenient solution._
-
 **Sample `tsconfig.json`:**
 
 ```ts

src/steps/computeAliases.ts

@@ -1,6 +1,7 @@
 import { resolve } from "path";
 
 import type { Alias } from "~/types";
+import { InvalidAliasError } from "~/utils/errors";
 
 /**
  * Compute the alias paths provided by the tsconfig.
@@ -10,11 +11,21 @@ export function computeAliases(
   paths: { [key: string]: string[] }
 ): Alias[] {
   const regex = /\*$/;
+
   const aliases: Alias[] = Object.keys(paths).map((alias) => ({
+    alias,
     prefix: alias.replace(regex, ""),
     aliasPaths: paths[alias].map((path: string) =>
       resolve(basePath, path.replace(regex, ""))
     ),
   }));
+
+  // Ensure that aliases do not start with a relative path
+  // This will lead to unknown behaviour, and why would you use ./ or ../ as an alias anyway?
+  for (const { alias } of aliases) {
+    if (alias.startsWith("./") || alias.startsWith("../"))
+      throw new InvalidAliasError(computeAliases.name, alias);
+  }
+
   return aliases;
 }

src/steps/generateChanges.ts

@@ -120,10 +120,15 @@ export function aliasToRelativePath(
 ): { file: string; original: string; replacement?: string } {
   const { srcPath, outPath } = programPaths;
 
+  // Ignore any relative paths and return the original path
+  // ASSUMPTION: they are either not an alias, or have already been replaced
+  if (path.startsWith("./") || path.startsWith("../"))
+    return { file: filePath, original: path };
+
   for (const alias of aliases) {
     const { prefix, aliasPaths } = alias;
 
-    // Skip the alias if the path does not start with it
+    // Skip the alias if the path does not start with the prefix
     if (!path.startsWith(prefix)) continue;
 
     const pathRelative = path.substring(prefix.length);

src/steps/loadTSConfig.ts

@@ -15,7 +15,7 @@ import { FileNotFoundError } from "~/utils/errors";
  */
 export function loadTSConfig(path: string): TSConfig {
   const configFileName = findConfigFile(process.cwd(), sys.fileExists, path);
-  if (!configFileName) throw new FileNotFoundError("loadTSConfig", path);
+  if (!configFileName) throw new FileNotFoundError(loadTSConfig.name, path);
   const configFile = readConfigFile(configFileName, sys.readFile);
   const options = parseJsonConfigFileContent(configFile.config, sys, ".");
   return options;

src/steps/resolvePaths.ts

@@ -28,10 +28,7 @@ export function resolvePaths(
     );
 
   if (!paths)
-    throw new TSConfigPropertyError(
-      "resolveConfigPaths",
-      "compilerOptions.paths"
-    );
+    throw new TSConfigPropertyError(resolvePaths.name, "compilerOptions.paths");
 
   const configFile = resolve(process.cwd(), options.project);
   const configPath = dirname(configFile);

src/types.ts

@@ -51,6 +51,8 @@ export interface ProgramPaths {
 }
 
 export interface Alias {
+  /** The original path alias. */
+  alias: string;
   /** The alias prefix that has been matched. */
   prefix: string;
   /** The paths that the alias points to. */

src/utils/errors.ts

@@ -20,14 +20,14 @@ export class FileNotFoundError extends FileError {
   }
 }
 
-export class JSONFileParsingError extends FileError {
-  constructor(step: string, path: string, message: string) {
-    super(step, path, `Failed to parse JSON. ${message}`);
-  }
-}
-
 export class TSConfigPropertyError extends StepError {
   constructor(public readonly step: string, public readonly property: string) {
     super(step, `${property} is not set in tsconfig`);
   }
 }
+
+export class InvalidAliasError extends StepError {
+  constructor(public readonly step: string, public readonly alias: string) {
+    super(step, `The alias ${alias} is not permitted`);
+  }
+}

test/steps/computeAliases.test.ts

@@ -1,6 +1,7 @@
 import { resolve } from "path";
 
 import { computeAliases } from "~/steps/computeAliases";
+import { InvalidAliasError } from "~/utils/errors";
 
 describe("steps/computeAliases", () => {
   it("computes aliases correctly from the root path", () => {
@@ -38,4 +39,24 @@ describe("steps/computeAliases", () => {
     expect(aliases[1].aliasPaths).toEqual([`${cwd}/src`, `${cwd}/root`]);
     expect(aliases[2].aliasPaths).toEqual([`${cwd}/src/app`]);
   });
+
+  it("throws an error if a path alias starting with ./ is detected", () => {
+    const attempt = () =>
+      computeAliases(resolve("."), {
+        "./*": ["./lib/*"],
+        "~/*": ["./src/*", "./root/*"],
+        "@app": ["./src/app/*"],
+      });
+    expect(attempt).toThrowError(InvalidAliasError);
+  });
+
+  it("throws an error if a path alias starting with ../ is detected", () => {
+    const attempt = () =>
+      computeAliases(resolve("."), {
+        "../*": ["./lib/*"],
+        "~/*": ["./src/*", "./root/*"],
+        "@app": ["./src/app/*"],
+      });
+    expect(attempt).toThrowError(InvalidAliasError);
+  });
 });

test/steps/generateChanges.test.ts

@@ -168,6 +168,7 @@ describe("steps/generateChanges", () => {
     const root = `${cwd}/test/fixtures/change`;
     const aliases: Alias[] = [
       {
+        alias: "~/*",
         prefix: "~/",
         aliasPaths: [`${root}/src`, `${root}/src/alternateSrc`],
       },
@@ -277,6 +278,22 @@ describe("steps/generateChanges", () => {
       `);
     });
 
+    it("does not replace paths that are already relative", () => {
+      const result = aliasToRelativePath(
+        "../..",
+        "test/fixtures/change/out/nested/imports.js",
+        [{ alias: "*", prefix: "", aliasPaths: [`${root}/src`] }],
+        programPaths
+      );
+
+      expect(result).toMatchInlineSnapshot(`
+        Object {
+          "file": "test/fixtures/change/out/nested/imports.js",
+          "original": "../..",
+        }
+      `);
+    });
+
     it("returns the correct relative path for an aliased path from a nested directory using a secondary alias", () => {
       const result = aliasToRelativePath(
         "~/alternate",
@@ -300,6 +317,7 @@ describe("steps/generateChanges", () => {
     const root = `${cwd}/test/fixtures/change`;
     const aliases: Alias[] = [
       {
+        alias: "~/*",
         prefix: "~/",
         aliasPaths: [`${root}/src`, `${root}/src/alternateSrc`],
       },
@@ -495,6 +513,7 @@ describe("steps/generateChanges", () => {
     const root = `${cwd}/test/fixtures/change`;
     const aliases: Alias[] = [
       {
+        alias: "~/*",
         prefix: "~/",
         aliasPaths: [`${root}/src`, `${root}/src/alternateSrc`],
       },