diff --git a/lib/base-package-manager.ts b/lib/base-package-manager.ts index 12b24df7d1..5ee9a96abd 100644 --- a/lib/base-package-manager.ts +++ b/lib/base-package-manager.ts @@ -17,17 +17,17 @@ export abstract class BasePackageManager implements INodePackageManager { public abstract install( packageName: string, pathToSave: string, - config: INodePackageManagerInstallOptions + config: INodePackageManagerInstallOptions, ): Promise; public abstract uninstall( packageName: string, config?: IDictionary, - path?: string + path?: string, ): Promise; public abstract view(packageName: string, config: Object): Promise; public abstract search( filter: string[], - config: IDictionary + config: IDictionary, ): Promise; public abstract searchNpms(keyword: string): Promise; public abstract getRegistryPackageData(packageName: string): Promise; @@ -38,7 +38,7 @@ export abstract class BasePackageManager implements INodePackageManager { protected $fs: IFileSystem, private $hostInfo: IHostInfo, private $pacoteService: IPacoteService, - private packageManager: string + private packageManager: string, ) {} public async isRegistered(packageName: string): Promise { @@ -65,7 +65,7 @@ export abstract class BasePackageManager implements INodePackageManager { } public async getPackageNameParts( - fullPackageName: string + fullPackageName: string, ): Promise { // support @ syntax, for example typescript@1.0.0 // support @ syntax, for example @nativescript/vue-template@1.0.0 @@ -84,7 +84,7 @@ export abstract class BasePackageManager implements INodePackageManager { } public async getPackageFullName( - packageNameParts: INpmPackageNameParts + packageNameParts: INpmPackageNameParts, ): Promise { return packageNameParts.version ? `${packageNameParts.name}@${packageNameParts.version}` @@ -104,10 +104,15 @@ export abstract class BasePackageManager implements INodePackageManager { protected async processPackageManagerInstall( packageName: string, params: string[], - opts: { cwd: string; isInstallingAllDependencies: boolean } + opts: { cwd: string; isInstallingAllDependencies: boolean }, ): Promise { const npmExecutable = this.getPackageManagerExecutableName(); - const stdioValue = isInteractive() ? "inherit" : "pipe"; + // stdin must be closed, not an open pipe: pnpm keeps the process alive + // listening on a piped stdin after the install completes, so waiting for + // "close" would hang forever. + const stdioValue: any = isInteractive() + ? "inherit" + : ["ignore", "pipe", "pipe"]; await this.$childProcess.spawnFromEvent(npmExecutable, params, "close", { cwd: opts.cwd, stdio: stdioValue, diff --git a/lib/pnpm-package-manager.ts b/lib/pnpm-package-manager.ts index f2de683552..f188b54bf8 100644 --- a/lib/pnpm-package-manager.ts +++ b/lib/pnpm-package-manager.ts @@ -26,7 +26,7 @@ export class PnpmPackageManager extends BasePackageManager { $hostInfo: IHostInfo, private $httpClient: Server.IHttpClient, private $logger: ILogger, - $pacoteService: IPacoteService + $pacoteService: IPacoteService, ) { super($childProcess, $fs, $hostInfo, $pacoteService, "pnpm"); } @@ -35,7 +35,7 @@ export class PnpmPackageManager extends BasePackageManager { public async install( packageName: string, pathToSave: string, - config: INodePackageManagerInstallOptions + config: INodePackageManagerInstallOptions, ): Promise { if (config.disableNpmInstall) { return; @@ -44,13 +44,25 @@ export class PnpmPackageManager extends BasePackageManager { if (config.ignoreScripts) { config["ignore-scripts"] = true; } + // CLI-internal options must never reach the command line: pnpm, unlike + // npm, hard-fails on unknown options. + delete config.ignoreScripts; + delete config.path; + delete config.frameworkPath; const packageJsonPath = path.join(pathToSave, "package.json"); const jsonContentBefore = this.$fs.readJson(packageJsonPath); const flags = this.getFlagsString(config, true); - // With pnpm we need to install as "flat" or some imports wont be found - let params = ["i", "--shamefully-hoist"]; + let params = ["i"]; + if (!this.projectManagesOwnHoisting(pathToSave)) { + // With pnpm's default isolated layout some imports won't be found, so + // install "flat". Skipped when the project configures its own layout: + // pnpm treats a hoisting flag that contradicts the stored install state + // as a config change and rebuilds node_modules from scratch after a + // prompt (aborting outright when there is no TTY). + params.push("--shamefully-hoist"); + } const isInstallingAllDependencies = packageName === pathToSave; if (!isInstallingAllDependencies) { params.push(packageName); @@ -63,7 +75,7 @@ export class PnpmPackageManager extends BasePackageManager { const result = await this.processPackageManagerInstall( packageName, params, - { cwd, isInstallingAllDependencies } + { cwd, isInstallingAllDependencies }, ); return result; } catch (e) { @@ -76,7 +88,7 @@ export class PnpmPackageManager extends BasePackageManager { public uninstall( packageName: string, config?: IDictionary, - cwd?: string + cwd?: string, ): Promise { // pnpm does not want save option in remove. It saves it by default delete config["save"]; @@ -94,7 +106,7 @@ export class PnpmPackageManager extends BasePackageManager { let viewResult: any; try { viewResult = await this.$childProcess.exec( - `pnpm info ${packageName} ${flags}` + `pnpm info ${packageName} ${flags}`, ); } catch (e) { this.$errors.fail(e.message); @@ -110,7 +122,7 @@ export class PnpmPackageManager extends BasePackageManager { @exported("pnpm") public search( filter: string[], - config: IDictionary + config: IDictionary, ): Promise { const flags = this.getFlagsString(config, false); return this.$childProcess.exec(`pnpm search ${filter.join(" ")} ${flags}`); @@ -118,7 +130,7 @@ export class PnpmPackageManager extends BasePackageManager { public async searchNpms(keyword: string): Promise { const httpRequestResult = await this.$httpClient.httpRequest( - `https://api.npms.io/v2/search?q=keywords:${keyword}` + `https://api.npms.io/v2/search?q=keywords:${keyword}`, ); const result: INpmsResult = JSON.parse(httpRequestResult.body); return result; @@ -129,15 +141,15 @@ export class PnpmPackageManager extends BasePackageManager { const registry = await this.$childProcess.exec(`pnpm config get registry`); const url = `${registry.trim()}/${packageName}`; this.$logger.trace( - `Trying to get data from pnpm registry for package ${packageName}, url is: ${url}` + `Trying to get data from pnpm registry for package ${packageName}, url is: ${url}`, ); const responseData = (await this.$httpClient.httpRequest(url)).body; this.$logger.trace( - `Successfully received data from pnpm registry for package ${packageName}. Response data is: ${responseData}` + `Successfully received data from pnpm registry for package ${packageName}. Response data is: ${responseData}`, ); const jsonData = JSON.parse(responseData); this.$logger.trace( - `Successfully parsed data from pnpm registry for package ${packageName}.` + `Successfully parsed data from pnpm registry for package ${packageName}.`, ); return jsonData; } @@ -145,7 +157,44 @@ export class PnpmPackageManager extends BasePackageManager { @exported("pnpm") public async getCachePath(): Promise { const cachePath = await this.$childProcess.exec(`pnpm config get cache`); - return path.join(cachePath.trim(), CACACHE_DIRECTORY_NAME); + const cacheDir = cachePath && cachePath.trim(); + // pnpm has no `cache` config key of its own: modern versions print + // "undefined" (older ones an empty string), which would yield a relative + // garbage path. Derive a stable per-user location from the store instead. + if (cacheDir && cacheDir !== "undefined" && cacheDir !== "null") { + return path.join(cacheDir, CACACHE_DIRECTORY_NAME); + } + const storePath = await this.$childProcess.exec(`pnpm store path`); + return path.join(path.dirname(storePath.trim()), CACACHE_DIRECTORY_NAME); + } + + private projectManagesOwnHoisting(installDir: string): boolean { + // A pnpm-workspace.yaml (pnpm's config home since v10) or an .npmrc with + // a layout key marks the node_modules layout as the project's own choice. + const layoutKeyPattern = + /^\s*(shamefully-hoist|node-linker|hoist|hoist-pattern|public-hoist-pattern)\s*[=:]/m; + let dir = path.resolve(installDir); + while (true) { + if (this.$fs.exists(path.join(dir, "pnpm-workspace.yaml"))) { + return true; + } + const npmrcPath = path.join(dir, ".npmrc"); + if (this.$fs.exists(npmrcPath)) { + try { + const npmrcContent = this.$fs.readText(npmrcPath); + if (npmrcContent && layoutKeyPattern.test(npmrcContent)) { + return true; + } + } catch (err) { + this.$logger.trace(`Unable to read ${npmrcPath}. Error is: `, err); + } + } + const parent = path.dirname(dir); + if (parent === dir) { + return false; + } + dir = parent; + } } } diff --git a/package-lock.json b/package-lock.json index b8c9b3872e..18441700b1 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "nativescript", - "version": "9.1.0", + "version": "9.1.1-dev.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "nativescript", - "version": "9.1.0", + "version": "9.1.1-dev.0", "hasInstallScript": true, "license": "Apache-2.0", "dependencies": { diff --git a/package.json b/package.json index fd04fe9c73..0c8f4d5191 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "nativescript", "main": "./dist/lib/nativescript-cli-lib.js", - "version": "9.1.0", + "version": "9.1.1-dev.0", "author": "NativeScript ", "description": "Command-line interface for building NativeScript projects", "bin": { diff --git a/test/pnpm-package-manager.ts b/test/pnpm-package-manager.ts new file mode 100644 index 0000000000..530a361c9a --- /dev/null +++ b/test/pnpm-package-manager.ts @@ -0,0 +1,237 @@ +import * as path from "path"; +import { Yok } from "../lib/common/yok"; +import * as stubs from "./stubs"; +import { assert } from "chai"; +import { PnpmPackageManager } from "../lib/pnpm-package-manager"; +import { IInjector } from "../lib/common/definitions/yok"; + +class RecordingChildProcessStub extends stubs.ChildProcessStub { + public execResponses: { [commandPrefix: string]: string } = {}; + public spawnedArgs: string[][] = []; + + public async exec( + command: string, + options?: any, + execOptions?: any, + ): Promise { + await super.exec(command, options, execOptions); + for (const prefix in this.execResponses) { + if (command.startsWith(prefix)) { + return this.execResponses[prefix]; + } + } + return null; + } + + public spawnedOptions: any[] = []; + + public async spawnFromEvent( + command: string, + args: string[], + event: string, + options?: any, + spawnFromEventOptions?: any, + ): Promise { + this.spawnedArgs.push(args); + this.spawnedOptions.push(options); + return super.spawnFromEvent( + command, + args, + event, + options, + spawnFromEventOptions, + ); + } +} + +class SelectiveFileSystemStub extends stubs.FileSystemStub { + public existingPaths: string[] = []; + public textFiles: { [filePath: string]: string } = {}; + + exists(filePath: string): boolean { + return this.existingPaths.indexOf(filePath) !== -1; + } + + readText(filename: string): string { + return this.textFiles[filename]; + } +} + +function createTestInjector(): IInjector { + const injector = new Yok(); + injector.register("hostInfo", { isWindows: false }); + injector.register("errors", stubs.ErrorsStub); + injector.register("logger", stubs.LoggerStub); + injector.register("childProcess", RecordingChildProcessStub); + injector.register("httpClient", {}); + injector.register("fs", SelectiveFileSystemStub); + injector.register("pnpm", PnpmPackageManager); + injector.register("pacoteService", { + manifest: () => Promise.resolve({ name: "left-pad", version: "1.3.0" }), + }); + + return injector; +} + +describe("pnpm-package-manager", () => { + const projectDir = path.join("/tmp", "some-project"); + + describe("install", () => { + it("passes --shamefully-hoist when the project has no pnpm layout config", async () => { + const testInjector = createTestInjector(); + const pnpm = testInjector.resolve("pnpm"); + const childProcess = + testInjector.resolve("childProcess"); + + await pnpm.install(projectDir, projectDir, {} as any); + + assert.deepEqual(childProcess.spawnedArgs[0], [ + "i", + "--shamefully-hoist", + ]); + }); + + it("omits --shamefully-hoist when a pnpm-workspace.yaml governs the project", async () => { + const testInjector = createTestInjector(); + const pnpm = testInjector.resolve("pnpm"); + const childProcess = + testInjector.resolve("childProcess"); + const fs = testInjector.resolve("fs"); + fs.existingPaths = [path.join(projectDir, "pnpm-workspace.yaml")]; + + await pnpm.install(projectDir, projectDir, {} as any); + + assert.deepEqual(childProcess.spawnedArgs[0], ["i"]); + }); + + it("omits --shamefully-hoist when an ancestor pnpm-workspace.yaml governs the project", async () => { + const testInjector = createTestInjector(); + const pnpm = testInjector.resolve("pnpm"); + const childProcess = + testInjector.resolve("childProcess"); + const fs = testInjector.resolve("fs"); + fs.existingPaths = [path.join("/tmp", "pnpm-workspace.yaml")]; + + await pnpm.install(projectDir, projectDir, {} as any); + + assert.deepEqual(childProcess.spawnedArgs[0], ["i"]); + }); + + it("omits --shamefully-hoist when an .npmrc sets a layout key", async () => { + const testInjector = createTestInjector(); + const pnpm = testInjector.resolve("pnpm"); + const childProcess = + testInjector.resolve("childProcess"); + const fs = testInjector.resolve("fs"); + const npmrcPath = path.join(projectDir, ".npmrc"); + fs.existingPaths = [npmrcPath]; + fs.textFiles[npmrcPath] = + "registry=https://example.com\nnode-linker=hoisted\n"; + + await pnpm.install(projectDir, projectDir, {} as any); + + assert.deepEqual(childProcess.spawnedArgs[0], ["i"]); + }); + + it("keeps --shamefully-hoist when an .npmrc has no layout key", async () => { + const testInjector = createTestInjector(); + const pnpm = testInjector.resolve("pnpm"); + const childProcess = + testInjector.resolve("childProcess"); + const fs = testInjector.resolve("fs"); + const npmrcPath = path.join(projectDir, ".npmrc"); + fs.existingPaths = [npmrcPath]; + fs.textFiles[npmrcPath] = "registry=https://example.com\n"; + + await pnpm.install(projectDir, projectDir, {} as any); + + assert.deepEqual(childProcess.spawnedArgs[0], [ + "i", + "--shamefully-hoist", + ]); + }); + + it("maps ignoreScripts to --ignore-scripts and drops internal options pnpm rejects", async () => { + const testInjector = createTestInjector(); + const pnpm = testInjector.resolve("pnpm"); + const childProcess = + testInjector.resolve("childProcess"); + + await pnpm.install(projectDir, projectDir, { + ignoreScripts: true, + path: "/some/path", + frameworkPath: "/some/framework", + } as any); + + const args = childProcess.spawnedArgs[0]; + assert.include(args, "--ignore-scripts"); + assert.notInclude(args, "--ignoreScripts"); + assert.notInclude(args, "--path"); + assert.notInclude(args, "--frameworkPath"); + }); + + it("spawns non-interactive installs with stdin closed", async () => { + const testInjector = createTestInjector(); + const pnpm = testInjector.resolve("pnpm"); + const childProcess = + testInjector.resolve("childProcess"); + + await pnpm.install(projectDir, projectDir, {} as any); + + // pnpm never exits while its stdin is an open pipe, so anything but + // "ignore" here hangs the CLI's wait for the child's "close" event. + assert.deepEqual(childProcess.spawnedOptions[0].stdio, [ + "ignore", + "pipe", + "pipe", + ]); + }); + + it("appends the package name when installing a single package", async () => { + const testInjector = createTestInjector(); + const pnpm = testInjector.resolve("pnpm"); + const childProcess = + testInjector.resolve("childProcess"); + + await pnpm.install("left-pad", projectDir, { save: true } as any); + + assert.deepEqual(childProcess.spawnedArgs[0], [ + "i", + "--shamefully-hoist", + "left-pad", + "--save", + ]); + }); + }); + + describe("getCachePath", () => { + it("uses the configured cache directory when pnpm reports one", async () => { + const testInjector = createTestInjector(); + const pnpm = testInjector.resolve("pnpm"); + const childProcess = + testInjector.resolve("childProcess"); + childProcess.execResponses["pnpm config get cache"] = "/custom/cache\n"; + + const cachePath = await pnpm.getCachePath(); + + assert.equal(cachePath, path.join("/custom/cache", "_cacache")); + }); + + it("falls back to the store's parent directory when the cache key is unset", async () => { + const testInjector = createTestInjector(); + const pnpm = testInjector.resolve("pnpm"); + const childProcess = + testInjector.resolve("childProcess"); + childProcess.execResponses["pnpm config get cache"] = "undefined\n"; + childProcess.execResponses["pnpm store path"] = + "/Users/someone/Library/pnpm/store/v10\n"; + + const cachePath = await pnpm.getCachePath(); + + assert.equal( + cachePath, + path.join("/Users/someone/Library/pnpm/store", "_cacache"), + ); + }); + }); +});