Skip to content

Commit 648fda6

Browse files
Merge pull request #4983 from NativeScript/vladimirov/fix-uninstall-form
fix: open feedback form on preuninstall
2 parents b419c21 + bf617da commit 648fda6

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)