RFC: Add ability to skip script hooks

[lumaxis]

See content.

[isaacs]

It turns out we're already running the specified script for npm run-script (and thus for test, start, etc.) when --ignore-scripts is set. (v6 does not do this, so this is an overlooked breaking change, now turned into a feature by this RFC ;)

The whole diff to do this in v7 is:

diff --git a/lib/run-script.js b/lib/run-script.js
index 270a91ab7..733534421 100644
--- a/lib/run-script.js
+++ b/lib/run-script.js
@@ -80,11 +80,13 @@ const runScript = async (args) => {
 
   // positional args only added to the main event, not pre/post
   const events = [[event, args]]
-  if (scripts[`pre${event}`]) {
-    events.unshift([`pre${event}`, []])
-  }
-  if (scripts[`post${event}`]) {
-    events.push([`post${event}`, []])
+  if (!npm.flatOptions.ignoreScripts) {
+    if (scripts[`pre${event}`]) {
+      events.unshift([`pre${event}`, []])
+    }
+    if (scripts[`post${event}`]) {
+      events.push([`post${event}`, []])
+    }
   }
 
   const opts = {

[ljharb]

ooh, so to confirm, npm test --ignore-scripts in npm 7 will no longer run nothing, but instead will run test without pretest/posttest??

[isaacs]

Well... in the current implementation on the beta branch, it'll run test with pretest/posttest. If we accept this RFC, it'll run without pretest/posttest. If we do a different thing, it'll do a different thing :)

Whether the current implementation is a feature or a bug is something we'll discuss wednesday in the call.

[isaacs]

Personally, I'm of the opinion that npm test should always run your test, even if you have --ignore-scripts set. Because that's the command you're running, it's a bit silly to have a flag that says "don't do the thing I'm telling you specifically to do". But I think it's reasonable to assume in that case that it should not run other scripts that ride along with that command, sort of in the spirit of --ignore-scripts, which is what this RFC proposes.

[ljharb]

I agree - but it shouldnt run the pre and post scripts, ie, I’m strongly in favor of this rfc.

The behavior in npm <= 6 is bizarre in that it runs nothing :-)

[ruyadorno]

Related: #158

[askirmas]

npm run $script --ignore-scripts looks too weird. It should be based on word hooks or lifecycle like --without-hooks or --no-lifecycle. Also can be leading --omit-, --skip-, --dry-.

--ignore-scripts omits dependency's(1) package(2) lifecycle but discussed option skips lifecycle of self(1) script(2). It will be puzzling name coincide and not option reusage.

Besides, is it ok that install and install {package} looks like completely different scripts? Considering

/* ./package.json */
{
  "scripts": {
    "preinstall": "echo 'main preinstall'",
    "postinstall": "echo 'main postinstall'"
  },
  "dependencies": {
    "sub": "file:sub"
  }
}

/* ./sub/package.json */
  "name": "sub",
  "version": "1.0.0",
  "scripts": {
    "preinstall": "echo 'sub preinstall'",
    "postinstall": "echo 'sub postinstall'"
  },

Output:

$ npm install
# main preinstall
# main postinstall
$ npm install ./sub
# sub preinstall
# sub postinstall

[isaacs]

We discussed this on the call last week and consensus was that adding another term for this (even the term lifecycle which is used internally) is not ideal for UX.

Going to ratify this proposal as-is, and get the implementation in one of the next few v7 beta releases for play-testing.

[lumaxis]

@isaacs Do we merge this or what's the process for the PR?