Skip to content

Commit 880ecb7

Browse files
fix(arborist): skip postinstall on store links in linked strategy (npm#9013)
Continuing the `install-strategy=linked` fixes from npm#8996. While testing on the [Gutenberg monorepo](WordPress/gutenberg#75814), `esbuild` installs fail because its postinstall script runs twice in parallel against the same store directory. ## Summary With `install-strategy=linked`, postinstall scripts run twice for every store package — once for the store entry and once for its symlink. For packages like `esbuild` whose postinstall modifies files in-place (`fs.linkSync` to replace the JS wrapper with a native binary), this race condition corrupts the install. ## Root cause In `rebuild.js`, `#runScripts` destructures `isStoreLink` from `node.target` (the store entry) to decide whether to skip a node. But `isStoreLink` is a property of the link node itself (`node`), not its target. Store entries don't have `isStoreLink`, so it's always `undefined` and the guard never triggers. Both the store entry and the store link run scripts against the same directory in parallel. ## Changes - Fixed the skip condition in `rebuild.js` `#runScripts` to use `node.isLink && node.target?.isInStore` instead of reading `isStoreLink` from `node.target`. This correctly skips store links (symlinks pointing to store entries) while still allowing workspace links and store entries themselves to run scripts. - Added a regression test that verifies postinstall scripts run exactly once for store packages. ## References Fixes npm#9012
1 parent 658b323 commit 880ecb7

2 files changed

Lines changed: 33 additions & 3 deletions

File tree

workspaces/arborist/lib/arborist/rebuild.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -295,12 +295,12 @@ module.exports = cls => class Builder extends cls {
295295
devOptional,
296296
package: pkg,
297297
location,
298-
isStoreLink,
299298
} = node.target
300299

301300
// skip any that we know we'll be deleting
302-
// or storeLinks
303-
if (this[_trashList].has(path) || isStoreLink) {
301+
// or links to store entries (their scripts run on the store
302+
// entry itself, not through the link)
303+
if (this[_trashList].has(path) || (node.isLink && node.target?.isInStore)) {
304304
return
305305
}
306306

workspaces/arborist/test/isolated-mode.js

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1571,6 +1571,36 @@ tap.test('postinstall scripts are run', async t => {
15711571
t.ok(postInstallRanBar)
15721572
})
15731573

1574+
tap.test('postinstall scripts run once for store packages', async t => {
1575+
// Regression test: store links should not cause scripts to run twice.
1576+
// The store entry and its symlink both end up in the build queue, but
1577+
// only the store entry should run scripts.
1578+
const graph = {
1579+
registry: [
1580+
{
1581+
name: 'which',
1582+
version: '1.0.0',
1583+
scripts: {
1584+
postinstall: 'node -e "var c=0;try{c=+fs.readFileSync(\'postinstall-count\',\'utf8\')}catch(e){};fs.writeFileSync(\'postinstall-count\',String(c+1))"',
1585+
},
1586+
},
1587+
],
1588+
root: {
1589+
name: 'foo', version: '1.2.3', dependencies: { which: '1.0.0' },
1590+
},
1591+
}
1592+
1593+
const { dir, registry } = await getRepo(graph)
1594+
1595+
const cache = fs.mkdtempSync(`${getTempDir()}/test-`)
1596+
const arborist = new Arborist({ path: dir, registry, packumentCache: new Map(), cache })
1597+
await arborist.reify({ installStrategy: 'linked' })
1598+
1599+
const whichDir = setupRequire(dir)('which')
1600+
const count = Number(fs.readFileSync(`${whichDir}/postinstall-count`, 'utf8'))
1601+
t.equal(count, 1, 'postinstall ran exactly once')
1602+
})
1603+
15741604
tap.test('bins are installed', async t => {
15751605
// Input of arborist
15761606
const graph = {

0 commit comments

Comments
 (0)