feat: restore extend-node-path option (#5910)

Co-authored-by: Zoltan Kochan <z@kochan.io>

.changeset/khaki-countries-drive.md

@@ -0,0 +1,7 @@
+---
+"@pnpm/get-context": minor
+"@pnpm/config": minor
+"pnpm": minor
+---
+
+When `extend-node-path` is set to `false`, the `NODE_PATH` environment variable is not set in the command shims [#5910](https://github.com/pnpm/pnpm/pull/5910)

config/config/src/index.ts

@@ -43,6 +43,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],
@@ -183,6 +184,7 @@ export async function getConfig (
     bail: true,
     color: 'auto',
     'enable-modules-dir': true,
+    'extend-node-path': true,
     'fetch-retries': 2,
     'fetch-retry-factor': 10,
     'fetch-retry-maxtimeout': 60000,

pkg-manager/core/src/install/extendInstallOptions.ts

@@ -121,6 +121,7 @@ export interface StrictInstallOptions {
   resolveSymlinksInInjectedDirs: boolean
   dedupeDirectDeps: boolean
   useLockfileV6?: boolean
+  extendNodePath: boolean
 }
 
 export type InstallOptions =
@@ -207,6 +208,7 @@ const defaults = async (opts: InstallOptions) => {
     resolveSymlinksInInjectedDirs: false,
     dedupeDirectDeps: false,
     resolvePeersFromWorkspaceRoot: false,
+    extendNodePath: true,
   } as StrictInstallOptions
 }
 

pkg-manager/core/test/install/hoist.ts

@@ -771,3 +771,26 @@ test('only hoist packages which is in the dependencies tree of the selected proj
   const { version: regeneratorVersion } = root.requireModule('.pnpm/node_modules/regenerator-runtime/package.json')
   expect(regeneratorVersion).toBe('0.13.9')
 })
+
+test('should add extra node paths to command shims', async () => {
+  prepareEmpty()
+
+  await addDependenciesToPackage({}, ['@pnpm.e2e/hello-world-js-bin'], await testDefaults({ fastUnpack: false, hoistPattern: '*' }))
+
+  const cmdShim = fs.readFileSync(path.join('node_modules', '.bin', 'hello-world-js-bin'), 'utf8')
+  expect(cmdShim).toContain('node_modules/.pnpm/node_modules')
+})
+
+test('should not add extra node paths to command shims, when extend-node-path is set to false', async () => {
+  prepareEmpty()
+
+  await addDependenciesToPackage({}, ['@pnpm.e2e/hello-world-js-bin'], await testDefaults({
+    fastUnpack: false,
+    extendNodePath: false,
+    hoistPattern: '*',
+  }))
+
+  const cmdShim = fs.readFileSync(path.join('node_modules', '.bin', 'hello-world-js-bin'), 'utf8')
+  console.log(cmdShim)
+  expect(cmdShim).not.toContain('node_modules/.pnpm/node_modules')
+})

pkg-manager/get-context/jest.config.js

@@ -0,0 +1,3 @@
+const config = require('../../jest.config.js');
+
+module.exports = Object.assign({}, config, {});

pkg-manager/get-context/package.json

@@ -12,8 +12,9 @@
     "node": ">=14.6"
   },
   "scripts": {
-    "lint": "eslint src/**/*.ts",
-    "test": "pnpm run compile",
+    "lint": "eslint src/**/*.ts test/**/*.ts",
+    "_test": "jest",
+    "test": "pnpm run compile && pnpm run _test",
     "prepublishOnly": "pnpm run compile",
     "compile": "tsc --build && pnpm run lint --fix"
   },

pkg-manager/get-context/src/index.ts

@@ -74,6 +74,7 @@ export interface GetContextOptions {
   forceSharedLockfile: boolean
   frozenLockfile?: boolean
   extraBinPaths: string[]
+  extendNodePath?: boolean
   lockfileDir: string
   modulesDir?: string
   nodeLinker: 'isolated' | 'hoisted' | 'pnp'
@@ -153,7 +154,7 @@ export async function getContext (
   const hoistPattern = importersContext.currentHoistPattern ?? opts.hoistPattern
   const ctx: PnpmContext = {
     extraBinPaths,
-    extraNodePaths: getExtraNodePaths({ nodeLinker: opts.nodeLinker, hoistPattern, virtualStoreDir }),
+    extraNodePaths: getExtraNodePaths({ extendNodePath: opts.extendNodePath, nodeLinker: opts.nodeLinker, hoistPattern, virtualStoreDir }),
     hoistedDependencies: importersContext.hoistedDependencies,
     hoistedModulesDir,
     hoistPattern,
@@ -371,6 +372,7 @@ export async function getContextForSingleImporter (
     forceNewModules?: boolean
     forceSharedLockfile: boolean
     extraBinPaths: string[]
+    extendNodePath?: boolean
     lockfileDir: string
     nodeLinker: 'isolated' | 'hoisted' | 'pnp'
     modulesDir?: string
@@ -457,7 +459,7 @@ export async function getContextForSingleImporter (
   const hoistPattern = currentHoistPattern ?? opts.hoistPattern
   const ctx: PnpmSingleContext = {
     extraBinPaths,
-    extraNodePaths: getExtraNodePaths({ nodeLinker: opts.nodeLinker, hoistPattern, virtualStoreDir }),
+    extraNodePaths: getExtraNodePaths({ extendNodePath: opts.extendNodePath, nodeLinker: opts.nodeLinker, hoistPattern, virtualStoreDir }),
     hoistedDependencies,
     hoistedModulesDir,
     hoistPattern,
@@ -506,13 +508,14 @@ export async function getContextForSingleImporter (
 }
 
 function getExtraNodePaths (
-  { hoistPattern, nodeLinker, virtualStoreDir }: {
+  { extendNodePath = true, hoistPattern, nodeLinker, virtualStoreDir }: {
+    extendNodePath?: boolean
     hoistPattern?: string[]
     nodeLinker: 'isolated' | 'hoisted' | 'pnp'
     virtualStoreDir: string
   }
 ) {
-  if (nodeLinker === 'isolated' && hoistPattern?.length) {
+  if (extendNodePath && nodeLinker === 'isolated' && hoistPattern?.length) {
     return [path.join(virtualStoreDir, 'node_modules')]
   }
   return []

pkg-manager/get-context/test/index.ts

@@ -0,0 +1,38 @@
+/// <reference path="../../../__typings__/index.d.ts"/>
+import { getContext } from '@pnpm/get-context'
+import path from 'path'
+import { GetContextOptions } from '../src'
+
+const DEFAULT_OPTIONS: GetContextOptions = {
+  allProjects: [],
+  extraBinPaths: [],
+  force: false,
+  forceSharedLockfile: false,
+  lockfileDir: path.join(__dirname, 'lockfile'),
+  nodeLinker: 'isolated',
+  hoistPattern: ['*'],
+  registries: { default: '' },
+  useLockfile: false,
+  include: {
+    dependencies: true,
+    devDependencies: true,
+    optionalDependencies: true,
+  },
+  storeDir: path.join(__dirname, 'store'),
+}
+
+test('getContext - extendNodePath false', async () => {
+  const context = await getContext({
+    ...DEFAULT_OPTIONS,
+    extendNodePath: false,
+  })
+  expect(context.extraNodePaths).toEqual([])
+})
+
+test('getContext - extendNodePath true', async () => {
+  const context = await getContext({
+    ...DEFAULT_OPTIONS,
+    extendNodePath: true,
+  })
+  expect(context.extraNodePaths).toEqual([path.join(context.virtualStoreDir, 'node_modules')])
+})