Skip to content

Commit bf617da

Browse files
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.
1 parent b419c21 commit bf617da

File tree

2 files changed

+71
-9
lines changed

2 files changed

+71
-9
lines changed

preuninstall.js

+6-9
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,11 @@
11
"use strict";
22

3+
var path = require("path");
34
var child_process = require("child_process");
4-
var commandArgs = ["bin/tns", "dev-preuninstall"];
5+
var commandArgs = [path.join(__dirname, "bin", "tns"), "dev-preuninstall"];
56

6-
var child = child_process.exec("node " + commandArgs.join(" "), function (error) {
7-
if (error) {
8-
// 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).
9-
// 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.
10-
console.error("Failed to complete all pre-uninstall steps.");
11-
}
12-
});
7+
var childProcess = child_process.spawn(process.execPath, commandArgs, { stdio: "inherit"})
138

14-
child.stdout.pipe(process.stdout);
9+
childProcess.on("error", (err) => {
10+
console.error(`Failed to complete all pre-uninstall steps. Error is ${err.message}`);
11+
});

test/preuninstall.ts

+65
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
import { assert } from "chai";
2+
3+
// Use require instead of import in order to replace the `spawn` method of child_process
4+
const childProcess = require("child_process");
5+
6+
import { SpawnOptions, ChildProcess } from "child_process";
7+
import * as path from "path";
8+
import { EventEmitter } from "events";
9+
10+
describe("preuninstall.js", () => {
11+
let isSpawnCalled = false;
12+
let argsPassedToSpawn: string[] = [];
13+
let optionsPassedToSpawn: any[];
14+
let dataPassedToConsoleError: string[] = [];
15+
const originalSpawn = childProcess.spawn;
16+
const originalConsoleError = console.error;
17+
let eventEmitter = new EventEmitter();
18+
19+
beforeEach(() => {
20+
isSpawnCalled = false;
21+
argsPassedToSpawn = [];
22+
dataPassedToConsoleError = [];
23+
optionsPassedToSpawn = [];
24+
eventEmitter = new EventEmitter();
25+
childProcess.spawn = (command: string, args?: string[], options?: SpawnOptions): ChildProcess => {
26+
isSpawnCalled = true;
27+
argsPassedToSpawn = args;
28+
optionsPassedToSpawn.push(options);
29+
return <any>eventEmitter;
30+
};
31+
32+
console.error = (data: string) => {
33+
dataPassedToConsoleError.push(data);
34+
};
35+
});
36+
37+
afterEach(() => {
38+
childProcess.spawn = originalSpawn;
39+
console.error = originalConsoleError;
40+
});
41+
42+
it("calls dev-preuninstall command of CLI and prints with console.error the error in case childProcess emits error event", () => {
43+
require(path.join(__dirname, "..", "preuninstall"));
44+
45+
assert.isTrue(isSpawnCalled, "child_process.spawn must be called from preuninstall.js");
46+
47+
const expectedPathToCliExecutable = path.join(__dirname, "..", "bin", "tns");
48+
49+
assert.isTrue(argsPassedToSpawn.indexOf(expectedPathToCliExecutable) !== -1, `The spawned args must contain path to TNS.
50+
Expected path is: ${expectedPathToCliExecutable}, current args are: ${argsPassedToSpawn}.`);
51+
assert.isTrue(argsPassedToSpawn.indexOf("dev-preuninstall") !== -1, `The spawned args must contain the name of the preuninstall command.
52+
Expected path is: ${expectedPathToCliExecutable}, current args are: ${argsPassedToSpawn}.`);
53+
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.");
54+
assert.deepEqual(dataPassedToConsoleError, []);
55+
56+
const errMsg = "this is error message";
57+
eventEmitter.emit("error", new Error(errMsg));
58+
assert.deepEqual(dataPassedToConsoleError, [`Failed to complete all pre-uninstall steps. Error is ${errMsg}`]);
59+
});
60+
61+
it("ensure package.json has correct preuninstall script", () => {
62+
const packageJsonData = require("../package.json");
63+
assert.equal(packageJsonData.scripts.preuninstall, "node preuninstall.js");
64+
});
65+
});

0 commit comments

Comments
 (0)