fix: remove global bin when unlink

close #1584
PR #2425

.changeset/chatty-colts-invent.md

@@ -0,0 +1,7 @@
+---
+"@pnpm/remove-bins": major
+"@pnpm/modules-cleaner": patch
+"supi": patch
+---
+
+Remove global bins when unlinking.

packages/modules-cleaner/package.json

@@ -31,21 +31,18 @@
     "@pnpm/filter-lockfile": "workspace:4.0.3",
     "@pnpm/lockfile-types": "workspace:2.0.1",
     "@pnpm/lockfile-utils": "workspace:2.0.14",
-    "@pnpm/package-bins": "workspace:4.0.6",
     "@pnpm/pkgid-to-filename": "^3.0.0",
     "@pnpm/read-modules-dir": "workspace:2.0.1",
-    "@pnpm/read-package-json": "workspace:3.1.2",
+    "@pnpm/remove-bins": "workspace:0.0.0",
     "@pnpm/store-controller-types": "workspace:8.0.1",
     "@pnpm/types": "workspace:6.1.0",
     "@zkochan/rimraf": "1.0.0",
     "dependency-path": "workspace:5.0.1",
-    "is-windows": "1.0.2",
     "ramda": "0.27.0"
   },
   "devDependencies": {
     "@pnpm/logger": "3.2.2",
     "@pnpm/modules-cleaner": "link:",
-    "@types/is-windows": "^1.0.0",
     "@types/ramda": "^0.27.6"
   },
   "bugs": {

packages/modules-cleaner/src/removeDirectDependency.ts

@@ -1,12 +1,8 @@
 import {
-  removalLogger,
   rootLogger,
 } from '@pnpm/core-loggers'
-import binify from '@pnpm/package-bins'
-import { safeReadPackageFromDir } from '@pnpm/read-package-json'
-import { DependenciesField, DependencyManifest } from '@pnpm/types'
-import rimraf = require('@zkochan/rimraf')
-import isWindows = require('is-windows')
+import { removeBin, removeBinsOfDependency } from '@pnpm/remove-bins'
+import { DependenciesField } from '@pnpm/types'
 import path = require('path')
 
 export default async function removeDirectDependency (
@@ -22,9 +18,10 @@ export default async function removeDirectDependency (
     rootDir: string,
   }
 ) {
+  const dependencyDir = path.join(opts.modulesDir, dependency.name)
   const results = await Promise.all([
-    removeBins(dependency.name, opts),
-    !opts.dryRun && remove(path.join(opts.modulesDir, dependency.name)) as any, // tslint:disable-line:no-any
+    removeBinsOfDependency(dependencyDir, opts),
+    !opts.dryRun && removeBin(dependencyDir) as any, // tslint:disable-line:no-any
   ])
 
   const uninstalledPkg = results[0]
@@ -42,44 +39,3 @@ export default async function removeDirectDependency (
     })
   }
 }
-
-async function removeOnWin (cmd: string) {
-  removalLogger.debug(cmd)
-  await Promise.all([
-    rimraf(cmd),
-    rimraf(`${cmd}.ps1`),
-    rimraf(`${cmd}.cmd`),
-  ])
-}
-
-function removeOnNonWin (p: string) {
-  removalLogger.debug(p)
-  return rimraf(p)
-}
-
-const remove = isWindows() ? removeOnWin : removeOnNonWin
-
-async function removeBins (
-  uninstalledPkg: string,
-  opts: {
-    dryRun?: boolean,
-    modulesDir: string,
-    binsDir: string,
-  }
-) {
-  const uninstalledPkgPath = path.join(opts.modulesDir, uninstalledPkg)
-  const uninstalledPkgJson = await safeReadPackageFromDir(uninstalledPkgPath) as DependencyManifest
-
-  if (!uninstalledPkgJson) return
-  const cmds = await binify(uninstalledPkgJson, uninstalledPkgPath)
-
-  if (!opts.dryRun) {
-    await Promise.all(
-      cmds
-        .map((cmd) => path.join(opts.binsDir, cmd.name))
-        .map(remove)
-    )
-  }
-
-  return uninstalledPkgJson
-}

packages/modules-cleaner/tsconfig.json

@@ -21,14 +21,11 @@
     {
       "path": "../lockfile-utils"
     },
-    {
-      "path": "../package-bins"
-    },
     {
       "path": "../read-modules-dir"
     },
     {
-      "path": "../read-package-json"
+      "path": "../remove-bins"
     },
     {
       "path": "../store-controller-types"

packages/remove-bins/README.md

@@ -0,0 +1,19 @@
+# @pnpm/remove-bins
+
+> Remove bins from ./bin
+
+## Install
+
+```
+pnpm install @pnpm/remove-bins
+```
+
+## API
+
+### `removeBin(...args)`
+
+### `removeBinsOfDependency(...args)`
+
+## License
+
+MIT

packages/remove-bins/package.json

@@ -0,0 +1,46 @@
+{
+  "name": "@pnpm/remove-bins",
+  "version": "0.0.0",
+  "description": "Remove bins from .bin",
+  "author": "Zoltan Kochan <z@kochan.io> (https://www.kochan.io/)",
+  "main": "lib/index.js",
+  "typings": "lib/index.d.ts",
+  "files": [
+    "lib",
+    "!*.map"
+  ],
+  "peerDependencies": {
+    "@pnpm/logger": "^3.1.0"
+  },
+  "keywords": [],
+  "license": "MIT",
+  "engines": {
+    "node": ">=10.13"
+  },
+  "repository": "https://github.com/pnpm/pnpm/blob/master/packages/remove-bins",
+  "homepage": "https://github.com/pnpm/pnpm/blob/master/packages/remove-bins#readme",
+  "scripts": {
+    "start": "pnpm run tsc -- --watch",
+    "test": "pnpm run compile",
+    "lint": "tslint -c ../../tslint.json src/**/*.ts test/**/*.ts",
+    "prepublishOnly": "pnpm run compile",
+    "compile": "rimraf lib tsconfig.tsbuildinfo && tsc --build"
+  },
+  "dependencies": {
+    "@pnpm/core-loggers": "workspace:4.1.1",
+    "@pnpm/package-bins": "workspace:4.0.6",
+    "@pnpm/read-package-json": "workspace:3.1.2",
+    "@pnpm/types": "workspace:6.1.0",
+    "@zkochan/rimraf": "1.0.0",
+    "is-windows": "1.0.2"
+  },
+  "devDependencies": {
+    "@pnpm/logger": "3.2.2",
+    "@types/is-windows": "^1.0.0",
+    "@types/ramda": "^0.27.6"
+  },
+  "bugs": {
+    "url": "https://github.com/pnpm/pnpm/issues"
+  },
+  "funding": "https://opencollective.com/pnpm"
+}

packages/remove-bins/src/index.ts

@@ -0,0 +1,6 @@
+import { removeBin, removeBinsOfDependency } from './removeBins'
+
+export {
+  removeBin,
+  removeBinsOfDependency,
+}

packages/remove-bins/src/removeBins.ts

@@ -0,0 +1,49 @@
+import {
+  removalLogger,
+} from '@pnpm/core-loggers'
+import binify from '@pnpm/package-bins'
+import { safeReadPackageFromDir } from '@pnpm/read-package-json'
+import { DependencyManifest } from '@pnpm/types'
+import rimraf = require('@zkochan/rimraf')
+import isWindows = require('is-windows')
+import path = require('path')
+
+async function removeOnWin (cmd: string) {
+  removalLogger.debug(cmd)
+  await Promise.all([
+    rimraf(cmd),
+    rimraf(`${cmd}.ps1`),
+    rimraf(`${cmd}.cmd`),
+  ])
+}
+
+function removeOnNonWin (p: string) {
+  removalLogger.debug(p)
+  return rimraf(p)
+}
+
+export const removeBin = isWindows() ? removeOnWin : removeOnNonWin
+
+export async function removeBinsOfDependency (
+  dependencyDir: string,
+  opts: {
+    dryRun?: boolean,
+    binsDir: string,
+  }
+) {
+
+  const uninstalledPkgJson = await safeReadPackageFromDir(dependencyDir) as DependencyManifest
+
+  if (!uninstalledPkgJson) return
+  const cmds = await binify(uninstalledPkgJson, dependencyDir)
+
+  if (!opts.dryRun) {
+    await Promise.all(
+      cmds
+        .map((cmd) => path.join(opts.binsDir, cmd.name))
+        .map(removeBin)
+    )
+  }
+
+  return uninstalledPkgJson
+}

packages/remove-bins/tsconfig.json

@@ -0,0 +1,25 @@
+{
+  "extends": "@pnpm/tsconfig",
+  "compilerOptions": {
+    "outDir": "lib",
+    "rootDir": "src"
+  },
+  "include": [
+    "src/**/*.ts",
+    "../../typings/**/*.d.ts"
+  ],
+  "references": [
+    {
+      "path": "../core-loggers"
+    },
+    {
+      "path": "../package-bins"
+    },
+    {
+      "path": "../read-package-json"
+    },
+    {
+      "path": "../types"
+    }
+  ]
+}

packages/supi/package.json

@@ -40,6 +40,7 @@
     "@pnpm/read-modules-dir": "workspace:2.0.1",
     "@pnpm/read-package-json": "workspace:3.1.2",
     "@pnpm/read-project-manifest": "workspace:1.0.8",
+    "@pnpm/remove-bins": "workspace:0.0.0",
     "@pnpm/resolve-dependencies": "workspace:16.0.1",
     "@pnpm/resolver-base": "workspace:7.0.2",
     "@pnpm/store-controller-types": "workspace:8.0.1",

packages/supi/src/install/extendInstallOptions.ts

@@ -71,6 +71,7 @@ export interface StrictInstallOptions {
   forceShamefullyHoist: boolean,
 
   global: boolean,
+  globalBin: string,
 }
 
 export type InstallOptions = Partial<StrictInstallOptions> &

packages/supi/src/install/index.ts

@@ -27,6 +27,7 @@ import { getAllDependenciesFromManifest } from '@pnpm/manifest-utils'
 import { write as writeModulesYaml } from '@pnpm/modules-yaml'
 import readModulesDirs from '@pnpm/read-modules-dir'
 import { safeReadPackageFromDir as safeReadPkgFromDir } from '@pnpm/read-package-json'
+import { removeBin } from '@pnpm/remove-bins'
 import resolveDependencies, {
   ResolvedDirectDependency,
   ResolvedPackage,
@@ -304,6 +305,9 @@ export async function mutateModules (
           break
         }
         case 'unlinkSome': {
+          if (project.manifest?.name && opts.globalBin) {
+            await removeBin(path.join(opts.globalBin, project.manifest?.name))
+          }
           const packagesToInstall: string[] = []
           const allDeps = getAllDependenciesFromManifest(project.manifest)
           for (const depName of project.dependencyNames) {

packages/supi/test/unlink.ts

@@ -1,5 +1,6 @@
 import { WANTED_LOCKFILE } from '@pnpm/constants'
 import { prepareEmpty } from '@pnpm/prepare'
+import fs = require('fs')
 import isInnerLink = require('is-inner-link')
 import path = require('path')
 import exists = require('path-exists')
@@ -282,3 +283,59 @@ test("don't unlink package that is not a link", async (t: tape.Test) => {
     message: 'is-positive is not an external link',
   }), 'reported warning')
 })
+
+test('unlink would remove global bin', async (t: tape.Test) => {
+  const project = prepareEmpty(t)
+  process.chdir('..')
+  fs.mkdirSync('bin')
+  fs.mkdirSync('is-subdir')
+  fs.writeFileSync('is-subdir/index.js', ' ')
+
+  await Promise.all([
+    writeJsonFile('is-subdir/package.json', {
+      bin: 'index.js',
+      dependencies: {
+        'is-windows': '^1.0.0',
+      },
+      name: 'is-subdir',
+      version: '1.0.0',
+    }),
+  ])
+
+  const opts = await testDefaults({
+    fastUnpack: false,
+    globalBin: path.resolve('bin'),
+    linkToBin: path.resolve('bin'),
+    store: path.resolve('.store'),
+  })
+
+  let manifest = await link(
+    ['is-subdir'],
+    path.join('project', 'node_modules'),
+    {
+      ...opts,
+      dir: path.resolve('project'),
+      manifest: {
+        dependencies: {
+          'is-subdir': '^1.0.0',
+        },
+        name: 'is-subdir',
+      },
+    }
+  )
+  t.ok(fs.existsSync(path.resolve('bin/is-subdir')), 'bin is installed in global bin directory')
+
+  await mutateModules(
+    [
+      {
+        dependencyNames: ['is-subdir'],
+        manifest,
+        mutation: 'unlinkSome',
+        rootDir: process.cwd(),
+      },
+    ],
+    opts
+  )
+
+  t.notOk(fs.existsSync(path.resolve('bin/is-subdir')), 'bin is removed in global bin directory')
+})

packages/supi/tsconfig.json

@@ -78,6 +78,9 @@
     {
       "path": "../read-project-manifest"
     },
+    {
+      "path": "../remove-bins"
+    },
     {
       "path": "../resolve-dependencies"
     },

pnpm-lock.yaml

@@ -1000,20 +1000,17 @@ importers:
       '@pnpm/filter-lockfile': 'link:../filter-lockfile'
       '@pnpm/lockfile-types': 'link:../lockfile-types'
       '@pnpm/lockfile-utils': 'link:../lockfile-utils'
-      '@pnpm/package-bins': 'link:../package-bins'
       '@pnpm/pkgid-to-filename': 3.0.0
       '@pnpm/read-modules-dir': 'link:../read-modules-dir'
-      '@pnpm/read-package-json': 'link:../read-package-json'
+      '@pnpm/remove-bins': 'link:../remove-bins'
       '@pnpm/store-controller-types': 'link:../store-controller-types'
       '@pnpm/types': 'link:../types'
       '@zkochan/rimraf': 1.0.0
       dependency-path: 'link:../dependency-path'
-      is-windows: 1.0.2
       ramda: 0.27.0
     devDependencies:
       '@pnpm/logger': 3.2.2
       '@pnpm/modules-cleaner': 'link:'
-      '@types/is-windows': 1.0.0
       '@types/ramda': 0.27.6
     specifiers:
       '@pnpm/core-loggers': 'workspace:4.1.1'
@@ -1022,17 +1019,14 @@ importers:
       '@pnpm/lockfile-utils': 'workspace:2.0.14'
       '@pnpm/logger': 3.2.2
       '@pnpm/modules-cleaner': 'link:'
-      '@pnpm/package-bins': 'workspace:4.0.6'
       '@pnpm/pkgid-to-filename': ^3.0.0
       '@pnpm/read-modules-dir': 'workspace:2.0.1'
-      '@pnpm/read-package-json': 'workspace:3.1.2'
+      '@pnpm/remove-bins': 'workspace:0.0.0'
       '@pnpm/store-controller-types': 'workspace:8.0.1'
       '@pnpm/types': 'workspace:6.1.0'
-      '@types/is-windows': ^1.0.0
       '@types/ramda': ^0.27.6
       '@zkochan/rimraf': 1.0.0
       dependency-path: 'workspace:5.0.1'
-      is-windows: 1.0.2
       ramda: 0.27.0
   packages/modules-yaml:
     dependencies:
@@ -2390,6 +2384,28 @@ importers:
       '@pnpm/normalize-registries': 'workspace:1.0.2'
       '@pnpm/types': 'workspace:6.1.0'
       realpath-missing: 1.0.0
+  packages/remove-bins:
+    dependencies:
+      '@pnpm/core-loggers': 'link:../core-loggers'
+      '@pnpm/package-bins': 'link:../package-bins'
+      '@pnpm/read-package-json': 'link:../read-package-json'
+      '@pnpm/types': 'link:../types'
+      '@zkochan/rimraf': 1.0.0
+      is-windows: 1.0.2
+    devDependencies:
+      '@pnpm/logger': 3.2.2
+      '@types/is-windows': 1.0.0
+      '@types/ramda': 0.27.6
+    specifiers:
+      '@pnpm/core-loggers': 'workspace:4.1.1'
+      '@pnpm/logger': 3.2.2
+      '@pnpm/package-bins': 'workspace:4.0.6'
+      '@pnpm/read-package-json': 'workspace:3.1.2'
+      '@pnpm/types': 'workspace:6.1.0'
+      '@types/is-windows': ^1.0.0
+      '@types/ramda': ^0.27.6
+      '@zkochan/rimraf': 1.0.0
+      is-windows: 1.0.2
   packages/resolve-dependencies:
     dependencies:
       '@pnpm/core-loggers': 'link:../core-loggers'
@@ -2588,6 +2604,7 @@ importers:
       '@pnpm/read-modules-dir': 'link:../read-modules-dir'
       '@pnpm/read-package-json': 'link:../read-package-json'
       '@pnpm/read-project-manifest': 'link:../read-project-manifest'
+      '@pnpm/remove-bins': 'link:../remove-bins'
       '@pnpm/resolve-dependencies': 'link:../resolve-dependencies'
       '@pnpm/resolver-base': 'link:../resolver-base'
       '@pnpm/store-controller-types': 'link:../store-controller-types'
@@ -2691,6 +2708,7 @@ importers:
       '@pnpm/read-modules-dir': 'workspace:2.0.1'
       '@pnpm/read-package-json': 'workspace:3.1.2'
       '@pnpm/read-project-manifest': 'workspace:1.0.8'
+      '@pnpm/remove-bins': 'workspace:0.0.0'
       '@pnpm/resolve-dependencies': 'workspace:16.0.1'
       '@pnpm/resolver-base': 'workspace:7.0.2'
       '@pnpm/store-controller-types': 'workspace:8.0.1'