feat: new option - extend-node-path (#3799)

close #2890

.changeset/six-ducks-perform.md

@@ -0,0 +1,10 @@
+---
+"@pnpm/build-modules": minor
+"@pnpm/config": minor
+"@pnpm/headless": minor
+"@pnpm/hoist": minor
+"@pnpm/link-bins": minor
+"supi": patch
+---
+
+New option added: `extendNodePath`. When it is set to `false`, pnpm does not set the `NODE_PATH` environment variable in the command shims.

.changeset/young-cows-fry.md

@@ -0,0 +1,5 @@
+---
+"@pnpm/config": major
+---
+
+NODE_PATH is not set in the command shims of globally installed packages.

packages/build-modules/src/index.ts

@@ -17,6 +17,7 @@ export default async (
   opts: {
     childConcurrency?: number
     depsToBuild?: Set<string>
+    extendNodePath?: boolean
     extraBinPaths?: string[]
     extraEnv?: Record<string, string>
     lockfileDir: string
@@ -65,6 +66,7 @@ async function buildDependency (
   depPath: string,
   depGraph: DependenciesGraph,
   opts: {
+    extendNodePath?: boolean
     extraBinPaths?: string[]
     extraEnv?: Record<string, string>
     lockfileDir: string
@@ -182,6 +184,7 @@ export async function linkBinsOfDependencies (
   depNode: DependenciesGraphNode,
   depGraph: DependenciesGraph,
   opts: {
+    extendNodePath?: boolean
     optional: boolean
     warn: (message: string) => void
   }
@@ -219,11 +222,11 @@ export async function linkBinsOfDependencies (
     }))
   )
 
-  await linkBinsOfPackages(pkgs, binPath, { warn: opts.warn })
+  await linkBinsOfPackages(pkgs, binPath, { extendNodePath: opts.extendNodePath, warn: opts.warn })
 
   // link also the bundled dependencies` bins
   if (depNode.hasBundledDependencies) {
     const bundledModules = path.join(depNode.dir, 'node_modules')
-    await linkBins(bundledModules, binPath, { warn: opts.warn })
+    await linkBins(bundledModules, binPath, { extendNodePath: opts.extendNodePath, warn: opts.warn })
   }
 }

packages/config/src/Config.ts

@@ -134,6 +134,7 @@ export interface Config {
 
   testPattern?: string[]
   changedFilesIgnorePattern?: string[]
+  extendNodePath?: boolean
 }
 
 export interface ConfigWithDeprecatedSettings extends Config {

packages/config/src/index.ts

@@ -42,6 +42,7 @@ export const types = Object.assign({
   dir: String,
   'enable-modules-dir': Boolean,
   'enable-pre-post-scripts': Boolean,
+  'extend-node-path': Boolean,
   'fetch-timeout': Number,
   'fetching-concurrency': Number,
   filter: [String, Array],
@@ -167,6 +168,7 @@ export default async (
     bail: true,
     color: 'auto',
     'enable-modules-dir': true,
+    'extend-node-path': true,
     'fetch-retries': 2,
     'fetch-retry-factor': 10,
     'fetch-retry-maxtimeout': 60000,
@@ -302,6 +304,7 @@ export default async (
     pnpmConfig.saveProd = true
     pnpmConfig.saveDev = false
     pnpmConfig.saveOptional = false
+    pnpmConfig.extendNodePath = false
     if ((pnpmConfig.hoistPattern != null) && (pnpmConfig.hoistPattern.length > 1 || pnpmConfig.hoistPattern[0] !== '*')) {
       if (opts.cliOptions['hoist-pattern']) {
         throw new PnpmError('CONFIG_CONFLICT_HOIST_PATTERN_WITH_GLOBAL',

packages/config/test/index.ts

@@ -48,7 +48,7 @@ test('throw error if --link-workspace-packages is used with --global', async ()
   }
 })
 
-test('"save" should always be true during global installation', async () => {
+test('correct settings on global install', async () => {
   const { config } = await getConfig({
     cliOptions: {
       global: true,
@@ -59,7 +59,8 @@ test('"save" should always be true during global installation', async () => {
       version: '1.0.0',
     },
   })
-  expect(config.save).toBeTruthy()
+  expect(config.save).toBe(true)
+  expect(config.extendNodePath).toBe(false)
 })
 
 test('throw error if --shared-workspace-lockfile is used with --global', async () => {

packages/headless/src/index.ts

@@ -81,6 +81,7 @@ export interface HeadlessOptions {
   }
   enablePnp?: boolean
   engineStrict: boolean
+  extendNodePath?: boolean
   extraBinPaths?: string[]
   ignoreScripts: boolean
   ignorePackageManifest?: boolean
@@ -285,6 +286,7 @@ export default async (opts: HeadlessOptions) => {
         packages: omit(Array.from(skipped), filteredLockfile.packages),
       }
       newHoistedDependencies = await hoist({
+        extendNodePath: opts.extendNodePath,
         lockfile: hoistLockfile,
         lockfileDir,
         privateHoistedModulesDir: hoistedModulesDir,
@@ -336,6 +338,7 @@ export default async (opts: HeadlessOptions) => {
       await buildModules(graph, Array.from(directNodes), {
         childConcurrency: opts.childConcurrency,
         extraBinPaths,
+        extendNodePath: opts.extendNodePath,
         extraEnv,
         lockfileDir,
         optional: opts.include.optionalDependencies,
@@ -367,7 +370,7 @@ export default async (opts: HeadlessOptions) => {
       virtualStoreDir,
     })
 
-    await linkAllBins(graph, { optional: opts.include.optionalDependencies, warn })
+    await linkAllBins(graph, { extendNodePath: opts.extendNodePath, optional: opts.include.optionalDependencies, warn })
 
     if ((currentLockfile != null) && !equals(importerIds.sort(), Object.keys(filteredLockfile.importers).sort())) {
       Object.assign(filteredLockfile.packages, currentLockfile.packages)
@@ -399,7 +402,7 @@ export default async (opts: HeadlessOptions) => {
 
       await Promise.all(opts.projects.map(async (project) => {
         if (opts.publicHoistPattern?.length && path.relative(opts.lockfileDir, project.rootDir) === '') {
-          await linkBinsOfImporter(project)
+          await linkBinsOfImporter(project, { extendNodePath: opts.extendNodePath })
         } else {
           const directPkgDirs = Object.values(directDependenciesByImporterId[project.id])
           await linkBinsOfPackages(
@@ -446,11 +449,15 @@ async function linkBinsOfImporter (
     manifest: ProjectManifest
     modulesDir: string
     rootDir: string
+  },
+  opts: {
+    extendNodePath?: boolean
   }
 ) {
   const warn = (message: string) => logger.info({ message, prefix: rootDir })
   return linkBins(modulesDir, binsDir, {
     allowExoticManifests: true,
+    extendNodePath: opts.extendNodePath,
     projectManifest: manifest,
     warn,
   })
@@ -801,6 +808,7 @@ async function linkAllPkgs (
 async function linkAllBins (
   depGraph: DependenciesGraph,
   opts: {
+    extendNodePath?: boolean
     optional: boolean
     warn: (message: string) => void
   }
@@ -822,7 +830,7 @@ async function linkAllBins (
         const pkgSnapshots = props<string, DependenciesGraphNode>(Object.values(childrenToLink), depGraph)
 
         if (pkgSnapshots.includes(undefined as any)) { // eslint-disable-line
-          await linkBins(depNode.modules, binPath, { warn: opts.warn })
+          await linkBins(depNode.modules, binPath, { extendNodePath: opts.extendNodePath, warn: opts.warn })
         } else {
           const pkgs = await Promise.all(
             pkgSnapshots
@@ -833,13 +841,13 @@ async function linkAllBins (
               }))
           )
 
-          await linkBinsOfPackages(pkgs, binPath, { warn: opts.warn })
+          await linkBinsOfPackages(pkgs, binPath, { extendNodePath: opts.extendNodePath, warn: opts.warn })
         }
 
         // link also the bundled dependencies` bins
         if (depNode.hasBundledDependencies) {
           const bundledModules = path.join(depNode.dir, 'node_modules')
-          await linkBins(bundledModules, binPath, { warn: opts.warn })
+          await linkBins(bundledModules, binPath, { extendNodePath: opts.extendNodePath, warn: opts.warn })
         }
       }))
   )

packages/hoist/src/index.ts

@@ -16,6 +16,7 @@ const hoistLogger = logger('hoist')
 
 export default async function hoistByLockfile (
   opts: {
+    extendNodePath?: boolean
     lockfile: Lockfile
     lockfileDir: string
     privateHoistPattern: string[]
@@ -65,7 +66,7 @@ export default async function hoistByLockfile (
   // the bins of the project's direct dependencies.
   // This is possible because the publicly hoisted modules
   // are in the same directory as the regular dependencies.
-  await linkAllBins(opts.privateHoistedModulesDir)
+  await linkAllBins(opts.privateHoistedModulesDir, { extendNodePath: opts.extendNodePath })
 
   return hoistedDependencies
 }
@@ -85,14 +86,14 @@ function createGetAliasHoistType (
   }
 }
 
-async function linkAllBins (modulesDir: string) {
+async function linkAllBins (modulesDir: string, opts: { extendNodePath?: boolean }) {
   const bin = path.join(modulesDir, '.bin')
   const warn: WarnFunction = (message, code) => {
     if (code === 'BINARIES_CONFLICT') return
     logger.info({ message, prefix: path.join(modulesDir, '../..') })
   }
   try {
-    await linkBins(modulesDir, bin, { allowExoticManifests: true, warn })
+    await linkBins(modulesDir, bin, { allowExoticManifests: true, extendNodePath: opts.extendNodePath, warn })
   } catch (err) {
     // Some packages generate their commands with lifecycle hooks.
     // At this stage, such commands are not generated yet.

packages/link-bins/src/index.ts

@@ -33,6 +33,7 @@ export default async (
   binsDir: string,
   opts: {
     allowExoticManifests?: boolean
+    extendNodePath?: boolean
     nodeExecPathByAlias?: Record<string, string>
     projectManifest?: ProjectManifest
     warn: WarnFunction
@@ -87,6 +88,7 @@ export async function linkBinsOfPackages (
   }>,
   binsTarget: string,
   opts: {
+    extendNodePath?: boolean
     warn: WarnFunction
   }
 ): Promise<string[]> {
@@ -114,6 +116,7 @@ async function linkBins (
   allCmds: CommandInfo[],
   binsDir: string,
   opts: {
+    extendNodePath?: boolean
     warn: WarnFunction
   }
 ): Promise<string[]> {
@@ -123,7 +126,7 @@ async function linkBins (
 
   const [cmdsWithOwnName, cmdsWithOtherNames] = partition(({ ownName }) => ownName, allCmds)
 
-  const results1 = await pSettle(cmdsWithOwnName.map(async (cmd) => linkBin(cmd, binsDir)))
+  const results1 = await pSettle(cmdsWithOwnName.map(async (cmd) => linkBin(cmd, binsDir, opts)))
 
   const usedNames = fromPairs(cmdsWithOwnName.map((cmd) => [cmd.name, cmd.name] as KeyValuePair<string, string>))
   const results2 = await pSettle(cmdsWithOtherNames.map(async (cmd) => {
@@ -132,7 +135,7 @@ async function linkBins (
       return Promise.resolve(undefined)
     }
     usedNames[cmd.name] = cmd.pkgName
-    return linkBin(cmd, binsDir)
+    return linkBin(cmd, binsDir, opts)
   }))
 
   // We want to create all commands that we can create before throwing an exception
@@ -190,16 +193,19 @@ async function getPackageBinsFromManifest (manifest: DependencyManifest, pkgDir:
   }))
 }
 
-async function linkBin (cmd: CommandInfo, binsDir: string) {
+async function linkBin (cmd: CommandInfo, binsDir: string, opts?: { extendNodePath?: boolean }) {
   const externalBinPath = path.join(binsDir, cmd.name)
 
   if (EXECUTABLE_SHEBANG_SUPPORTED) {
     await fs.chmod(cmd.path, 0o755)
   }
-  let nodePath = await getBinNodePaths(cmd.path)
-  const binsParentDir = path.dirname(binsDir)
-  if (path.relative(cmd.path, binsParentDir) !== '') {
-    nodePath = union(nodePath, await getBinNodePaths(binsParentDir))
+  let nodePath: string[] | undefined
+  if (opts?.extendNodePath !== false) {
+    nodePath = await getBinNodePaths(cmd.path)
+    const binsParentDir = path.dirname(binsDir)
+    if (path.relative(cmd.path, binsParentDir) !== '') {
+      nodePath = union(nodePath, await getBinNodePaths(binsParentDir))
+    }
   }
   return cmdShim(cmd.path, externalBinPath, {
     createPwshFile: cmd.makePowerShellShim,

packages/supi/src/install/extendInstallOptions.ts

@@ -17,6 +17,7 @@ export interface StrictInstallOptions {
   frozenLockfile: boolean
   frozenLockfileIfExists: boolean
   enablePnp: boolean
+  extendNodePath: boolean
   extraBinPaths: string[]
   useLockfile: boolean
   linkWorkspacePackagesDepth: number
@@ -98,6 +99,7 @@ const defaults = async (opts: InstallOptions) => {
     depth: 0,
     enablePnp: false,
     engineStrict: false,
+    extendNodePath: true,
     force: false,
     forceSharedLockfile: false,
     frozenLockfile: false,

packages/supi/src/install/index.ts

@@ -247,6 +247,7 @@ export async function mutateModules (
             currentLockfile: ctx.currentLockfile,
             enablePnp: opts.enablePnp,
             engineStrict: opts.engineStrict,
+            extendNodePath: opts.extendNodePath,
             extraBinPaths: opts.extraBinPaths,
             force: opts.force,
             hoistedDependencies: ctx.hoistedDependencies,
@@ -807,6 +808,7 @@ const _installInContext: InstallFunction = async (projects, ctx, opts) => {
         currentLockfile: ctx.currentLockfile,
         dependenciesByProjectId,
         force: opts.force,
+        extendNodePath: opts.extendNodePath,
         hoistedDependencies: ctx.hoistedDependencies,
         hoistedModulesDir: ctx.hoistedModulesDir,
         hoistPattern: ctx.hoistPattern,
@@ -865,6 +867,7 @@ const _installInContext: InstallFunction = async (projects, ctx, opts) => {
       await buildModules(dependenciesGraph, rootNodes, {
         childConcurrency: opts.childConcurrency,
         depsToBuild: new Set(result.newDepPaths),
+        extendNodePath: opts.extendNodePath,
         extraBinPaths: ctx.extraBinPaths,
         extraEnv,
         lockfileDir: ctx.lockfileDir,
@@ -901,6 +904,7 @@ const _installInContext: InstallFunction = async (projects, ctx, opts) => {
           }, {})
         linkedPackages = await linkBins(project.modulesDir, project.binsDir, {
           allowExoticManifests: true,
+          extendNodePath: opts.extendNodePath,
           projectManifest: project.manifest,
           nodeExecPathByAlias,
           warn: binWarn.bind(null, project.rootDir),
@@ -931,7 +935,7 @@ const _installInContext: InstallFunction = async (projects, ctx, opts) => {
           )
             .filter(({ manifest }) => manifest != null) as Array<{ location: string, manifest: DependencyManifest }>,
           project.binsDir,
-          { warn: binWarn.bind(null, project.rootDir) }
+          { extendNodePath: opts.extendNodePath, warn: binWarn.bind(null, project.rootDir) }
         )
       }
       const projectToInstall = projects[index]

packages/supi/src/install/link.ts

@@ -46,6 +46,7 @@ export default async function linkPackages (
       [id: string]: {[alias: string]: string}
     }
     force: boolean
+    extendNodePath?: boolean
     hoistedDependencies: HoistedDependencies
     hoistedModulesDir: string
     hoistPattern?: string[]
@@ -246,6 +247,7 @@ export default async function linkPackages (
       packages: omit(Array.from(opts.skipped), currentLockfile.packages),
     }
     newHoistedDependencies = await hoist({
+      extendNodePath: opts.extendNodePath,
       lockfile: hoistLockfile,
       lockfileDir: opts.lockfileDir,
       privateHoistedModulesDir: opts.hoistedModulesDir,

packages/supi/test/install/misc.ts

@@ -557,6 +557,27 @@ test('bin specified in the directories property linked to .bin folder', async ()
   await project.isExecutable('.bin/pkg-with-directories-bin')
 })
 
+test('command shim is created withoud NODE_PATH', async () => {
+  const project = prepareEmpty()
+
+  const manifest = await addDependenciesToPackage({}, ['rimraf@2.5.1'], await testDefaults({ fastUnpack: false, extendNodePath: false }))
+
+  {
+    await project.isExecutable('.bin/rimraf')
+    const cmdContent = await fs.readFile('node_modules/.bin/rimraf')
+    expect(cmdContent).not.toContain('NODE_PATH')
+  }
+
+  await rimraf('node_modules')
+  await install(manifest, await testDefaults({ extendNodePath: true, fastUnpack: false, frozenLockfile: true }))
+
+  {
+    await project.isExecutable('.bin/rimraf')
+    const cmdContent = await fs.readFile('node_modules/.bin/rimraf')
+    expect(cmdContent).not.toContain('NODE_PATH')
+  }
+})
+
 testOnNonWindows('building native addons', async () => {
   const project = prepareEmpty()