From bf617dae728304217d75d571102298431bd8cd0a Mon Sep 17 00:00:00 2001
From: rosen-vladimirov <rosen.vladimirov@progress.com>
Date: Tue, 27 Aug 2019 18:13:28 +0300
Subject: [PATCH] fix: open feedback form on preuninstall

Currently the when the preuninstall step executes CLI's command, it exec's the process without inheriting the stdout. This way CLI always thinks the terminal is not interactive and never opens the feedback form.

Replace the exec with spawn and set "inherit" for stdout. This way CLI will determine if the terminal is in interactive mode correctly.
---
 preuninstall.js      | 15 ++++------
 test/preuninstall.ts | 65 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 71 insertions(+), 9 deletions(-)
 create mode 100644 test/preuninstall.ts

diff --git a/preuninstall.js b/preuninstall.js
index 252905118b..de893183a3 100644
--- a/preuninstall.js
+++ b/preuninstall.js
@@ -1,14 +1,11 @@
 "use strict";
 
+var path = require("path");
 var child_process = require("child_process");
-var commandArgs = ["bin/tns", "dev-preuninstall"];
+var commandArgs = [path.join(__dirname, "bin", "tns"), "dev-preuninstall"];
 
-var child = child_process.exec("node " + commandArgs.join(" "), function (error) {
-	if (error) {
-		// Some npm versions (3.0, 3.5.1, 3.7.3) remove the NativeScript node_modules before the preuninstall script is executed and the script can't find them (the preuninstall script is like postinstall script).
-		// The issue is described in the npm repository https://github.com/npm/npm/issues/8806 and it is not fixed in version 3.1.1 as commented in the conversation.
-		console.error("Failed to complete all pre-uninstall steps.");
-	}
-});
+var childProcess = child_process.spawn(process.execPath, commandArgs, { stdio: "inherit"})
 
-child.stdout.pipe(process.stdout);
+childProcess.on("error", (err) => {
+	console.error(`Failed to complete all pre-uninstall steps. Error is ${err.message}`);
+});
diff --git a/test/preuninstall.ts b/test/preuninstall.ts
new file mode 100644
index 0000000000..27f29f8aa5
--- /dev/null
+++ b/test/preuninstall.ts
@@ -0,0 +1,65 @@
+import { assert } from "chai";
+
+// Use require instead of import in order to replace the `spawn` method of child_process
+const childProcess = require("child_process");
+
+import { SpawnOptions, ChildProcess } from "child_process";
+import * as path from "path";
+import { EventEmitter } from "events";
+
+describe("preuninstall.js", () => {
+	let isSpawnCalled = false;
+	let argsPassedToSpawn: string[] = [];
+	let optionsPassedToSpawn: any[];
+	let dataPassedToConsoleError: string[] = [];
+	const originalSpawn = childProcess.spawn;
+	const originalConsoleError = console.error;
+	let eventEmitter = new EventEmitter();
+
+	beforeEach(() => {
+		isSpawnCalled = false;
+		argsPassedToSpawn = [];
+		dataPassedToConsoleError = [];
+		optionsPassedToSpawn = [];
+		eventEmitter = new EventEmitter();
+		childProcess.spawn = (command: string, args?: string[], options?: SpawnOptions): ChildProcess => {
+			isSpawnCalled = true;
+			argsPassedToSpawn = args;
+			optionsPassedToSpawn.push(options);
+			return <any>eventEmitter;
+		};
+
+		console.error = (data: string) => {
+			dataPassedToConsoleError.push(data);
+		};
+	});
+
+	afterEach(() => {
+		childProcess.spawn = originalSpawn;
+		console.error = originalConsoleError;
+	});
+
+	it("calls dev-preuninstall command of CLI and prints with console.error the error in case childProcess emits error event", () => {
+		require(path.join(__dirname, "..", "preuninstall"));
+
+		assert.isTrue(isSpawnCalled, "child_process.spawn must be called from preuninstall.js");
+
+		const expectedPathToCliExecutable = path.join(__dirname, "..", "bin", "tns");
+
+		assert.isTrue(argsPassedToSpawn.indexOf(expectedPathToCliExecutable) !== -1, `The spawned args must contain path to TNS.
+				Expected path is: ${expectedPathToCliExecutable}, current args are: ${argsPassedToSpawn}.`);
+		assert.isTrue(argsPassedToSpawn.indexOf("dev-preuninstall") !== -1, `The spawned args must contain the name of the preuninstall command.
+				Expected path is: ${expectedPathToCliExecutable}, current args are: ${argsPassedToSpawn}.`);
+		assert.deepEqual(optionsPassedToSpawn, [{ stdio: "inherit" }], "The stdio must be inherit as this way CLI's command can determine correctly if terminal is in interactive mode.");
+		assert.deepEqual(dataPassedToConsoleError, []);
+
+		const errMsg = "this is error message";
+		eventEmitter.emit("error", new Error(errMsg));
+		assert.deepEqual(dataPassedToConsoleError, [`Failed to complete all pre-uninstall steps. Error is ${errMsg}`]);
+	});
+
+	it("ensure package.json has correct preuninstall script", () => {
+		const packageJsonData = require("../package.json");
+		assert.equal(packageJsonData.scripts.preuninstall, "node preuninstall.js");
+	});
+});