fix: allow overriding path to npm bin in workspaces

npm · Sep 14, 2023 · de319e8 · de319e8
1 parent cad156a
commit de319e8
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 23 deletions.
diff --git a/lib/config.js b/lib/config.js
@@ -22,19 +22,36 @@ const deglob = (v) => makePosix(v).replace(/[/*]+$/, '')
 const posixDir = (v) => `${v === '.' ? '' : deglob(v).replace(/\/$/, '')}${posix.sep}`
 const posixGlob = (str) => `${posixDir(str)}**`
 
-const getCmdPath = (key, { rootConfig, defaultConfig, isRoot, pkg, rootPkg }) => {
-  // Make a path relative from a workspace to the root if we are in a workspace
-  const wsToRoot = (p) => isRoot ? p : makePosix(join(relative(pkg.path, rootPkg.path), p))
+const getCmdPath = (key, { pkgConfig, rootConfig, isRoot, pkg, rootPkg }) => {
+  const result = (local, isRelative) => {
+    let root = local
+    const isLocal = local.startsWith('.') || local.startsWith('/')
+
+    if (isLocal) {
+      if (isRelative) {
+        // Make a path relative from a workspace to the root if we are in a workspace
+        local = makePosix(join(relative(pkg.path, rootPkg.path), local))
+      }
+      local = `node ${local}`
+      root = `node ${root}`
+    }
 
-  const rootPath = rootConfig[key]
-  const defaultPath = defaultConfig[key]
-  const isLocal = rootPath && rootPath !== defaultPath
+    return {
+      isLocal,
+      local,
+      root,
+    }
+  }
 
-  return {
-    isLocal,
-    root: !isLocal ? defaultPath : `node ${rootPath}`,
-    local: !isLocal ? defaultPath : `node ${wsToRoot(rootPath)}`,
+  if (pkgConfig[key]) {
+    return result(pkgConfig[key])
   }
+
+  if (rootConfig[key]) {
+    return result(rootConfig[key], !isRoot)
+  }
+
+  return result(key)
 }
 
 const mergeConfigs = (...configs) => {
@@ -138,9 +155,8 @@ const getFullConfig = async ({
     ] : [],
   ]
 
-  // root only configs
-  const npmPath = getCmdPath('npm', { rootConfig, defaultConfig, isRoot, pkg, rootPkg })
-  const npxPath = getCmdPath('npx', { rootConfig, defaultConfig, isRoot, pkg, rootPkg })
+  const npmPath = getCmdPath('npm', { pkgConfig, rootConfig, isRoot, pkg, rootPkg })
+  const npxPath = getCmdPath('npx', { pkgConfig, rootConfig, isRoot, pkg, rootPkg })
 
   // these are written to ci yml files so it needs to always use posix
   const pkgPath = makePosix(relative(rootPkg.path, pkg.path)) || '.'

diff --git a/lib/content/index.js b/lib/content/index.js
@@ -163,8 +163,6 @@ module.exports = {
   codeowner: '@npm/cli-team',
   eslint: true,
   publish: false,
-  npm: 'npm',
-  npx: 'npx',
   updateNpm: true,
   dependabot: 'increase-if-necessary',
   unwantedPackages: [

diff --git a/test/apply/npm-bin.js b/test/apply/npm-bin.js
@@ -21,16 +21,33 @@ t.test('relative npm bin with workspaces', async (t) => {
     ok: true,
     package: {
       templateOSS: {
-        npm: 'cli.js',
+        content: 'rootContent',
       },
     },
-    workspaces: { a: '@name/aaaa', b: 'bbb' },
+    workspaces: {
+      a: 'a',
+      b: { name: 'b', templateOSS: { npm: './local-workspace-cli.js' } },
+      c: { name: 'c', templateOSS: { npm: 'npm' } },
+      d: { name: 'd', templateOSS: { content: '../../wsContent' } },
+      e: { name: 'e', templateOSS: { content: '../../wsContent', npm: 'npm' } },
+    },
+    testdir: {
+      rootContent: { 'index.js': 'module.exports={ npm: "./cli.js" }' },
+      wsContent: { 'index.js': 'module.exports={ npm: "../../cli.js" }' },
+    },
   })
   await s.apply()
-  const { scripts } = await s.readJson('package.json')
-  const { scripts: scriptsA } = await s.readJson(join(s.workspaces.a, 'package.json'))
-  const { scripts: scriptsB } = await s.readJson(join(s.workspaces.b, 'package.json'))
-  t.equal(scripts.posttest, 'node cli.js run lint')
-  t.equal(scriptsA.posttest, 'node ../../cli.js run lint')
-  t.equal(scriptsB.posttest, 'node ../../cli.js run lint')
+
+  const readScripts = (p) => s.readJson(join(p, 'package.json')).then(r => r.scripts)
+
+  const ws = s.workspaces
+  const pkgs = ['', ws.a, ws.b, ws.c, ws.d, ws.e]
+  const [root, a, b, c, d, e] = await Promise.all(pkgs.map(readScripts))
+
+  t.equal(root.posttest, 'node ./cli.js run lint')
+  t.equal(a.posttest, 'node ../../cli.js run lint')
+  t.equal(b.posttest, 'node ./local-workspace-cli.js run lint')
+  t.equal(c.posttest, 'npm run lint')
+  t.equal(d.posttest, 'node ../../cli.js run lint')
+  t.equal(e.posttest, 'npm run lint')
 })