Skip to content

Commit aaeb2f1

Browse files
fix(arborist): expose store node_modules via NODE_PATH for linked-strategy install scripts (#9578)
Backport of #9551 to `release/v11`. Co-authored-by: Manzoor Wani <manzoorwani.jk@gmail.com>
1 parent 05b6f0f commit aaeb2f1

2 files changed

Lines changed: 80 additions & 2 deletions

File tree

‎workspaces/arborist/lib/arborist/rebuild.js‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ const { depth: dfwalk } = require('treeverse')
1111
const { isNodeGypPackage, defaultGypInstallScript } = require('@npmcli/node-gyp')
1212
const { promiseRetry } = require('@gar/promise-retry')
1313
const { log, time } = require('proc-log')
14-
const { resolve } = require('node:path')
14+
const { resolve, delimiter } = require('node:path')
1515
const { isScriptAllowed } = require('../script-allowed.js')
1616

1717
const boolEnv = b => b ? '1' : ''
@@ -301,6 +301,7 @@ module.exports = cls => class Builder extends cls {
301301
await promiseCallLimit(queue.map(node => async () => {
302302
const {
303303
path,
304+
name,
304305
integrity,
305306
resolved,
306307
optional,
@@ -309,6 +310,7 @@ module.exports = cls => class Builder extends cls {
309310
devOptional,
310311
package: pkg,
311312
location,
313+
isInStore,
312314
} = node.target
313315

314316
// skip any that we know we'll be deleting
@@ -330,6 +332,12 @@ module.exports = cls => class Builder extends cls {
330332
npm_package_dev_optional:
331333
boolEnv(devOptional && !dev && !optional),
332334
}
335+
// In the linked strategy a store package's dependencies are symlinked siblings in its store node_modules.
336+
// A separate bin invoked by the script (e.g. napi-postinstall) resolves modules from its own realpath in the store and cannot see those deps, so expose them via NODE_PATH.
337+
if (isInStore) {
338+
const storeNodeModules = resolve(path, ...name.split('/').map(() => '..'))
339+
env.NODE_PATH = [storeNodeModules, process.env.NODE_PATH].filter(Boolean).join(delimiter)
340+
}
333341
const runOpts = {
334342
event,
335343
path,

‎workspaces/arborist/test/arborist/reify.js‎

Lines changed: 71 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
const { join, resolve, basename } = require('node:path')
1+
const { join, resolve, basename, delimiter } = require('node:path')
22
const t = require('tap')
33
const runScript = require('@npmcli/run-script')
44
const localeCompare = require('@isaacs/string-locale-compare')('en')
@@ -4352,6 +4352,76 @@ t.test('install strategy linked', async (t) => {
43524352
})
43534353
})
43544354

4355+
t.test('linked strategy exposes store node_modules via NODE_PATH for lifecycle scripts', async t => {
4356+
// Regression for #9549. In the linked strategy a store package's deps are symlinked siblings in its store node_modules.
4357+
// A separate bin invoked by the script (e.g. napi-postinstall) resolves modules from its own store realpath and cannot see them, so npm exposes them via NODE_PATH.
4358+
const Arborist = require('../../lib/index.js')
4359+
const pacote = require('pacote')
4360+
4361+
const testdir = t.testdir({
4362+
src: {
4363+
'package.json': JSON.stringify({
4364+
name: 'has-postinstall',
4365+
version: '1.0.0',
4366+
scripts: { postinstall: 'node -e ""' },
4367+
}),
4368+
},
4369+
project: {
4370+
'package.json': JSON.stringify({
4371+
name: 'myproject',
4372+
version: '1.0.0',
4373+
dependencies: { 'has-postinstall': '1.0.0' },
4374+
}),
4375+
},
4376+
})
4377+
4378+
const tgz = await pacote.tarball(resolve(testdir, 'src'), { Arborist })
4379+
4380+
const packument = JSON.stringify({
4381+
_id: 'has-postinstall',
4382+
name: 'has-postinstall',
4383+
'dist-tags': { latest: '1.0.0' },
4384+
versions: {
4385+
'1.0.0': {
4386+
name: 'has-postinstall',
4387+
version: '1.0.0',
4388+
hasInstallScript: true,
4389+
scripts: { postinstall: 'node -e ""' },
4390+
dist: {
4391+
tarball: 'https://registry.npmjs.org/has-postinstall/-/has-postinstall-1.0.0.tgz',
4392+
},
4393+
},
4394+
},
4395+
})
4396+
4397+
tnock(t, 'https://registry.npmjs.org')
4398+
.get('/has-postinstall')
4399+
.reply(200, packument)
4400+
4401+
tnock(t, 'https://registry.npmjs.org')
4402+
.get('/has-postinstall/-/has-postinstall-1.0.0.tgz')
4403+
.reply(200, tgz)
4404+
4405+
const path = resolve(testdir, 'project')
4406+
const arb = new Arborist({
4407+
path,
4408+
registry: 'https://registry.npmjs.org',
4409+
cache: resolve(testdir, 'cache'),
4410+
installStrategy: 'linked',
4411+
dangerouslyAllowAllScripts: true,
4412+
})
4413+
await arb.reify()
4414+
4415+
const run = [...arb.scriptsRun]
4416+
.find(s => s.pkg.name === 'has-postinstall' && s.event === 'postinstall')
4417+
t.ok(run, 'postinstall ran for the store package')
4418+
t.match(run.path, /[\\/]\.store[\\/]/, 'script ran on the store entry')
4419+
// Assert the leading entry: the fix prepends the store node_modules to any pre-existing NODE_PATH (e.g. the coverage harness on Windows CI).
4420+
const [firstNodePath] = run.env.NODE_PATH.split(delimiter)
4421+
t.equal(firstNodePath, resolve(run.path, '..'),
4422+
'NODE_PATH leads with the store node_modules holding the package deps')
4423+
})
4424+
43554425
t.test('workspace installs retain existing versions with newer package specs', async t => {
43564426
const path = t.testdir({
43574427
'package.json': JSON.stringify({

0 commit comments

Comments
 (0)