Repository: benyap/resolve-tspaths (ts)

Issue:
Word "import" in comments confuses resolve-tspaths
This is similar to #311.

## Steps to Reproduce
To reproduce, build [this commit](https://github.com/gristlabs/grist-electron/commit/667392804eeb755984a34e902106bfdcac4e8a4e) of gristlabs/grist-electron. Make sure to use Node.js version 18.

```
git clone https://github.com/gristlabs/grist-electron
cd grist-electron
git checkout 6673928
git submodule init
git submodule update
yarn install
yarn run setup
yarn build
yarn run electron:preview
```

The last command should give the following error:
```
Error: Cannot find module 'app/common/gutil'
```

This is because resolve-tspaths failed to expand the import properly.

## More Info
To take a closer look, go to `core/_build/ext/app/electron/main.js`. This file is compiled from `ext/app/electron/main.ts`. On line 38, there is a word "import" in the comments. All imports after that line will not be correctly expanded by resolve-tspaths. The problem goes away if the word "import" is removed.

The minimal commands to see the problem are `tsc --build tsconfig-ext.json`, then `resolve-tspaths` in the `core` directory. You need to make `core/ext` point to `ext` in the grist-electron root before doing this.

Pull request:
fix: import/export regex matching on the word "import" in comments
Fixes #336 and #311

Patch:
diff --git a/src/steps/generateChanges.ts b/src/steps/generateChanges.ts
index c5aa5af..4948ea1 100644
--- a/src/steps/generateChanges.ts
+++ b/src/steps/generateChanges.ts
@@ -6,14 +6,20 @@ import { normalizePath } from "~/utils/path";
 
 import type { Alias, Change, ProgramPaths, TextChange } from "~/types";
 
-export const IMPORT_EXPORT_REGEX =
-  /((?:require\(|require\.resolve\(|import\()|(?:import|export)\s+(?:[\s\S]*?from\s+)?)['"]([^'"]*)['"]\)?/g;
-
-export const ESM_IMPORT_EXPORT_REGEX =
-  /(?:(?:import\()|(?:import|export)\s+(?:[\s\S]*?from\s+)?)['"]([^'"]*)['"]\)?/g;
-
-export const COMMONJS_IMPORT_EXPORT_REGEX =
-  /(?:(?:require\(|require\.resolve\()\s+)['"]([^'"]*)['"]\)/g;
+export const MATCHERS = [
+  // require("package"), require.resolve("package"), import("package")
+  /((?:require|require\.resolve|import)\((?:"([^"]+)")\))/g,
+  /((?:require|require\.resolve|import)\((?:'([^']+)')\))/g,
+  // import "package"
+  /((?:import|export) (?:"([^"]+)"))/g,
+  /((?:import|export) (?:'([^']+)'))/g,
+  // import {} from "package", import * as name from "package"
+  /((?:import|export) .+ from (?:"([^"]+)"))/g,
+  /((?:import|export) .+ from (?:'([^']+)'))/g,
+  // multiline import/exports with {}
+  /((?:import|export) {\n[^}]+} from (?:"([^']+)"))/g,
+  /((?:import|export) {\n[^}]+} from (?:'([^']+)'))/g,
+];
 
 const MODULE_EXTS = [
   ".js",
@@ -74,37 +80,41 @@ export function replaceAliasPathsInFile(
   const originalText = readFileSync(filePath, "utf-8");
   const changes: TextChange[] = [];
 
-  const newText = originalText.replace(
-    IMPORT_EXPORT_REGEX,
-    (original, importStatement: string, importSpecifier: string) => {
-      // The import is an esm import if it is inside a typescript (definition) file or if it uses `import` or `export`
-      const esmImport =
-        !filePath.endsWith(".ts") &&
-        (importStatement.includes("import") ||
-          importStatement.includes("export"));
-
-      const result = aliasToRelativePath(
-        importSpecifier,
-        filePath,
-        aliases,
-        programPaths,
-        esmImport,
-      );
-
-      if (!result.replacement) return original;
-
-      const index = original.lastIndexOf(importSpecifier);
-      changes.push({
-        original: normalizePath(result.original),
-        modified: normalizePath(result.replacement),
-      });
-
-      return (
-        original.substring(0, index) +
-        result.replacement +
-        original.substring(index + importSpecifier.length)
-      );
-    },
+  const newText = MATCHERS.reduce(
+    (text, regex) =>
+      text.replace(
+        regex,
+        (original, importStatement: string, importSpecifier: string) => {
+          // The import is an esm import if it is inside a typescript (definition) file or if it uses `import` or `export`
+          const esmImport =
+            !filePath.endsWith(".ts") &&
+            (importStatement.includes("import") ||
+              importStatement.includes("export"));
+
+          const result = aliasToRelativePath(
+            importSpecifier,
+            filePath,
+            aliases,
+            programPaths,
+            esmImport,
+          );
+
+          if (!result.replacement) return original;
+
+          const index = original.lastIndexOf(importSpecifier);
+          changes.push({
+            original: normalizePath(result.original),
+            modified: normalizePath(result.replacement),
+          });
+
+          return (
+            original.substring(0, index) +
+            result.replacement +
+            original.substring(index + importSpecifier.length)
+          );
+        },
+      ),
+    originalText,
   );
 
   return { changed: originalText !== newText, text: newText, changes };


Test patch:
diff --git a/test/steps/generateChanges.test.ts b/test/steps/generateChanges.test.ts
index 3df8d10..5cf561b 100644
--- a/test/steps/generateChanges.test.ts
+++ b/test/steps/generateChanges.test.ts
@@ -1,7 +1,7 @@
 import { describe, expect, beforeEach, it } from "vitest";
 
 import {
-  IMPORT_EXPORT_REGEX,
+  MATCHERS,
   aliasToRelativePath,
   replaceAliasPathsInFile,
   generateChanges,
@@ -10,210 +10,395 @@ import type { Alias, ProgramPaths } from "~/types";
 
 describe("steps/generateChanges", () => {
   describe("IMPORT_EXPORT_REGEX", () => {
-    let regex: RegExp;
+    let regexes: RegExp[];
 
     beforeEach(() => {
-      regex = new RegExp(IMPORT_EXPORT_REGEX);
+      regexes = MATCHERS.map((regex) => new RegExp(regex, "g"));
     });
 
     it("matches import * statements", () => {
-      const result = regex.exec(`import * as package from 'package';`);
+      const result = regexes.map((regex) =>
+        regex.exec(`import * as package from 'package';`),
+      );
       expect(result).toMatchInlineSnapshot(`
         [
-          "import * as package from 'package'",
-          "import * as package from ",
-          "package",
+          null,
+          null,
+          null,
+          null,
+          null,
+          [
+            "import * as package from 'package'",
+            "import * as package from 'package'",
+            "package",
+          ],
+          null,
+          null,
         ]
       `);
     });
 
     it("matches import {} statements", () => {
-      const result = regex.exec(`import { package } from '~/package';`);
+      const result = regexes.map((regex) =>
+        regex.exec(`import { package } from '~/package';`),
+      );
       expect(result).toMatchInlineSnapshot(`
         [
-          "import { package } from '~/package'",
-          "import { package } from ",
-          "~/package",
+          null,
+          null,
+          null,
+          null,
+          null,
+          [
+            "import { package } from '~/package'",
+            "import { package } from '~/package'",
+            "~/package",
+          ],
+          null,
+          null,
         ]
       `);
     });
 
     it("matches import { as } statements", () => {
-      const result = regex.exec(
-        `import { package as myPackage } from '../package';`,
+      const result = regexes.map((regex) =>
+        regex.exec(`import { package as myPackage } from '../package';`),
       );
       expect(result).toMatchInlineSnapshot(`
         [
-          "import { package as myPackage } from '../package'",
-          "import { package as myPackage } from ",
-          "../package",
+          null,
+          null,
+          null,
+          null,
+          null,
+          [
+            "import { package as myPackage } from '../package'",
+            "import { package as myPackage } from '../package'",
+            "../package",
+          ],
+          null,
+          null,
         ]
       `);
     });
 
     it("matches import statements", () => {
-      const result = regex.exec(`import 'package';`);
+      const result = regexes.map((regex) => regex.exec(`import 'package';`));
       expect(result).toMatchInlineSnapshot(`
         [
-          "import 'package'",
-          "import ",
-          "package",
+          null,
+          null,
+          null,
+          [
+            "import 'package'",
+            "import 'package'",
+            "package",
+          ],
+          null,
+          null,
+          null,
+          null,
         ]
       `);
     });
 
     it("matches import statements with multiple named imports", () => {
-      const result = regex.exec(`import {
+      const result = regexes.map((regex) =>
+        regex.exec(`import {
   package as myPackage,
   otherPackage,
-} from '../package';`);
+} from '../package';`),
+      );
       expect(result).toMatchInlineSnapshot(`
         [
-          "import {
+          null,
+          null,
+          null,
+          null,
+          null,
+          null,
+          null,
+          [
+            "import {
           package as myPackage,
           otherPackage,
         } from '../package'",
-          "import {
+            "import {
           package as myPackage,
           otherPackage,
-        } from ",
-          "../package",
+        } from '../package'",
+            "../package",
+          ],
         ]
       `);
     });
 
     it("matches export * statements", () => {
-      const result = regex.exec(`export * from 'package';`);
+      const result = regexes.map((regex) =>
+        regex.exec(`export * from 'package';`),
+      );
       expect(result).toMatchInlineSnapshot(`
         [
-          "export * from 'package'",
-          "export * from ",
-          "package",
+          null,
+          null,
+          null,
+          null,
+          null,
+          [
+            "export * from 'package'",
+            "export * from 'package'",
+            "package",
+          ],
+          null,
+          null,
         ]
       `);
     });
 
     it("matches export * as statements", () => {
-      const result = regex.exec(`export * as package from 'package';`);
+      const result = regexes.map((regex) =>
+        regex.exec(`export * as package from 'package';`),
+      );
       expect(result).toMatchInlineSnapshot(`
         [
-          "export * as package from 'package'",
-          "export * as package from ",
-          "package",
+          null,
+          null,
+          null,
+          null,
+          null,
+          [
+            "export * as package from 'package'",
+            "export * as package from 'package'",
+            "package",
+          ],
+          null,
+          null,
         ]
       `);
     });
 
     it("matches export {} statements", () => {
-      const result = regex.exec(`export { package } from '~/package';`);
+      const result = regexes.map((regex) =>
+        regex.exec(`export { package } from '~/package';`),
+      );
       expect(result).toMatchInlineSnapshot(`
         [
-          "export { package } from '~/package'",
-          "export { package } from ",
-          "~/package",
+          null,
+          null,
+          null,
+          null,
+          null,
+          [
+            "export { package } from '~/package'",
+            "export { package } from '~/package'",
+            "~/package",
+          ],
+          null,
+          null,
         ]
       `);
     });
 
     it("matches export { as } statements", () => {
-      const result = regex.exec(
-        `export { package as myPackage } from '../package';`,
+      const result = regexes.map((regex) =>
+        regex.exec(`export { package as myPackage } from '../package';`),
       );
       expect(result).toMatchInlineSnapshot(`
         [
-          "export { package as myPackage } from '../package'",
-          "export { package as myPackage } from ",
-          "../package",
+          null,
+          null,
+          null,
+          null,
+          null,
+          [
+            "export { package as myPackage } from '../package'",
+            "export { package as myPackage } from '../package'",
+            "../package",
+          ],
+          null,
+          null,
         ]
       `);
     });
 
     it("matches export statements", () => {
-      const result = regex.exec(`export 'package';`);
+      const result = regexes.map((regex) => regex.exec(`export 'package';`));
       expect(result).toMatchInlineSnapshot(`
         [
-          "export 'package'",
-          "export ",
-          "package",
+          null,
+          null,
+          null,
+          [
+            "export 'package'",
+            "export 'package'",
+            "package",
+          ],
+          null,
+          null,
+          null,
+          null,
         ]
       `);
     });
 
     it("matches import statements with multiple named imports", () => {
-      const result = regex.exec(`export {
+      const result = regexes.map((regex) =>
+        regex.exec(`export {
   package as myPackage,
   otherPackage,
-} from '../package';`);
+} from '../package';`),
+      );
       expect(result).toMatchInlineSnapshot(`
         [
-          "export {
+          null,
+          null,
+          null,
+          null,
+          null,
+          null,
+          null,
+          [
+            "export {
           package as myPackage,
           otherPackage,
         } from '../package'",
-          "export {
+            "export {
           package as myPackage,
           otherPackage,
-        } from ",
-          "../package",
+        } from '../package'",
+            "../package",
+          ],
         ]
       `);
     });
 
     it("matches require statements", () => {
-      const result = regex.exec(`require('package');`);
+      const result = regexes.map((regex) => regex.exec(`require('package');`));
       expect(result).toMatchInlineSnapshot(`
         [
-          "require('package')",
-          "require(",
-          "package",
+          null,
+          [
+            "require('package')",
+            "require('package')",
+            "package",
+          ],
+          null,
+          null,
+          null,
+          null,
+          null,
+          null,
         ]
       `);
     });
 
     it("matches const require statements", () => {
-      const result = regex.exec(`const package = require('../package');`);
+      const result = regexes.map((regex) =>
+        regex.exec(`const package = require('../package');`),
+      );
       expect(result).toMatchInlineSnapshot(`
         [
-          "require('../package')",
-          "require(",
-          "../package",
+          null,
+          [
+            "require('../package')",
+            "require('../package')",
+            "../package",
+          ],
+          null,
+          null,
+          null,
+          null,
+          null,
+          null,
         ]
       `);
     });
 
     it("matches const require.resolve statements", () => {
-      const result = regex.exec(
-        `const package = require.resolve('../package');`,
+      const result = regexes.map((regex) =>
+        regex.exec(`const package = require.resolve('../package');`),
       );
       expect(result).toMatchInlineSnapshot(`
         [
-          "require.resolve('../package')",
-          "require.resolve(",
-          "../package",
+          null,
+          [
+            "require.resolve('../package')",
+            "require.resolve('../package')",
+            "../package",
+          ],
+          null,
+          null,
+          null,
+          null,
+          null,
+          null,
         ]
       `);
     });
 
     it("matches const {} require statements", () => {
-      const result = regex.exec(
-        `const { package } = require('~/package/package');`,
+      const result = regexes.map((regex) =>
+        regex.exec(`const { package } = require('~/package/package');`),
       );
       expect(result).toMatchInlineSnapshot(`
         [
-          "require('~/package/package')",
-          "require(",
-          "~/package/package",
+          null,
+          [
+            "require('~/package/package')",
+            "require('~/package/package')",
+            "~/package/package",
+          ],
+          null,
+          null,
+          null,
+          null,
+          null,
+          null,
         ]
       `);
     });
 
     it("matches dynamic import statements", () => {
-      const result = regex.exec(`import('package');`);
+      const result = regexes.map((regex) => regex.exec(`import('package');`));
       expect(result).toMatchInlineSnapshot(`
         [
-          "import('package')",
-          "import(",
-          "package",
+          null,
+          [
+            "import('package')",
+            "import('package')",
+            "package",
+          ],
+          null,
+          null,
+          null,
+          null,
+          null,
+          null,
+        ]
+      `);
+    });
+
+    it("does not match comments with the word 'import'", () => {
+      const result = regexes.map((regex) =>
+        regex.exec(`
+// import comment
+import * as package from "hello";
+      `),
+      );
+      expect(result).toMatchInlineSnapshot(`
+        [
+          null,
+          null,
+          null,
+          null,
+          [
+            "import * as package from \\"hello\\"",
+            "import * as package from \\"hello\\"",
+            "hello",
+          ],
+          null,
+          null,
+          null,
         ]
       `);
     });