Commit a4d1bc2

Anton Golub <antongolub@antongolub.com>
2025-10-14 18:56:37
fix: checks `node_modules` ref on linking (#1355)
continues #1349
1 parent 9ef6d3c
build/cli.cjs
@@ -218,11 +218,9 @@ function main() {
   });
 }
 var rmrf = (p) => {
+  var _a2;
   if (!p) return;
-  try {
-    import_index.fs.lstatSync(p).isSymbolicLink() ? import_index.fs.unlinkSync(p) : import_index.fs.rmSync(p, { force: true, recursive: true });
-  } catch (e) {
-  }
+  ((_a2 = lstat(p)) == null ? void 0 : _a2.isSymbolicLink()) ? import_index.fs.unlinkSync(p) : import_index.fs.rmSync(p, { force: true, recursive: true });
 };
 function runScript(script, scriptPath, tempPath) {
   return __async(this, null, function* () {
@@ -238,16 +236,7 @@ function runScript(script, scriptPath, tempPath) {
       }
       const cwd = import_index.path.dirname(scriptPath);
       if (typeof argv.preferLocal === "string") {
-        linkNodeModules(cwd, argv.preferLocal);
-        try {
-          const aliasPath = import_index.path.resolve(cwd, "node_modules");
-          if (import_index.fs.existsSync(aliasPath) && import_index.fs.lstatSync(aliasPath).isSymbolicLink()) {
-            nmLink = aliasPath;
-          } else {
-            nmLink = "";
-          }
-        } catch (e) {
-        }
+        nmLink = linkNodeModules(cwd, argv.preferLocal);
       }
       if (argv.install) {
         yield (0, import_deps.installDeps)((0, import_deps.parseDeps)(script), cwd, argv.registry);
@@ -264,9 +253,23 @@ function linkNodeModules(cwd, external) {
   const nm = "node_modules";
   const alias = import_index.path.resolve(cwd, nm);
   const target = import_index.path.basename(external) === nm ? import_index.path.resolve(external) : import_index.path.resolve(external, nm);
-  if (import_index.fs.existsSync(alias) || !import_index.fs.existsSync(target)) return "";
+  const aliasStat = lstat(alias);
+  const targetStat = lstat(target);
+  if (!(targetStat == null ? void 0 : targetStat.isDirectory()))
+    throw new import_index.Fail(
+      `Can't link node_modules: ${target} doesn't exist or is not a directory`
+    );
+  if ((aliasStat == null ? void 0 : aliasStat.isDirectory()) && alias !== target)
+    throw new import_index.Fail(`Can't link node_modules: ${alias} already exists`);
+  if (aliasStat) return "";
   import_index.fs.symlinkSync(target, alias, "junction");
-  return target;
+  return alias;
+}
+function lstat(p) {
+  try {
+    return import_index.fs.lstatSync(p);
+  } catch (e) {
+  }
 }
 function readScript() {
   return __async(this, null, function* () {
src/cli.ts
@@ -128,22 +128,19 @@ export async function main(): Promise<void> {
   await runScript(script, scriptPath, tempPath)
 }
 
-// Short & safe remove: unlink symlinks; recurse only for real dirs/files
 const rmrf = (p: string) => {
   if (!p) return
-  try {
-    fs.lstatSync(p).isSymbolicLink()
-      ? fs.unlinkSync(p)
-      : fs.rmSync(p, { force: true, recursive: true })
-  } catch {}
-}
 
+  lstat(p)?.isSymbolicLink()
+    ? fs.unlinkSync(p)
+    : fs.rmSync(p, { force: true, recursive: true })
+}
 async function runScript(
   script: string,
   scriptPath: string,
   tempPath: string
 ): Promise<void> {
-  let nmLink = '' // will hold the alias path (./node_modules) ONLY if it's a symlink
+  let nmLink = ''
   const rmTemp = () => {
     rmrf(tempPath)
     rmrf(nmLink)
@@ -154,25 +151,9 @@ async function runScript(
       await fs.writeFile(tempPath, script)
     }
     const cwd = path.dirname(scriptPath)
-
     if (typeof argv.preferLocal === 'string') {
-      // Keep original behaviour: linkNodeModules returns TARGET (unchanged API)
-      linkNodeModules(cwd, argv.preferLocal)
-
-      // For cleanup, compute ALIAS and only unlink if it's a symlink
-      try {
-        const aliasPath = path.resolve(cwd, 'node_modules')
-        if (
-          fs.existsSync(aliasPath) &&
-          fs.lstatSync(aliasPath).isSymbolicLink()
-        ) {
-          nmLink = aliasPath
-        } else {
-          nmLink = ''
-        }
-      } catch {}
+      nmLink = linkNodeModules(cwd, argv.preferLocal)
     }
-
     if (argv.install) {
       await installDeps(parseDeps(script), cwd, argv.registry)
     }
@@ -194,12 +175,25 @@ function linkNodeModules(cwd: string, external: string): string {
     path.basename(external) === nm
       ? path.resolve(external)
       : path.resolve(external, nm)
+  const aliasStat = lstat(alias)
+  const targetStat = lstat(target)
 
-  if (fs.existsSync(alias) || !fs.existsSync(target)) return ''
+  if (!targetStat?.isDirectory())
+    throw new Fail(
+      `Can't link node_modules: ${target} doesn't exist or is not a directory`
+    )
+  if (aliasStat?.isDirectory() && alias !== target)
+    throw new Fail(`Can't link node_modules: ${alias} already exists`)
+  if (aliasStat) return ''
 
   fs.symlinkSync(target, alias, 'junction')
-  // Keep behaviour stable: return TARGET (not alias)
-  return target
+  return alias
+}
+
+function lstat(p: string) {
+  try {
+    return fs.lstatSync(p)
+  } catch {}
 }
 
 async function readScript() {
test/cli.test.js
@@ -175,28 +175,99 @@ describe('cli', () => {
     }
   })
 
-  test('supports --prefer-local to load modules', async () => {
-    const cwd = tmpdir()
-    const external = tmpdir()
-    await fs.outputJson(path.join(external, 'node_modules/a/package.json'), {
+  describe('`--prefer-local`', () => {
+    const pkgIndex = `export const a = 'AAA'`
+    const pkgJson = {
       name: 'a',
       version: '1.0.0',
       type: 'module',
       exports: './index.js',
-    })
-    await fs.outputFile(
-      path.join(external, 'node_modules/a/index.js'),
-      `
-export const a = 'AAA'
-`
-    )
+    }
     const script = `
 import {a} from 'a'
 console.log(a);
 `
-    const out =
-      await $`node build/cli.js --cwd=${cwd} --prefer-local=${external} --test <<< ${script}`
-    assert.equal(out.stdout, 'AAA\n')
+
+    test('true', async () => {
+      const cwd = tmpdir()
+      await fs.outputFile(path.join(cwd, 'node_modules/a/index.js'), pkgIndex)
+      await fs.outputJson(
+        path.join(cwd, 'node_modules/a/package.json'),
+        pkgJson
+      )
+
+      const out =
+        await $`node build/cli.js --cwd=${cwd} --prefer-local=true --test <<< ${script}`
+      assert.equal(out.stdout, 'AAA\n')
+      assert.ok(await fs.exists(path.join(cwd, 'node_modules/a/index.js')))
+    })
+
+    test('external dir', async () => {
+      const cwd = tmpdir()
+      const external = tmpdir()
+      await fs.outputFile(
+        path.join(external, 'node_modules/a/index.js'),
+        pkgIndex
+      )
+      await fs.outputJson(
+        path.join(external, 'node_modules/a/package.json'),
+        pkgJson
+      )
+
+      const out =
+        await $`node build/cli.js --cwd=${cwd} --prefer-local=${external} --test <<< ${script}`
+      assert.equal(out.stdout, 'AAA\n')
+      assert.ok(await fs.exists(path.join(external, 'node_modules/a/index.js')))
+    })
+
+    test('external alias', async () => {
+      const cwd = tmpdir()
+      const external = tmpdir()
+      await fs.outputFile(
+        path.join(external, 'node_modules/a/index.js'),
+        pkgIndex
+      )
+      await fs.outputJson(
+        path.join(external, 'node_modules/a/package.json'),
+        pkgJson
+      )
+      await fs.symlinkSync(
+        path.join(external, 'node_modules'),
+        path.join(cwd, 'node_modules'),
+        'junction'
+      )
+
+      const out =
+        await $`node build/cli.js --cwd=${cwd} --prefer-local=true --test <<< ${script}`
+      assert.equal(out.stdout, 'AAA\n')
+      assert.ok(await fs.exists(path.join(cwd, 'node_modules')))
+    })
+
+    test('throws if exists', async () => {
+      const cwd = tmpdir()
+      const external = tmpdir()
+      await fs.outputFile(path.join(cwd, 'node_modules/a/index.js'), pkgIndex)
+      await fs.outputFile(
+        path.join(external, 'node_modules/a/index.js'),
+        pkgIndex
+      )
+      assert.rejects(
+        () =>
+          $`node build/cli.js --cwd=${cwd} --prefer-local=${external} --test <<< ${script}`,
+        /node_modules already exists/
+      )
+    })
+
+    test('throws if not dir', async () => {
+      const cwd = tmpdir()
+      const external = tmpdir()
+      await fs.outputFile(path.join(external, 'node_modules'), pkgIndex)
+      assert.rejects(
+        () =>
+          $`node build/cli.js --cwd=${cwd} --prefer-local=${external} --test <<< ${script}`,
+        /node_modules doesn't exist or is not a directory/
+      )
+    })
   })
 
   test('scripts from https 200', async () => {
.size-limit.json
@@ -66,7 +66,7 @@
       "README.md",
       "LICENSE"
     ],
-    "limit": "874.6 kB",
+    "limit": "874.70 kB",
     "brotli": false,
     "gzip": false
   }