Skip to content

Commit 6b9ffeb

Browse files
Fix getting information for installed npm package (#2813)
Some npm versions (like 3.9.6) do not report result when package is already installed and `--dry-run` is passed. This breaks the `JSON.parse()` of the result as it is an empty string. In this case just parse the original output of `npm install` and search for a result like: ``` <project dir> `-- [email protected] ``` In case this operation also fails to find which is the installed package, try to find the name from the user specified value. For example: ``` $ tns plugin add [email protected] ``` The name is `nativescript-barcodescanner` and the version is `1.0.0`. In case a scoped package is passed, the parsing should also work correctly. In case a package, that is not NativeScript plugin, is installed via `tns plugin add` command, we have logic to revert the installation. However the promise that should uninstall the app had never been awaited, so the package was still declared and installed. When `--json` is passed to `npm install` command, it generates `/etc` dir in the project where npm command is executed. We have logic to delete it after finishing our command, but it's been executed too early. Fix the code, so command to remove `/etc` will be executed no matter if any of the other operations fails.
1 parent 0d13919 commit 6b9ffeb

File tree

3 files changed

+113
-14
lines changed

3 files changed

+113
-14
lines changed

lib/declarations.d.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ interface INpmInstallResultInfo {
193193
* The original output that npm CLI produced upon installation.
194194
* @type {INpmInstallCLIResult}
195195
*/
196-
originalOutput: INpmInstallCLIResult;
196+
originalOutput?: INpmInstallCLIResult;
197197
}
198198

199199
interface INpmInstallOptions {

lib/node-package-manager.ts

+111-12
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@ import * as path from "path";
22
import { exported } from "./common/decorators";
33

44
export class NodePackageManager implements INodePackageManager {
5+
private static SCOPED_DEPENDENCY_REGEXP = /^(@.+?)(?:@(.+?))?$/;
6+
private static DEPENDENCY_REGEXP = /^(.+?)(?:@(.+?))?$/;
7+
58
constructor(private $fs: IFileSystem,
69
private $hostInfo: IHostInfo,
710
private $errors: IErrors,
@@ -47,10 +50,7 @@ export class NodePackageManager implements INodePackageManager {
4750
}
4851

4952
try {
50-
let spawnResult: ISpawnResult = await this.$childProcess.spawnFromEvent(this.getNpmExecutableName(), params, "close", { cwd, stdio: "inherit" });
51-
if (!etcExistsPriorToInstallation) {
52-
this.$fs.deleteDirectory(etcDirectoryLocation);
53-
}
53+
let spawnResult: ISpawnResult = await this.getNpmInstallResult(params, cwd);
5454

5555
// Whenever calling npm install without any arguments (hence installing all dependencies) no output is emitted on stdout
5656
// Luckily, whenever you call npm install to install all dependencies chances are you won't need the name/version of the package you're installing because there is none.
@@ -63,8 +63,8 @@ export class NodePackageManager implements INodePackageManager {
6363
// We cannot use the actual install with --json to get the information because of post-install scripts which may print on stdout
6464
// dry-run install is quite fast when the dependencies are already installed even for many dependencies (e.g. angular) so we can live with this approach
6565
// We need the --prefix here because without it no output is emitted on stdout because all the dependencies are already installed.
66-
spawnResult = await this.$childProcess.spawnFromEvent(this.getNpmExecutableName(), params, "close");
67-
return this.parseNpmInstallResult(spawnResult.stdout);
66+
let spawnNpmDryRunResult = await this.$childProcess.spawnFromEvent(this.getNpmExecutableName(), params, "close");
67+
return this.parseNpmInstallResult(spawnNpmDryRunResult.stdout, spawnResult.stdout, packageName);
6868
} catch (err) {
6969
if (err.message && err.message.indexOf("EPEERINVALID") !== -1) {
7070
// Not installed peer dependencies are treated by npm 2 as errors, but npm 3 treats them as warnings.
@@ -76,6 +76,10 @@ export class NodePackageManager implements INodePackageManager {
7676
this.$fs.writeJson(packageJsonPath, jsonContentBefore);
7777
throw err;
7878
}
79+
} finally {
80+
if (!etcExistsPriorToInstallation) {
81+
this.$fs.deleteDirectory(etcDirectoryLocation);
82+
}
7983
}
8084
}
8185

@@ -136,16 +140,111 @@ export class NodePackageManager implements INodePackageManager {
136140
return array.join(" ");
137141
}
138142

139-
private parseNpmInstallResult(npmInstallOutput: string): INpmInstallResultInfo {
140-
const originalOutput: INpmInstallCLIResult = JSON.parse(npmInstallOutput);
141-
const name = _.head(_.keys(originalOutput.dependencies));
142-
const dependency = _.pick<INpmDependencyInfo, INpmDependencyInfo | INpmPeerDependencyInfo>(originalOutput.dependencies, name);
143+
private parseNpmInstallResult(npmDryRunInstallOutput: string, npmInstallOutput: string, userSpecifiedPackageName: string): INpmInstallResultInfo {
144+
// TODO: Add tests for this functionality
145+
try {
146+
const originalOutput: INpmInstallCLIResult = JSON.parse(npmDryRunInstallOutput);
147+
const name = _.head(_.keys(originalOutput.dependencies));
148+
const dependency = _.pick<INpmDependencyInfo, INpmDependencyInfo | INpmPeerDependencyInfo>(originalOutput.dependencies, name);
149+
return {
150+
name,
151+
originalOutput,
152+
version: dependency[name].version
153+
};
154+
} catch (err) {
155+
this.$logger.trace(`Unable to parse result of npm --dry-run operation. Output is: ${npmDryRunInstallOutput}.`);
156+
this.$logger.trace("Now we'll try to parse the real output of npm install command.");
157+
158+
let npmOutputMatchRegExp = /^.--\s+(?!UNMET)(.*)@((?:\d+\.){2}\d+)/m;
159+
const match = npmInstallOutput.match(npmOutputMatchRegExp);
160+
if (match) {
161+
return {
162+
name: match[1],
163+
version: match[2]
164+
};
165+
}
166+
}
167+
168+
this.$logger.trace("Unable to get information from npm installation, trying to return value specified by user.");
169+
return this.getDependencyInformation(userSpecifiedPackageName);
170+
}
171+
172+
private getDependencyInformation(dependency: string): INpmInstallResultInfo {
173+
const scopeDependencyMatch = dependency.match(NodePackageManager.SCOPED_DEPENDENCY_REGEXP);
174+
let name: string = null,
175+
version: string = null;
176+
177+
if (scopeDependencyMatch) {
178+
name = scopeDependencyMatch[1];
179+
version = scopeDependencyMatch[2];
180+
} else {
181+
const matches = dependency.match(NodePackageManager.DEPENDENCY_REGEXP);
182+
if (matches) {
183+
name = matches[1];
184+
version = matches[2];
185+
}
186+
}
187+
143188
return {
144189
name,
145-
originalOutput,
146-
version: dependency[name].version
190+
version
147191
};
148192
}
193+
194+
private async getNpmInstallResult(params: string[], cwd: string): Promise<ISpawnResult> {
195+
return new Promise<ISpawnResult>((resolve, reject) => {
196+
const npmExecutable = this.getNpmExecutableName();
197+
let childProcess = this.$childProcess.spawn(npmExecutable, params, { cwd, stdio: "pipe" });
198+
199+
let isFulfilled = false;
200+
let capturedOut = "";
201+
let capturedErr = "";
202+
203+
childProcess.stdout.on("data", (data: string) => {
204+
this.$logger.write(data.toString());
205+
capturedOut += data;
206+
});
207+
208+
if (childProcess.stderr) {
209+
childProcess.stderr.on("data", (data: string) => {
210+
console.error(data.toString());
211+
capturedErr += data;
212+
});
213+
}
214+
215+
childProcess.on("close", (arg: any) => {
216+
const exitCode = typeof arg === "number" ? arg : arg && arg.code;
217+
218+
if (exitCode === 0) {
219+
isFulfilled = true;
220+
const result = {
221+
stdout: capturedOut,
222+
stderr: capturedErr,
223+
exitCode
224+
};
225+
226+
resolve(result);
227+
} else {
228+
let errorMessage = `Command ${npmExecutable} ${params && params.join(" ")} failed with exit code ${exitCode}`;
229+
if (capturedErr) {
230+
errorMessage += ` Error output: \n ${capturedErr}`;
231+
}
232+
233+
if (!isFulfilled) {
234+
isFulfilled = true;
235+
reject(new Error(errorMessage));
236+
}
237+
}
238+
});
239+
240+
childProcess.on("error", (err: Error) => {
241+
if (!isFulfilled) {
242+
isFulfilled = true;
243+
reject(err);
244+
}
245+
});
246+
});
247+
}
149248
}
150249

151250
$injector.register("npm", NodePackageManager);

lib/services/plugins-service.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ export class PluginsService implements IPluginsService {
7171

7272
this.$logger.out(`Successfully installed plugin ${realNpmPackageJson.name}.`);
7373
} else {
74-
this.$npm.uninstall(realNpmPackageJson.name, { save: true }, projectData.projectDir);
74+
await this.$npm.uninstall(realNpmPackageJson.name, { save: true }, projectData.projectDir);
7575
this.$errors.failWithoutHelp(`${plugin} is not a valid NativeScript plugin. Verify that the plugin package.json file contains a nativescript key and try again.`);
7676
}
7777
}

0 commit comments

Comments
 (0)