diff --git a/lib/base-package-manager.ts b/lib/base-package-manager.ts index b53ee9dcb7..58cec10363 100644 --- a/lib/base-package-manager.ts +++ b/lib/base-package-manager.ts @@ -4,6 +4,7 @@ export class BasePackageManager { constructor( protected $childProcess: IChildProcess, private $hostInfo: IHostInfo, + private $pacoteService: IPacoteService, private packageManager: string ) { } @@ -17,10 +18,23 @@ export class BasePackageManager { return npmExecutableName; } - protected async processPackageManagerInstall(params: string[], opts: { cwd: string }) { + protected async processPackageManagerInstall(packageName: string, params: string[], opts: { cwd: string, isInstallingAllDependencies: boolean }): Promise { const npmExecutable = this.getPackageManagerExecutableName(); const stdioValue = isInteractive() ? "inherit" : "pipe"; - return await this.$childProcess.spawnFromEvent(npmExecutable, params, "close", { cwd: opts.cwd, stdio: stdioValue }); + await this.$childProcess.spawnFromEvent(npmExecutable, params, "close", { cwd: opts.cwd, stdio: stdioValue }); + + // Whenever calling "npm install" or "yarn add" without any arguments (hence installing all dependencies) no output is emitted on stdout + // Luckily, whenever you call "npm install" or "yarn add" to install all dependencies chances are you won't need the name/version of the package you're installing because there is none. + const { isInstallingAllDependencies } = opts; + if (isInstallingAllDependencies) { + return null; + } + + const packageMetadata = await this.$pacoteService.manifest(packageName); + return { + name: packageMetadata.name, + version: packageMetadata.version + }; } protected getFlagsString(config: any, asArray: boolean): any { diff --git a/lib/definitions/pacote-service.d.ts b/lib/definitions/pacote-service.d.ts index 8c24302a5c..3ad967e907 100644 --- a/lib/definitions/pacote-service.d.ts +++ b/lib/definitions/pacote-service.d.ts @@ -9,7 +9,7 @@ declare global { * @param packageName The name of the package * @param options The provided options can control which properties from package.json file will be returned. In case when fullMetadata option is provided, all data from package.json file will be returned. */ - manifest(packageName: string, options: IPacoteManifestOptions): Promise; + manifest(packageName: string, options?: IPacoteManifestOptions): Promise; /** * Downloads the specified package and extracts it in specified destination directory * @param packageName The name of the package diff --git a/lib/node-package-manager.ts b/lib/node-package-manager.ts index c3af8dcb08..ffc3b39119 100644 --- a/lib/node-package-manager.ts +++ b/lib/node-package-manager.ts @@ -4,17 +4,15 @@ import { exported, cache } from "./common/decorators"; import { CACACHE_DIRECTORY_NAME } from "./constants"; export class NodePackageManager extends BasePackageManager implements INodePackageManager { - private static SCOPED_DEPENDENCY_REGEXP = /^(@.+?)(?:@(.+?))?$/; - private static DEPENDENCY_REGEXP = /^(.+?)(?:@(.+?))?$/; - constructor( $childProcess: IChildProcess, private $errors: IErrors, private $fs: IFileSystem, $hostInfo: IHostInfo, private $logger: ILogger, - private $httpClient: Server.IHttpClient) { - super($childProcess, $hostInfo, 'npm'); + private $httpClient: Server.IHttpClient, + $pacoteService: IPacoteService) { + super($childProcess, $hostInfo, $pacoteService, 'npm'); } @exported("npm") @@ -56,21 +54,8 @@ export class NodePackageManager extends BasePackageManager implements INodePacka } try { - const spawnResult: ISpawnResult = await this.processPackageManagerInstall(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. - if (isInstallingAllDependencies) { - return null; - } - - params = params.concat(["--json", "--dry-run", "--prefix", cwd]); - // After the actual install runs successfully execute a dry-run in order to get information about the package. - // 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. - const spawnNpmDryRunResult = await this.$childProcess.spawnFromEvent(this.getPackageManagerExecutableName(), params, "close"); - return this.parseNpmInstallResult(spawnNpmDryRunResult.stdout, spawnResult.stdout, packageName); + const result = await this.processPackageManagerInstall(packageName, params, { cwd, isInstallingAllDependencies }); + return result; } 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. @@ -138,79 +123,6 @@ export class NodePackageManager extends BasePackageManager implements INodePacka const cachePath = await this.$childProcess.exec(`npm config get cache`); return path.join(cachePath.trim(), CACACHE_DIRECTORY_NAME); } - - private parseNpmInstallResult(npmDryRunInstallOutput: string, npmInstallOutput: string, userSpecifiedPackageName: string): INpmInstallResultInfo { - // TODO: Add tests for this functionality - try { - const originalOutput: INpmInstallCLIResult | INpm5InstallCliResult = JSON.parse(npmDryRunInstallOutput); - const npm5Output = originalOutput; - const npmOutput = originalOutput; - let name: string; - _.forOwn(npmOutput.dependencies, (peerDependency: INpmPeerDependencyInfo, key: string) => { - if (!peerDependency.required && !peerDependency.peerMissing) { - name = key; - return false; - } - }); - - // Npm 5 return different object after performing `npm install --dry-run`. - // We find the correct dependency by searching for the `userSpecifiedPackageName` in the - // `npm5Output.updated` array and as a fallback, considering that the dependency is already installed, - // we find it as the first element. - if (!name && npm5Output.updated) { - const packageNameWithoutVersion = userSpecifiedPackageName.split('@')[0]; - const updatedDependency = _.find(npm5Output.updated, ['name', packageNameWithoutVersion]) || npm5Output.updated[0]; - return { - name: updatedDependency.name, - originalOutput, - version: updatedDependency.version - }; - } - const dependency = _.pick(npmOutput.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."); - - const 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; - let 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, - version - }; - } } $injector.register("npm", NodePackageManager); diff --git a/lib/services/pacote-service.ts b/lib/services/pacote-service.ts index 94c12fbd0c..e69909931f 100644 --- a/lib/services/pacote-service.ts +++ b/lib/services/pacote-service.ts @@ -1,12 +1,19 @@ import * as pacote from "pacote"; import * as tar from "tar"; import * as path from "path"; +import { cache } from "../common/decorators"; export class PacoteService implements IPacoteService { constructor(private $fs: IFileSystem, - private $npm: INodePackageManager, - private $proxyService: IProxyService, - private $logger: ILogger) { } + private $injector: IInjector, + private $logger: ILogger, + private $proxyService: IProxyService) { } + + @cache() + public get $packageManager(): INodePackageManager { + // need to be resolved here due to cyclic dependency + return this.$injector.resolve("packageManager"); + } public async manifest(packageName: string, options?: IPacoteManifestOptions): Promise { this.$logger.trace(`Calling pacoteService.manifest for packageName: '${packageName}' and options: ${options}`); @@ -59,7 +66,7 @@ export class PacoteService implements IPacoteService { private async getPacoteBaseOptions(): Promise { // In case `tns create myapp --template https://github.com/NativeScript/template-hello-world.git` command is executed, pacote module throws an error if cache option is not provided. - const cache = await this.$npm.getCachePath(); + const cache = await this.$packageManager.getCachePath(); const pacoteOptions = { cache }; const proxySettings = await this.$proxyService.getCache(); if (proxySettings) { diff --git a/lib/yarn-package-manager.ts b/lib/yarn-package-manager.ts index a20347c45a..4897cb13b9 100644 --- a/lib/yarn-package-manager.ts +++ b/lib/yarn-package-manager.ts @@ -11,9 +11,9 @@ export class YarnPackageManager extends BasePackageManager implements INodePacka $hostInfo: IHostInfo, private $httpClient: Server.IHttpClient, private $logger: ILogger, - private $pacoteService: IPacoteService + $pacoteService: IPacoteService ) { - super($childProcess, $hostInfo, 'yarn'); + super($childProcess, $hostInfo, $pacoteService, 'yarn'); } @exported("yarn") @@ -39,18 +39,8 @@ export class YarnPackageManager extends BasePackageManager implements INodePacka const cwd = pathToSave; try { - await this.processPackageManagerInstall(params, { cwd }); - - if (isInstallingAllDependencies) { - return null; - } - - const packageMetadata = await this.$pacoteService.manifest(packageName, {}); - return { - name: packageMetadata.name, - version: packageMetadata.version - }; - + const result = await this.processPackageManagerInstall(packageName, params, { cwd, isInstallingAllDependencies }); + return result; } catch (e) { this.$fs.writeJson(packageJsonPath, jsonContentBefore); throw e; @@ -102,9 +92,9 @@ export class YarnPackageManager extends BasePackageManager implements INodePacka } @exported("yarn") - getCachePath(): Promise { - this.$errors.fail("Method not implemented"); - return null; + public async getCachePath(): Promise { + const result = await this.$childProcess.exec(`yarn cache dir`); + return result; } } diff --git a/test/plugins-service.ts b/test/plugins-service.ts index 297fadc310..2a6650afa8 100644 --- a/test/plugins-service.ts +++ b/test/plugins-service.ts @@ -132,6 +132,26 @@ function createTestInjector() { generateHashes: async (files: string[]): Promise => ({}) }); testInjector.register("pacoteService", { + manifest: async (packageName: string) => { + const projectData = testInjector.resolve("projectData"); + const fs = testInjector.resolve("fs"); + let result = {}; + let packageJsonPath = null; + + const packageToInstall = packageName.split("@")[0]; + + if (fs.exists(packageToInstall)) { + packageJsonPath = path.join(packageName, "package.json"); + } else { + packageJsonPath = path.join(projectData.projectDir, "node_modules", packageToInstall, "package.json"); + } + + if (fs.exists(packageJsonPath)) { + result = fs.readJson(packageJsonPath); + } + + return result; + }, extractPackage: async (packageName: string, destinationDirectory: string, options?: IPacoteExtractOptions): Promise => undefined }); return testInjector;