diff --git a/docs/lib/content/using-npm/scripts.md b/docs/lib/content/using-npm/scripts.md index 380c4aa49e36f..ffdb35f481da9 100644 --- a/docs/lib/content/using-npm/scripts.md +++ b/docs/lib/content/using-npm/scripts.md @@ -111,7 +111,7 @@ It is run AFTER the changes have been applied and the `package.json` and `packag #### [`npm ci`](/commands/npm-ci) -* `preinstall` +* `preinstall` (before dependencies are installed) * `install` * `postinstall` * `prepublish` @@ -119,8 +119,9 @@ It is run AFTER the changes have been applied and the `package.json` and `packag * `prepare` * `postprepare` -These all run after the actual installation of modules into - `node_modules`, in order, with no internal actions happening in between +`preinstall` runs before any dependencies are fetched or unpacked into +`node_modules`. The remaining scripts run after the installation of modules +into `node_modules`, in order, with no internal actions happening in between. #### [`npm diff`](/commands/npm-diff) @@ -128,9 +129,9 @@ These all run after the actual installation of modules into #### [`npm install`](/commands/npm-install) -These also run when you run `npm install -g ` +These run on a bare `npm install` in a local project (no package arguments). -* `preinstall` +* `preinstall` (before dependencies are installed) * `install` * `postinstall` * `prepublish` @@ -138,6 +139,11 @@ These also run when you run `npm install -g ` * `prepare` * `postprepare` +`preinstall` runs before any dependencies are fetched or unpacked into +`node_modules`, so scripts can prepare the environment (for example, setting +up authentication for a private registry) before resolution begins. The +remaining scripts run after installation has completed. + If there is a `binding.gyp` file in the root of your package and you haven't defined your own `install` or `preinstall` scripts, npm will default the `install` command to compile using node-gyp via `node-gyp rebuild` These are run from the scripts of `` diff --git a/lib/commands/ci.js b/lib/commands/ci.js index 5971f921cb71d..34dd4f118be46 100644 --- a/lib/commands/ci.js +++ b/lib/commands/ci.js @@ -96,12 +96,24 @@ class CI extends ArboristWorkspaceCmd { }) } + // Root lifecycle scripts for `npm ci` mirror those run by `npm install`. `preinstall` runs *before* reify so that scripts can bootstrap the environment (e.g. private-registry auth) before any dependency is fetched or unpacked. The remaining scripts run after reify as they did before. + const scriptShell = this.npm.config.get('script-shell') || undefined + const runRootScript = (event) => runScript({ + path: where, + args: [], + scriptShell, + stdio: 'inherit', + event, + }) + + if (!ignoreScripts) { + await runRootScript('preinstall') + } + await arb.reify(opts) - // run the same set of scripts that `npm install` runs. if (!ignoreScripts) { - const scripts = [ - 'preinstall', + const postReifyScripts = [ 'install', 'postinstall', 'prepublish', // XXX should we remove this finally?? @@ -109,15 +121,8 @@ class CI extends ArboristWorkspaceCmd { 'prepare', 'postprepare', ] - const scriptShell = this.npm.config.get('script-shell') || undefined - for (const event of scripts) { - await runScript({ - path: where, - args: [], - scriptShell, - stdio: 'inherit', - event, - }) + for (const event of postReifyScripts) { + await runRootScript(event) } } await reifyFinish(this.npm, arb) diff --git a/lib/commands/install.js b/lib/commands/install.js index 5970fddfdfe4f..cadc2712fae5a 100644 --- a/lib/commands/install.js +++ b/lib/commands/install.js @@ -142,12 +142,26 @@ class Install extends ArboristWorkspaceCmd { add: args, workspaces: this.workspaceNames, } + + // Root lifecycle scripts only run for a bare `npm install` in a local project. `preinstall` runs *before* Arborist touches the filesystem so that scripts can bootstrap the environment (e.g. set up private-registry auth, generate files consumed during resolution) before dependencies are fetched or unpacked. The remaining scripts run after reify as they did before. + const runRootLifecycle = !args.length && !isGlobalInstall && !ignoreScripts + const runRootScript = (event) => runScript({ + path: where, + args: [], + scriptShell, + stdio: 'inherit', + event, + }) + + if (runRootLifecycle) { + await runRootScript('preinstall') + } + const arb = new Arborist(opts) await arb.reify(opts) - if (!args.length && !isGlobalInstall && !ignoreScripts) { - const scripts = [ - 'preinstall', + if (runRootLifecycle) { + const postReifyScripts = [ 'install', 'postinstall', 'prepublish', // XXX(npm9) should we remove this finally?? @@ -155,14 +169,8 @@ class Install extends ArboristWorkspaceCmd { 'prepare', 'postprepare', ] - for (const event of scripts) { - await runScript({ - path: where, - args: [], - scriptShell, - stdio: 'inherit', - event, - }) + for (const event of postReifyScripts) { + await runRootScript(event) } } await reifyFinish(this.npm, arb) diff --git a/test/lib/commands/ci.js b/test/lib/commands/ci.js index 733b46a828133..468569add7e2a 100644 --- a/test/lib/commands/ci.js +++ b/test/lib/commands/ci.js @@ -182,6 +182,46 @@ t.test('lifecycle scripts', async t => { ], 'runs appropriate scripts, in order') }) +// Regression test: `npm ci` must run root `preinstall` before reify populates node_modules, matching `npm install` behavior. +t.test('preinstall runs before reify for npm ci', async t => { + const events = [] + const { npm, registry } = await loadMockNpm(t, { + prefixDir: { + abbrev: abbrev, + 'package.json': JSON.stringify({ + ...packageJson, + scripts: { + preinstall: 'echo preinstall', + postinstall: 'echo postinstall', + }, + }), + 'package-lock.json': JSON.stringify(packageLock), + }, + mocks: { + '@npmcli/run-script': (opts) => { + if (opts.path === npm.prefix) { + const abbrevPkg = path.join(npm.prefix, 'node_modules', 'abbrev', 'package.json') + events.push({ event: opts.event, depInstalled: fs.existsSync(abbrevPkg) }) + } + }, + }, + }) + const manifest = registry.manifest({ name: 'abbrev' }) + await registry.tarball({ + manifest: manifest.versions['1.0.0'], + tarball: path.join(npm.prefix, 'abbrev'), + }) + registry.nock.post('/-/npm/v1/security/advisories/bulk').reply(200, {}) + await npm.exec('ci', []) + + const pre = events.find(e => e.event === 'preinstall') + const post = events.find(e => e.event === 'postinstall') + t.ok(pre, 'preinstall ran') + t.ok(post, 'postinstall ran') + t.equal(pre.depInstalled, false, 'preinstall runs before dependencies are installed') + t.equal(post.depInstalled, true, 'postinstall runs after dependencies are installed') +}) + t.test('should throw if package-lock.json is missing', async t => { const { npm } = await loadMockNpm(t, { prefixDir: { diff --git a/test/lib/commands/install.js b/test/lib/commands/install.js index 584690b68b5c6..1f30c1be76136 100644 --- a/test/lib/commands/install.js +++ b/test/lib/commands/install.js @@ -101,6 +101,82 @@ t.test('exec commands', async t => { t.strictSame(lifecycleScripts, runOrder, 'all script ran in the correct order') }) + // Regression test: root `preinstall` must run before any dependency is fetched/unpacked, while `install` and `postinstall` run after reify has populated node_modules. + await t.test('preinstall runs before reify, post-reify scripts run after', async t => { + const events = [] + const { npm, registry } = await loadMockNpm(t, { + config: { audit: false }, + prefixDir: { + 'package.json': JSON.stringify({ + ...packageJson, + scripts: { + preinstall: 'echo preinstall', + install: 'echo install', + postinstall: 'echo postinstall', + }, + }), + abbrev, + }, + mocks: { + '@npmcli/run-script': async (opts) => { + // Only record scripts targeted at the project root, not any that arborist may run for dependencies during reify. + if (opts.path === npm.prefix) { + const abbrevPkg = path.join(npm.prefix, 'node_modules', 'abbrev', 'package.json') + events.push({ event: opts.event, depInstalled: fs.existsSync(abbrevPkg) }) + } + }, + }, + }) + const manifest = registry.manifest({ name: 'abbrev' }) + await registry.package({ manifest }) + await registry.tarball({ + manifest: manifest.versions['1.0.0'], + tarball: path.join(npm.prefix, 'abbrev'), + }) + + await npm.exec('install') + + const pre = events.find(e => e.event === 'preinstall') + const post = events.find(e => e.event === 'postinstall') + t.ok(pre, 'preinstall ran') + t.ok(post, 'postinstall ran') + t.equal(pre.depInstalled, false, 'preinstall runs before dependencies are installed') + t.equal(post.depInstalled, true, 'postinstall runs after dependencies are installed') + }) + + await t.test('without args, --ignore-scripts skips preinstall entirely', async t => { + const events = [] + const { npm, registry } = await loadMockNpm(t, { + config: { audit: false, 'ignore-scripts': true }, + prefixDir: { + 'package.json': JSON.stringify({ + ...packageJson, + scripts: { + preinstall: 'echo preinstall', + postinstall: 'echo postinstall', + }, + }), + abbrev, + }, + mocks: { + '@npmcli/run-script': async (opts) => { + if (opts.path === npm.prefix) { + events.push(opts.event) + } + }, + }, + }) + const manifest = registry.manifest({ name: 'abbrev' }) + await registry.package({ manifest }) + await registry.tarball({ + manifest: manifest.versions['1.0.0'], + tarball: path.join(npm.prefix, 'abbrev'), + }) + + await npm.exec('install') + t.strictSame(events, [], 'no root lifecycle scripts run when --ignore-scripts is set') + }) + await t.test('should ignore scripts with --ignore-scripts', async t => { const { npm, registry } = await loadMockNpm(t, { config: {