Skip to content

Commit

Permalink
fix run unchanged links lifecycle scripts
Browse files Browse the repository at this point in the history
Users expect workspaces (or regular linked deps) to run their prepare
lifecycle script on every reify, since it's often used as some form of
compilation step and should run even though that node hasn't changed.

Fixes: npm/cli#2900
  • Loading branch information
ruyadorno committed Jun 25, 2021
1 parent ab0d8f2 commit ae6c2af
Show file tree
Hide file tree
Showing 8 changed files with 175 additions and 2 deletions.
4 changes: 2 additions & 2 deletions lib/arborist/rebuild.js
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ module.exports = cls => class Builder extends cls {
const queue = [...set].sort(sortNodes)

for (const node of queue) {
const { package: { bin, scripts = {} } } = node
const { package: { bin, scripts = {} } } = node.target || node
const { preinstall, install, postinstall, prepare } = scripts
const tests = { bin, preinstall, install, postinstall, prepare }
for (const [key, has] of Object.entries(tests)) {
Expand Down Expand Up @@ -202,7 +202,7 @@ module.exports = cls => class Builder extends cls {
!(meta.originalLockfileVersion >= 2)
}

const { package: pkg, hasInstallScript } = node
const { package: pkg, hasInstallScript } = node.target || node
const { gypfile, bin, scripts = {} } = pkg

const { preinstall, install, postinstall, prepare } = scripts
Expand Down
7 changes: 7 additions & 0 deletions lib/arborist/reify.js
Original file line number Diff line number Diff line change
Expand Up @@ -887,6 +887,13 @@ module.exports = cls => class Reifier extends cls {
filter: diff => diff.action === 'ADD' || diff.action === 'CHANGE',
})

// pick up link nodes from the unchanged list as we want to run their
// scripts in every install despite of having a diff status change
for (const node of this.diff.unchanged) {
if (node.isLink)
nodes.push(node)
}

return this.rebuild({ nodes, handleOptionalFailure: true })
.then(() => process.emit('timeEnd', 'reify:build'))
}
Expand Down
47 changes: 47 additions & 0 deletions tap-snapshots/test/arborist/reify.js.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -31821,6 +31821,53 @@ ArboristNode {
}
`

exports[`test/arborist/reify.js TAP running lifecycle scripts of unchanged link nodes on reify > result 1`] = `
ArboristNode {
"children": Map {
"a" => ArboristLink {
"edgesIn": Set {
EdgeIn {
"from": "",
"name": "a",
"spec": "file:./a",
"type": "prod",
},
},
"location": "node_modules/a",
"name": "a",
"path": "{CWD}/test/arborist/tap-testdir-reify-running-lifecycle-scripts-of-unchanged-link-nodes-on-reify/node_modules/a",
"realpath": "{CWD}/test/arborist/tap-testdir-reify-running-lifecycle-scripts-of-unchanged-link-nodes-on-reify/a",
"resolved": "file:../a",
"target": ArboristNode {
"location": "a",
},
"version": "1.0.0",
},
},
"edgesOut": Map {
"a" => EdgeOut {
"name": "a",
"spec": "file:./a",
"to": "node_modules/a",
"type": "prod",
},
},
"fsChildren": Set {
ArboristNode {
"location": "a",
"name": "a",
"path": "{CWD}/test/arborist/tap-testdir-reify-running-lifecycle-scripts-of-unchanged-link-nodes-on-reify/a",
"version": "1.0.0",
},
},
"location": "",
"name": "tap-testdir-reify-running-lifecycle-scripts-of-unchanged-link-nodes-on-reify",
"packageName": "link-dep-lifecycle-scripts",
"path": "{CWD}/test/arborist/tap-testdir-reify-running-lifecycle-scripts-of-unchanged-link-nodes-on-reify",
"version": "1.0.0",
}
`

exports[`test/arborist/reify.js TAP save complete lockfile on update-all > should have abbrev 1.0.4 1`] = `
{
"name": "save-package-lock-after-update-test",
Expand Down
8 changes: 8 additions & 0 deletions test/arborist/reify.js
Original file line number Diff line number Diff line change
Expand Up @@ -1723,6 +1723,14 @@ console.log('ok 1 - this is fine')
}), 'test result')
})

t.test('running lifecycle scripts of unchanged link nodes on reify', async t => {
const path = fixture(t, 'link-dep-lifecycle-scripts')
t.matchSnapshot(await printReified(path), 'result')

t.ok(fs.lstatSync(resolve(path, 'a/a-prepare')).isFile(),
'run prepare lifecycle scripts')
})

t.test('save-prod, with optional', async t => {
const path = t.testdir({
'package.json': JSON.stringify({
Expand Down
7 changes: 7 additions & 0 deletions test/fixtures/link-dep-lifecycle-scripts/a/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"name": "a",
"version": "1.0.0",
"scripts": {
"prepare": "node -e \"require('fs').writeFileSync(require('path').resolve('a-prepare'), '')\""
}
}
26 changes: 26 additions & 0 deletions test/fixtures/link-dep-lifecycle-scripts/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions test/fixtures/link-dep-lifecycle-scripts/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"name": "link-dep-lifecycle-scripts",
"version": "1.0.0",
"dependencies": {
"a": "file:./a"
}
}
71 changes: 71 additions & 0 deletions test/fixtures/reify-cases/link-dep-lifecycle-scripts.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
// generated from test/fixtures/link-dep-lifecycle-scripts
module.exports = t => {
const path = t.testdir({
"a": {
"package.json": JSON.stringify({
"name": "a",
"version": "1.0.0",
"scripts": {
"prepare": "node -e \"require('fs').writeFileSync(require('path').resolve('a-prepare'), '')\""
}
})
},
"node_modules": {
".package-lock.json": JSON.stringify({
"name": "link-dep-lifecycle-scripts",
"version": "1.0.0",
"lockfileVersion": 2,
"requires": true,
"packages": {
"a": {
"version": "1.0.0"
},
"node_modules/a": {
"resolved": "a",
"link": true
}
}
}),
"a": t.fixture('symlink', "../a")
},
"package-lock.json": JSON.stringify({
"name": "link-dep-lifecycle-scripts",
"version": "1.0.0",
"lockfileVersion": 2,
"requires": true,
"packages": {
"": {
"version": "1.0.0",
"dependencies": {
"a": "file:./a"
}
},
"a": {
"version": "1.0.0"
},
"node_modules/a": {
"resolved": "a",
"link": true
}
},
"dependencies": {
"a": {
"version": "file:a"
}
}
}),
"package.json": JSON.stringify({
"name": "link-dep-lifecycle-scripts",
"version": "1.0.0",
"dependencies": {
"a": "file:./a"
}
})
})
const {utimesSync} = require('fs')
const n = Date.now() + 10000
const {resolve} = require('path')

utimesSync(resolve(path, "node_modules/.package-lock.json"), new Date(n), new Date(n))
return path
}

0 comments on commit ae6c2af

Please sign in to comment.