From b4a03a56b443b2358938efbea0b47d4518c9e491 Mon Sep 17 00:00:00 2001
From: Nick O'Leary <nick.oleary@gmail.com>
Date: Mon, 19 Apr 2021 20:29:30 +0100
Subject: [PATCH] Allow preInstall hook to return false to skip npm install

---
 .../@node-red/registry/lib/externalModules.js | 10 +++++++---
 .../@node-red/registry/lib/installer.js       | 10 +++++++---
 .../registry/lib/externalModules_spec.js      | 19 +++++++++++++++++++
 .../@node-red/registry/lib/installer_spec.js  |  7 +++++--
 4 files changed, 38 insertions(+), 8 deletions(-)

diff --git a/packages/node_modules/@node-red/registry/lib/externalModules.js b/packages/node_modules/@node-red/registry/lib/externalModules.js
index 2bedcce1d..839f63b23 100644
--- a/packages/node_modules/@node-red/registry/lib/externalModules.js
+++ b/packages/node_modules/@node-red/registry/lib/externalModules.js
@@ -196,11 +196,15 @@ async function installModule(moduleDetails) {
         "version": moduleDetails.version,
         "dir": installDir,
     }
-    return hooks.trigger("preInstall", triggerPayload).then(() => {
+    return hooks.trigger("preInstall", triggerPayload).then((result) => {
         // preInstall passed
         // - run install
-        log.trace(NPM_COMMAND + JSON.stringify(args));
-        return exec.run(NPM_COMMAND, args, { cwd: installDir },true)
+        if (result !== false) {
+            log.trace(NPM_COMMAND + JSON.stringify(args));
+            return exec.run(NPM_COMMAND, args, { cwd: installDir },true)
+        } else {
+            log.trace("skipping npm install");
+        }
     }).then(() => {
         return hooks.trigger("postInstall", triggerPayload)
     }).then(() => {
diff --git a/packages/node_modules/@node-red/registry/lib/installer.js b/packages/node_modules/@node-red/registry/lib/installer.js
index ef38b89a2..f501f0d4c 100644
--- a/packages/node_modules/@node-red/registry/lib/installer.js
+++ b/packages/node_modules/@node-red/registry/lib/installer.js
@@ -178,11 +178,15 @@ async function installModule(module,version,url) {
             "isUpgrade": isUpgrade
         }
 
-        return hooks.trigger("preInstall", triggerPayload).then(() => {
+        return hooks.trigger("preInstall", triggerPayload).then((result) => {
             // preInstall passed
             // - run install
-            log.trace(npmCommand + JSON.stringify(args));
-            return exec.run(npmCommand,args,{ cwd: installDir}, true)
+            if (result !== false) {
+                log.trace(npmCommand + JSON.stringify(args));
+                return exec.run(npmCommand,args,{ cwd: installDir}, true)
+            } else {
+                log.trace("skipping npm install");
+            }
         }).then(() => {
             return hooks.trigger("postInstall", triggerPayload)
         }).then(() => {
diff --git a/test/unit/@node-red/registry/lib/externalModules_spec.js b/test/unit/@node-red/registry/lib/externalModules_spec.js
index 99c74786f..f226f45e5 100644
--- a/test/unit/@node-red/registry/lib/externalModules_spec.js
+++ b/test/unit/@node-red/registry/lib/externalModules_spec.js
@@ -123,6 +123,25 @@ describe("externalModules api", function() {
             fs.existsSync(path.join(homeDir,"externalModules")).should.be.true();
         })
 
+        it("skips npm install if preInstall returns false", async function() {
+            externalModules.init({userDir: homeDir});
+            externalModules.register("function", "libs");
+            let receivedPreEvent,receivedPostEvent;
+            hooks.add("preInstall", function(event) { receivedPreEvent = event; return false })
+            hooks.add("postInstall", function(event) { receivedPostEvent = event; })
+
+            await externalModules.checkFlowDependencies([
+                {type: "function", libs:[{module: "foo"}]}
+            ])
+            exec.run.called.should.be.false();
+            receivedPreEvent.should.have.property("module","foo")
+            receivedPreEvent.should.have.property("version")
+            receivedPreEvent.should.have.property("dir")
+            receivedPreEvent.should.eql(receivedPostEvent)
+            fs.existsSync(path.join(homeDir,"externalModules")).should.be.true();
+        })
+
+
         it("installs missing modules from inside subflow module", async function() {
             externalModules.init({userDir: homeDir});
             externalModules.register("function", "libs");
diff --git a/test/unit/@node-red/registry/lib/installer_spec.js b/test/unit/@node-red/registry/lib/installer_spec.js
index 5b03ed309..5311e55d0 100644
--- a/test/unit/@node-red/registry/lib/installer_spec.js
+++ b/test/unit/@node-red/registry/lib/installer_spec.js
@@ -291,16 +291,19 @@ describe('nodes/registry/installer', function() {
             }).catch(done);
         });
 
-        it("fails install if preInstall hook fails", function(done) {
+        it("skips invoking npm if preInstall returns false", function(done) {
             let receivedEvent;
-            hooks.add("preInstall", function(event) { throw new Error("preInstall-error"); })
+            hooks.add("preInstall", function(event) { return false })
+            hooks.add("postInstall", function(event) { receivedEvent = event; })
             var nodeInfo = {nodes:{module:"foo",types:["a"]}};
 
             installer.installModule("this_wont_exist","1.2.3").catch(function(err) {
                 exec.run.called.should.be.false();
+                should.exist(receivedEvent);
                 done();
             }).catch(done);
         });
+
         it("rollsback install if postInstall hook fails", function(done) {
             hooks.add("postInstall", function(event) { throw new Error("fail"); })
             installer.installModule("this_wont_exist","1.2.3").catch(function(err) {