From 2c627a0a18eb0a69aaac6c3c4eb6c952ff9fc587 Mon Sep 17 00:00:00 2001 From: rosen-vladimirov Date: Mon, 15 May 2017 13:16:00 +0300 Subject: [PATCH] Fix getting information for installed npm package 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: ``` `-- nativescript-barcodescanner@1.0.0 ``` 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 nativescript-barcodescanner@1.0.0 ``` 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. --- lib/declarations.d.ts | 2 +- lib/node-package-manager.ts | 123 ++++++++++++++++++++++++++++---- lib/services/plugins-service.ts | 2 +- 3 files changed, 113 insertions(+), 14 deletions(-) diff --git a/lib/declarations.d.ts b/lib/declarations.d.ts index bff60a8780..d3b566fbb4 100644 --- a/lib/declarations.d.ts +++ b/lib/declarations.d.ts @@ -193,7 +193,7 @@ interface INpmInstallResultInfo { * The original output that npm CLI produced upon installation. * @type {INpmInstallCLIResult} */ - originalOutput: INpmInstallCLIResult; + originalOutput?: INpmInstallCLIResult; } interface INpmInstallOptions { diff --git a/lib/node-package-manager.ts b/lib/node-package-manager.ts index 7b9ad7f05e..c64f2c8236 100644 --- a/lib/node-package-manager.ts +++ b/lib/node-package-manager.ts @@ -2,6 +2,9 @@ import * as path from "path"; import { exported } from "./common/decorators"; export class NodePackageManager implements INodePackageManager { + private static SCOPED_DEPENDENCY_REGEXP = /^(@.+?)(?:@(.+?))?$/; + private static DEPENDENCY_REGEXP = /^(.+?)(?:@(.+?))?$/; + constructor(private $fs: IFileSystem, private $hostInfo: IHostInfo, private $errors: IErrors, @@ -47,10 +50,7 @@ export class NodePackageManager implements INodePackageManager { } try { - let spawnResult: ISpawnResult = await this.$childProcess.spawnFromEvent(this.getNpmExecutableName(), params, "close", { cwd, stdio: "inherit" }); - if (!etcExistsPriorToInstallation) { - this.$fs.deleteDirectory(etcDirectoryLocation); - } + let spawnResult: ISpawnResult = await this.getNpmInstallResult(params, cwd); // Whenever calling npm install without any arguments (hence installing all dependencies) no output is emitted on stdout // 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 { // We cannot use the actual install with --json to get the information because of post-install scripts which may print on stdout // 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 // We need the --prefix here because without it no output is emitted on stdout because all the dependencies are already installed. - spawnResult = await this.$childProcess.spawnFromEvent(this.getNpmExecutableName(), params, "close"); - return this.parseNpmInstallResult(spawnResult.stdout); + let spawnNpmDryRunResult = await this.$childProcess.spawnFromEvent(this.getNpmExecutableName(), params, "close"); + return this.parseNpmInstallResult(spawnNpmDryRunResult.stdout, spawnResult.stdout, packageName); } catch (err) { if (err.message && err.message.indexOf("EPEERINVALID") !== -1) { // 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 { this.$fs.writeJson(packageJsonPath, jsonContentBefore); throw err; } + } finally { + if (!etcExistsPriorToInstallation) { + this.$fs.deleteDirectory(etcDirectoryLocation); + } } } @@ -136,16 +140,111 @@ export class NodePackageManager implements INodePackageManager { return array.join(" "); } - private parseNpmInstallResult(npmInstallOutput: string): INpmInstallResultInfo { - const originalOutput: INpmInstallCLIResult = JSON.parse(npmInstallOutput); - const name = _.head(_.keys(originalOutput.dependencies)); - const dependency = _.pick(originalOutput.dependencies, name); + private parseNpmInstallResult(npmDryRunInstallOutput: string, npmInstallOutput: string, userSpecifiedPackageName: string): INpmInstallResultInfo { + // TODO: Add tests for this functionality + try { + const originalOutput: INpmInstallCLIResult = JSON.parse(npmDryRunInstallOutput); + const name = _.head(_.keys(originalOutput.dependencies)); + const dependency = _.pick(originalOutput.dependencies, name); + return { + name, + originalOutput, + version: dependency[name].version + }; + } catch (err) { + this.$logger.trace(`Unable to parse result of npm --dry-run operation. Output is: ${npmDryRunInstallOutput}.`); + this.$logger.trace("Now we'll try to parse the real output of npm install command."); + + let npmOutputMatchRegExp = /^.--\s+(?!UNMET)(.*)@((?:\d+\.){2}\d+)/m; + const match = npmInstallOutput.match(npmOutputMatchRegExp); + if (match) { + return { + name: match[1], + version: match[2] + }; + } + } + + this.$logger.trace("Unable to get information from npm installation, trying to return value specified by user."); + return this.getDependencyInformation(userSpecifiedPackageName); + } + + private getDependencyInformation(dependency: string): INpmInstallResultInfo { + const scopeDependencyMatch = dependency.match(NodePackageManager.SCOPED_DEPENDENCY_REGEXP); + let name: string = null, + version: string = null; + + if (scopeDependencyMatch) { + name = scopeDependencyMatch[1]; + version = scopeDependencyMatch[2]; + } else { + const matches = dependency.match(NodePackageManager.DEPENDENCY_REGEXP); + if (matches) { + name = matches[1]; + version = matches[2]; + } + } + return { name, - originalOutput, - version: dependency[name].version + version }; } + + private async getNpmInstallResult(params: string[], cwd: string): Promise { + return new Promise((resolve, reject) => { + const npmExecutable = this.getNpmExecutableName(); + let childProcess = this.$childProcess.spawn(npmExecutable, params, { cwd, stdio: "pipe" }); + + let isFulfilled = false; + let capturedOut = ""; + let capturedErr = ""; + + childProcess.stdout.on("data", (data: string) => { + this.$logger.write(data.toString()); + capturedOut += data; + }); + + if (childProcess.stderr) { + childProcess.stderr.on("data", (data: string) => { + console.error(data.toString()); + capturedErr += data; + }); + } + + childProcess.on("close", (arg: any) => { + const exitCode = typeof arg === "number" ? arg : arg && arg.code; + + if (exitCode === 0) { + isFulfilled = true; + const result = { + stdout: capturedOut, + stderr: capturedErr, + exitCode + }; + + resolve(result); + } else { + let errorMessage = `Command ${npmExecutable} ${params && params.join(" ")} failed with exit code ${exitCode}`; + if (capturedErr) { + errorMessage += ` Error output: \n ${capturedErr}`; + } + + if (!isFulfilled) { + isFulfilled = true; + reject(new Error(errorMessage)); + } + } + }); + + childProcess.on("error", (err: Error) => { + if (!isFulfilled) { + isFulfilled = true; + reject(err); + } + }); + }); + } } $injector.register("npm", NodePackageManager); diff --git a/lib/services/plugins-service.ts b/lib/services/plugins-service.ts index 1af98fb70c..c1fae529f9 100644 --- a/lib/services/plugins-service.ts +++ b/lib/services/plugins-service.ts @@ -71,7 +71,7 @@ export class PluginsService implements IPluginsService { this.$logger.out(`Successfully installed plugin ${realNpmPackageJson.name}.`); } else { - this.$npm.uninstall(realNpmPackageJson.name, { save: true }, projectData.projectDir); + await this.$npm.uninstall(realNpmPackageJson.name, { save: true }, projectData.projectDir); this.$errors.failWithoutHelp(`${plugin} is not a valid NativeScript plugin. Verify that the plugin package.json file contains a nativescript key and try again.`); } }