From f40ff395b0c3b76174400e212178dcb20a244453 Mon Sep 17 00:00:00 2001 From: Igor Randjelovic Date: Wed, 16 Sep 2026 11:11:32 +0200 Subject: [PATCH 1/6] refactor: move package managers into lib/package-managers Group the package manager dispatcher, the per-manager implementations (npm, yarn, yarn2, pnpm, bun), their shared base class and the installation manager under a single directory, and shorten the implementation class names to match their file names. --- lib/bootstrap.ts | 14 +++++----- .../base-package-manager.ts | 6 ++--- .../bun.ts} | 14 +++++----- .../index.ts} | 14 +++++----- .../npm.ts} | 14 +++++----- .../package-installation-manager.ts | 10 +++---- .../pnpm.ts} | 14 +++++----- .../yarn.ts} | 12 ++++----- .../yarn2.ts} | 12 ++++----- test/bun-package-manager.ts | 8 +++--- test/controllers/add-platform-controller.ts | 22 ++++++++-------- test/ios-project-service.ts | 10 +++---- test/node-package-manager.ts | 8 +++--- test/package-installation-manager.ts | 24 ++++++++--------- test/plugins-service.ts | 24 ++++++++--------- test/pnpm-package-manager.ts | 26 +++++++++---------- test/services/extensibility-service.ts | 22 ++++++++-------- 17 files changed, 127 insertions(+), 127 deletions(-) rename lib/{ => package-managers}/base-package-manager.ts (98%) rename lib/{bun-package-manager.ts => package-managers/bun.ts} (93%) rename lib/{package-manager.ts => package-managers/index.ts} (93%) rename lib/{node-package-manager.ts => package-managers/npm.ts} (94%) rename lib/{ => package-managers}/package-installation-manager.ts (97%) rename lib/{pnpm-package-manager.ts => package-managers/pnpm.ts} (95%) rename lib/{yarn-package-manager.ts => package-managers/yarn.ts} (93%) rename lib/{yarn2-package-manager.ts => package-managers/yarn2.ts} (94%) diff --git a/lib/bootstrap.ts b/lib/bootstrap.ts index 154a325910..a1dfe80dc5 100644 --- a/lib/bootstrap.ts +++ b/lib/bootstrap.ts @@ -379,12 +379,12 @@ registerBuiltInCommand< typeof import("./commands/setup").setupCommandDefinition >("setup|*", () => require("./commands/setup").setupCommandDefinition); -injector.requirePublic("packageManager", "./package-manager"); -injector.requirePublic("npm", "./node-package-manager"); -injector.requirePublic("yarn", "./yarn-package-manager"); -injector.requirePublic("yarn2", "./yarn2-package-manager"); -injector.requirePublic("pnpm", "./pnpm-package-manager"); -injector.requirePublic("bun", "./bun-package-manager"); +injector.requirePublic("packageManager", "./package-managers/index"); +injector.requirePublic("npm", "./package-managers/npm"); +injector.requirePublic("yarn", "./package-managers/yarn"); +injector.requirePublic("yarn2", "./package-managers/yarn2"); +injector.requirePublic("pnpm", "./package-managers/pnpm"); +injector.requirePublic("bun", "./package-managers/bun"); registerBuiltInCommand< typeof import("./common/commands/package-manager-get").packageManagerGetCommandDefinition >( @@ -404,7 +404,7 @@ registerBuiltInCommand< injector.require( "packageInstallationManager", - "./package-installation-manager", + "./package-managers/package-installation-manager", ); injector.require("deviceLogProvider", "./common/mobile/device-log-provider"); diff --git a/lib/base-package-manager.ts b/lib/package-managers/base-package-manager.ts similarity index 98% rename from lib/base-package-manager.ts rename to lib/package-managers/base-package-manager.ts index 5ee9a96abd..ab177a6a12 100644 --- a/lib/base-package-manager.ts +++ b/lib/package-managers/base-package-manager.ts @@ -1,17 +1,17 @@ -import { isInteractive } from "./common/helpers"; +import { isInteractive } from "../common/helpers"; import { INodePackageManager, INodePackageManagerInstallOptions, INpmInstallResultInfo, INpmsResult, INpmPackageNameParts, -} from "./declarations"; +} from "../declarations"; import { IDictionary, IChildProcess, IFileSystem, IHostInfo, -} from "./common/declarations"; +} from "../common/declarations"; export abstract class BasePackageManager implements INodePackageManager { public abstract install( diff --git a/lib/bun-package-manager.ts b/lib/package-managers/bun.ts similarity index 93% rename from lib/bun-package-manager.ts rename to lib/package-managers/bun.ts index cfd5ffc057..0bf36bb0d7 100644 --- a/lib/bun-package-manager.ts +++ b/lib/package-managers/bun.ts @@ -1,23 +1,23 @@ import * as path from "path"; import { BasePackageManager } from "./base-package-manager"; -import { exported, cache } from "./common/decorators"; -import { CACACHE_DIRECTORY_NAME } from "./constants"; +import { exported, cache } from "../common/decorators"; +import { CACACHE_DIRECTORY_NAME } from "../constants"; import * as _ from "lodash"; import { INodePackageManagerInstallOptions, INpmInstallResultInfo, INpmsResult, -} from "./declarations"; +} from "../declarations"; import { IChildProcess, IErrors, IFileSystem, IHostInfo, Server, -} from "./common/declarations"; -import { injector } from "./common/yok"; +} from "../common/declarations"; +import { injector } from "../common/yok"; -export class BunPackageManager extends BasePackageManager { +export class Bun extends BasePackageManager { constructor( $childProcess: IChildProcess, private $errors: IErrors, @@ -152,4 +152,4 @@ export class BunPackageManager extends BasePackageManager { } } -injector.register("bun", BunPackageManager); +injector.register("bun", Bun); diff --git a/lib/package-manager.ts b/lib/package-managers/index.ts similarity index 93% rename from lib/package-manager.ts rename to lib/package-managers/index.ts index df6d18aa92..3230e14bd8 100644 --- a/lib/package-manager.ts +++ b/lib/package-managers/index.ts @@ -1,6 +1,6 @@ -import { cache, exported, invokeInit } from "./common/decorators"; -import { performanceLog } from "./common/decorators"; -import { PackageManagers } from "./constants"; +import { cache, exported, invokeInit } from "../common/decorators"; +import { performanceLog } from "../common/decorators"; +import { PackageManagers } from "../constants"; import { IPackageManager, INodePackageManager, @@ -9,14 +9,14 @@ import { INpmInstallResultInfo, INpmsResult, INpmPackageNameParts, -} from "./declarations"; +} from "../declarations"; import { IErrors, IUserSettingsService, IDictionary, -} from "./common/declarations"; -import { injector } from "./common/yok"; -import { IProjectConfigService } from "./definitions/project"; +} from "../common/declarations"; +import { injector } from "../common/yok"; +import { IProjectConfigService } from "../definitions/project"; export class PackageManager implements IPackageManager { private packageManager: INodePackageManager; private _packageManagerName: string; diff --git a/lib/node-package-manager.ts b/lib/package-managers/npm.ts similarity index 94% rename from lib/node-package-manager.ts rename to lib/package-managers/npm.ts index bf63cae523..1e508f7c27 100644 --- a/lib/node-package-manager.ts +++ b/lib/package-managers/npm.ts @@ -1,23 +1,23 @@ import { join, relative } from "path"; import { BasePackageManager } from "./base-package-manager"; -import { exported, cache } from "./common/decorators"; -import { CACACHE_DIRECTORY_NAME } from "./constants"; +import { exported, cache } from "../common/decorators"; +import { CACACHE_DIRECTORY_NAME } from "../constants"; import * as _ from "lodash"; import { INodePackageManagerInstallOptions, INpmInstallResultInfo, INpmsResult, -} from "./declarations"; +} from "../declarations"; import { IChildProcess, IErrors, IFileSystem, IHostInfo, Server, -} from "./common/declarations"; -import { injector } from "./common/yok"; +} from "../common/declarations"; +import { injector } from "../common/yok"; -export class NodePackageManager extends BasePackageManager { +export class NPM extends BasePackageManager { constructor( $childProcess: IChildProcess, private $errors: IErrors, @@ -172,4 +172,4 @@ export class NodePackageManager extends BasePackageManager { } } -injector.register("npm", NodePackageManager); +injector.register("npm", NPM); diff --git a/lib/package-installation-manager.ts b/lib/package-managers/package-installation-manager.ts similarity index 97% rename from lib/package-installation-manager.ts rename to lib/package-managers/package-installation-manager.ts index 535ff2d942..d34fe6290f 100644 --- a/lib/package-installation-manager.ts +++ b/lib/package-managers/package-installation-manager.ts @@ -1,20 +1,20 @@ import * as path from "path"; -import * as constants from "./constants"; +import * as constants from "../constants"; import { INpmInstallOptions, INpmInstallResultInfo, IPackageInstallationManager, IPackageManager, IStaticConfig, -} from "./declarations"; -import { IProjectDataService } from "./definitions/project"; +} from "../declarations"; +import { IProjectDataService } from "../definitions/project"; import { IChildProcess, IDictionary, IFileSystem, ISettingsService, -} from "./common/declarations"; -import { injector } from "./common/yok"; +} from "../common/declarations"; +import { injector } from "../common/yok"; import * as semver from "semver"; diff --git a/lib/pnpm-package-manager.ts b/lib/package-managers/pnpm.ts similarity index 95% rename from lib/pnpm-package-manager.ts rename to lib/package-managers/pnpm.ts index 5970d50c2e..c14814b1d2 100644 --- a/lib/pnpm-package-manager.ts +++ b/lib/package-managers/pnpm.ts @@ -1,13 +1,13 @@ import * as path from "path"; import * as _ from "lodash"; import { BasePackageManager } from "./base-package-manager"; -import { exported } from "./common/decorators"; -import { CACACHE_DIRECTORY_NAME } from "./constants"; +import { exported } from "../common/decorators"; +import { CACACHE_DIRECTORY_NAME } from "../constants"; import { INodePackageManagerInstallOptions, INpmInstallResultInfo, INpmsResult, -} from "./declarations"; +} from "../declarations"; import { IChildProcess, IErrors, @@ -15,10 +15,10 @@ import { IHostInfo, Server, IDictionary, -} from "./common/declarations"; -import { injector } from "./common/yok"; +} from "../common/declarations"; +import { injector } from "../common/yok"; -export class PnpmPackageManager extends BasePackageManager { +export class PNPM extends BasePackageManager { constructor( $childProcess: IChildProcess, private $errors: IErrors, @@ -199,4 +199,4 @@ export class PnpmPackageManager extends BasePackageManager { } } -injector.register("pnpm", PnpmPackageManager); +injector.register("pnpm", PNPM); diff --git a/lib/yarn-package-manager.ts b/lib/package-managers/yarn.ts similarity index 93% rename from lib/yarn-package-manager.ts rename to lib/package-managers/yarn.ts index d4d08ad7f0..253f5efbd5 100644 --- a/lib/yarn-package-manager.ts +++ b/lib/package-managers/yarn.ts @@ -1,12 +1,12 @@ import * as path from "path"; import * as _ from "lodash"; import { BasePackageManager } from "./base-package-manager"; -import { exported } from "./common/decorators"; +import { exported } from "../common/decorators"; import { INodePackageManagerInstallOptions, INpmInstallResultInfo, INpmsResult, -} from "./declarations"; +} from "../declarations"; import { IChildProcess, IErrors, @@ -14,10 +14,10 @@ import { IHostInfo, Server, IDictionary, -} from "./common/declarations"; -import { injector } from "./common/yok"; +} from "../common/declarations"; +import { injector } from "../common/yok"; -export class YarnPackageManager extends BasePackageManager { +export class Yarn extends BasePackageManager { constructor( $childProcess: IChildProcess, private $errors: IErrors, @@ -147,4 +147,4 @@ export class YarnPackageManager extends BasePackageManager { } } -injector.register("yarn", YarnPackageManager); +injector.register("yarn", Yarn); diff --git a/lib/yarn2-package-manager.ts b/lib/package-managers/yarn2.ts similarity index 94% rename from lib/yarn2-package-manager.ts rename to lib/package-managers/yarn2.ts index a8312abff3..d717cdf793 100644 --- a/lib/yarn2-package-manager.ts +++ b/lib/package-managers/yarn2.ts @@ -1,12 +1,12 @@ import * as path from "path"; import * as _ from "lodash"; import { BasePackageManager } from "./base-package-manager"; -import { exported } from "./common/decorators"; +import { exported } from "../common/decorators"; import { INodePackageManagerInstallOptions, INpmInstallResultInfo, INpmsResult, -} from "./declarations"; +} from "../declarations"; import { IChildProcess, IErrors, @@ -14,10 +14,10 @@ import { IHostInfo, Server, IDictionary, -} from "./common/declarations"; -import { injector } from "./common/yok"; +} from "../common/declarations"; +import { injector } from "../common/yok"; -export class Yarn2PackageManager extends BasePackageManager { +export class Yarn2 extends BasePackageManager { private $hostInfo_: IHostInfo; constructor( $childProcess: IChildProcess, @@ -165,4 +165,4 @@ export class Yarn2PackageManager extends BasePackageManager { } } -injector.register("yarn2", Yarn2PackageManager); +injector.register("yarn2", Yarn2); diff --git a/test/bun-package-manager.ts b/test/bun-package-manager.ts index 758b620f54..8569831ab2 100644 --- a/test/bun-package-manager.ts +++ b/test/bun-package-manager.ts @@ -1,7 +1,7 @@ import { Yok } from "../lib/common/yok"; import * as stubs from "./stubs"; import { assert } from "chai"; -import { BunPackageManager } from "../lib/bun-package-manager"; +import { Bun } from "../lib/package-managers/bun"; import { IInjector } from "../lib/common/definitions/yok"; function createTestInjector(configuration: {} = {}): IInjector { @@ -12,7 +12,7 @@ function createTestInjector(configuration: {} = {}): IInjector { injector.register("childProcess", stubs.ChildProcessStub); injector.register("httpClient", {}); injector.register("fs", stubs.FileSystemStub); - injector.register("bun", BunPackageManager); + injector.register("bun", Bun); injector.register("pacoteService", { manifest: () => Promise.resolve(), }); @@ -50,7 +50,7 @@ describe("node-package-manager", () => { ].forEach((testCase) => { it(testCase.name, async () => { const testInjector = createTestInjector(); - const npm = testInjector.resolve("bun"); + const npm = testInjector.resolve("bun"); const templateNameParts = await npm.getPackageNameParts( testCase.templateFullName ); @@ -85,7 +85,7 @@ describe("node-package-manager", () => { ].forEach((testCase) => { it(testCase.name, async () => { const testInjector = createTestInjector(); - const npm = testInjector.resolve("bun"); + const npm = testInjector.resolve("bun"); const templateFullName = await npm.getPackageFullName({ name: testCase.templateName, version: testCase.templateVersion, diff --git a/test/controllers/add-platform-controller.ts b/test/controllers/add-platform-controller.ts index f7680f789f..fa869f692e 100644 --- a/test/controllers/add-platform-controller.ts +++ b/test/controllers/add-platform-controller.ts @@ -5,12 +5,12 @@ import { assert } from "chai"; import { format } from "util"; import * as _ from "lodash"; import { AddPlaformErrors } from "../../lib/constants"; -import { PackageManager } from "../../lib/package-manager"; -import { NodePackageManager } from "../../lib/node-package-manager"; -import { YarnPackageManager } from "../../lib/yarn-package-manager"; -import { Yarn2PackageManager } from "../../lib/yarn2-package-manager"; -import { PnpmPackageManager } from "../../lib/pnpm-package-manager"; -import { BunPackageManager } from "../../lib/bun-package-manager"; +import { PackageManager } from "../../lib/package-managers"; +import { NPM } from "../../lib/package-managers/npm"; +import { Yarn } from "../../lib/package-managers/yarn"; +import { Yarn2 } from "../../lib/package-managers/yarn2"; +import { PNPM } from "../../lib/package-managers/pnpm"; +import { Bun } from "../../lib/package-managers/bun"; import { MobileHelper } from "../../lib/common/mobile/mobile-helper"; let actualMessage: string = null; @@ -29,11 +29,11 @@ function createInjector(data?: { latestFrameworkVersion: string }) { trackEventActionInGoogleAnalytics: () => ({}), }); injector.register("packageManager", PackageManager); - injector.register("npm", NodePackageManager); - injector.register("yarn", YarnPackageManager); - injector.register("yarn2", Yarn2PackageManager); - injector.register("pnpm", PnpmPackageManager); - injector.register("bun", BunPackageManager); + injector.register("npm", NPM); + injector.register("yarn", Yarn); + injector.register("yarn2", Yarn2); + injector.register("pnpm", PNPM); + injector.register("bun", Bun); injector.register("userSettingsService", { getSettingValue: async (settingName: string): Promise => undefined, diff --git a/test/ios-project-service.ts b/test/ios-project-service.ts index e55c28f3f2..399557963e 100644 --- a/test/ios-project-service.ts +++ b/test/ios-project-service.ts @@ -24,9 +24,9 @@ import { IOSDeviceDiscovery } from "../lib/common/mobile/mobile-core/ios-device- import { AndroidDeviceDiscovery } from "../lib/common/mobile/mobile-core/android-device-discovery"; import { Utils } from "../lib/common/utils"; import { CocoaPodsService } from "../lib/services/cocoapods-service"; -import { PackageManager } from "../lib/package-manager"; -import { NodePackageManager } from "../lib/node-package-manager"; -import { YarnPackageManager } from "../lib/yarn-package-manager"; +import { PackageManager } from "../lib/package-managers"; +import { NPM } from "../lib/package-managers/npm"; +import { Yarn } from "../lib/package-managers/yarn"; import { assert } from "chai"; import { SettingsService } from "../lib/common/test/unit-tests/stubs"; @@ -183,8 +183,8 @@ function createTestInjector( }); testInjector.register("packageManager", PackageManager); testInjector.register("projectConfigService", ProjectConfigServiceStub); - testInjector.register("npm", NodePackageManager); - testInjector.register("yarn", YarnPackageManager); + testInjector.register("npm", NPM); + testInjector.register("yarn", Yarn); testInjector.register("xcconfigService", XcconfigService); testInjector.register("settingsService", SettingsService); testInjector.register("httpClient", {}); diff --git a/test/node-package-manager.ts b/test/node-package-manager.ts index 27efb270f3..79af0c71d0 100644 --- a/test/node-package-manager.ts +++ b/test/node-package-manager.ts @@ -1,7 +1,7 @@ import { Yok } from "../lib/common/yok"; import * as stubs from "./stubs"; import { assert } from "chai"; -import { NodePackageManager } from "../lib/node-package-manager"; +import { NPM } from "../lib/package-managers/npm"; import { IInjector } from "../lib/common/definitions/yok"; function createTestInjector(configuration: {} = {}): IInjector { @@ -12,7 +12,7 @@ function createTestInjector(configuration: {} = {}): IInjector { injector.register("childProcess", stubs.ChildProcessStub); injector.register("httpClient", {}); injector.register("fs", stubs.FileSystemStub); - injector.register("npm", NodePackageManager); + injector.register("npm", NPM); injector.register("pacoteService", { manifest: () => Promise.resolve(), }); @@ -52,7 +52,7 @@ describe("node-package-manager", () => { ].forEach((testCase) => { it(testCase.name, async () => { const testInjector = createTestInjector(); - const npm = testInjector.resolve("npm"); + const npm = testInjector.resolve("npm"); const templateNameParts = await npm.getPackageNameParts( testCase.templateFullName ); @@ -87,7 +87,7 @@ describe("node-package-manager", () => { ].forEach((testCase) => { it(testCase.name, async () => { const testInjector = createTestInjector(); - const npm = testInjector.resolve("npm"); + const npm = testInjector.resolve("npm"); const templateFullName = await npm.getPackageFullName({ name: testCase.templateName, version: testCase.templateVersion, diff --git a/test/package-installation-manager.ts b/test/package-installation-manager.ts index ca2cbe409e..c1a5a462a6 100644 --- a/test/package-installation-manager.ts +++ b/test/package-installation-manager.ts @@ -4,13 +4,13 @@ import * as ErrorsLib from "../lib/common/errors"; import * as FsLib from "../lib/common/file-system"; import * as HostInfoLib from "../lib/common/host-info"; import * as LoggerLib from "../lib/common/logger/logger"; -import * as NpmLib from "../lib/node-package-manager"; -import * as YarnLib from "../lib/yarn-package-manager"; -import * as Yarn2Lib from "../lib/yarn2-package-manager"; -import * as PnpmLib from "../lib/pnpm-package-manager"; -import * as BunLib from "../lib/bun-package-manager"; -import * as PackageManagerLib from "../lib/package-manager"; -import * as PackageInstallationManagerLib from "../lib/package-installation-manager"; +import * as NpmLib from "../lib/package-managers/npm"; +import * as YarnLib from "../lib/package-managers/yarn"; +import * as Yarn2Lib from "../lib/package-managers/yarn2"; +import * as PnpmLib from "../lib/package-managers/pnpm"; +import * as BunLib from "../lib/package-managers/bun"; +import * as PackageManagerLib from "../lib/package-managers"; +import * as PackageInstallationManagerLib from "../lib/package-managers/package-installation-manager"; import * as OptionsLib from "../lib/options"; import * as StaticConfigLib from "../lib/config"; import * as yok from "../lib/common/yok"; @@ -46,11 +46,11 @@ function createTestInjector(): IInjector { testInjector.register("userSettingsService", { getSettingValue: async (settingName: string): Promise => undefined, }); - testInjector.register("npm", NpmLib.NodePackageManager); - testInjector.register("yarn", YarnLib.YarnPackageManager); - testInjector.register("yarn2", Yarn2Lib.Yarn2PackageManager); - testInjector.register("pnpm", PnpmLib.PnpmPackageManager); - testInjector.register("bun", BunLib.BunPackageManager); + testInjector.register("npm", NpmLib.NPM); + testInjector.register("yarn", YarnLib.Yarn); + testInjector.register("yarn2", Yarn2Lib.Yarn2); + testInjector.register("pnpm", PnpmLib.PNPM); + testInjector.register("bun", BunLib.Bun); testInjector.register("packageManager", PackageManagerLib.PackageManager); testInjector.register("projectConfigService", ProjectConfigServiceStub); testInjector.register( diff --git a/test/plugins-service.ts b/test/plugins-service.ts index edf62e97b0..3372674ed3 100644 --- a/test/plugins-service.ts +++ b/test/plugins-service.ts @@ -1,12 +1,12 @@ import { Yok } from "../lib/common/yok"; import * as stubs from "./stubs"; -import { PackageManager } from "../lib/package-manager"; -import { PackageInstallationManager } from "../lib/package-installation-manager"; -import { NodePackageManager } from "../lib/node-package-manager"; -import { YarnPackageManager } from "../lib/yarn-package-manager"; -import { Yarn2PackageManager } from "../lib/yarn2-package-manager"; -import { PnpmPackageManager } from "../lib/pnpm-package-manager"; -import { BunPackageManager } from "../lib/bun-package-manager"; +import { PackageManager } from "../lib/package-managers"; +import { PackageInstallationManager } from "../lib/package-managers/package-installation-manager"; +import { NPM } from "../lib/package-managers/npm"; +import { Yarn } from "../lib/package-managers/yarn"; +import { Yarn2 } from "../lib/package-managers/yarn2"; +import { PNPM } from "../lib/package-managers/pnpm"; +import { Bun } from "../lib/package-managers/bun"; import { ProjectData } from "../lib/project-data"; import { ChildProcess } from "../lib/common/child-process"; import { Options } from "../lib/options"; @@ -75,11 +75,11 @@ function createTestInjector() { "projectConfigService", stubs.PackageInstallationManagerStub, ); - testInjector.register("npm", NodePackageManager); - testInjector.register("yarn", YarnPackageManager); - testInjector.register("yarn2", Yarn2PackageManager); - testInjector.register("pnpm", PnpmPackageManager); - testInjector.register("bun", BunPackageManager); + testInjector.register("npm", NPM); + testInjector.register("yarn", Yarn); + testInjector.register("yarn2", Yarn2); + testInjector.register("pnpm", PNPM); + testInjector.register("bun", Bun); testInjector.register("fs", FileSystem); // const fileSystemStub = new stubs.FileSystemStub(); // fileSystemStub.exists = (fileName: string) => { diff --git a/test/pnpm-package-manager.ts b/test/pnpm-package-manager.ts index 3c825f877a..6c6a4645bb 100644 --- a/test/pnpm-package-manager.ts +++ b/test/pnpm-package-manager.ts @@ -3,7 +3,7 @@ import { Yok } from "../lib/common/yok"; import * as stubs from "./stubs"; import { assert } from "chai"; import { setIsInteractive } from "../lib/common/helpers"; -import { PnpmPackageManager } from "../lib/pnpm-package-manager"; +import { PNPM } from "../lib/package-managers/pnpm"; import { IInjector } from "../lib/common/definitions/yok"; class RecordingChildProcessStub extends stubs.ChildProcessStub { @@ -66,7 +66,7 @@ function createTestInjector(): IInjector { injector.register("childProcess", RecordingChildProcessStub); injector.register("httpClient", {}); injector.register("fs", SelectiveFileSystemStub); - injector.register("pnpm", PnpmPackageManager); + injector.register("pnpm", PNPM); injector.register("pacoteService", { manifest: () => Promise.resolve({ name: "left-pad", version: "1.3.0" }), }); @@ -80,7 +80,7 @@ describe("pnpm-package-manager", () => { describe("install", () => { it("passes --shamefully-hoist when the project has no pnpm layout config", async () => { const testInjector = createTestInjector(); - const pnpm = testInjector.resolve("pnpm"); + const pnpm = testInjector.resolve("pnpm"); const childProcess = testInjector.resolve("childProcess"); @@ -94,7 +94,7 @@ describe("pnpm-package-manager", () => { it("omits --shamefully-hoist when a pnpm-workspace.yaml governs the project", async () => { const testInjector = createTestInjector(); - const pnpm = testInjector.resolve("pnpm"); + const pnpm = testInjector.resolve("pnpm"); const childProcess = testInjector.resolve("childProcess"); const fs = testInjector.resolve("fs"); @@ -107,7 +107,7 @@ describe("pnpm-package-manager", () => { it("omits --shamefully-hoist when an ancestor pnpm-workspace.yaml governs the project", async () => { const testInjector = createTestInjector(); - const pnpm = testInjector.resolve("pnpm"); + const pnpm = testInjector.resolve("pnpm"); const childProcess = testInjector.resolve("childProcess"); const fs = testInjector.resolve("fs"); @@ -120,7 +120,7 @@ describe("pnpm-package-manager", () => { it("omits --shamefully-hoist when an .npmrc sets a layout key", async () => { const testInjector = createTestInjector(); - const pnpm = testInjector.resolve("pnpm"); + const pnpm = testInjector.resolve("pnpm"); const childProcess = testInjector.resolve("childProcess"); const fs = testInjector.resolve("fs"); @@ -137,7 +137,7 @@ describe("pnpm-package-manager", () => { ["hoist-pattern[]", "public-hoist-pattern[]"].forEach((layoutKey) => { it(`omits --shamefully-hoist when an .npmrc sets array-valued ${layoutKey}`, async () => { const testInjector = createTestInjector(); - const pnpm = testInjector.resolve("pnpm"); + const pnpm = testInjector.resolve("pnpm"); const childProcess = testInjector.resolve("childProcess"); const fs = testInjector.resolve("fs"); @@ -154,7 +154,7 @@ describe("pnpm-package-manager", () => { it("keeps --shamefully-hoist when an .npmrc has no layout key", async () => { const testInjector = createTestInjector(); - const pnpm = testInjector.resolve("pnpm"); + const pnpm = testInjector.resolve("pnpm"); const childProcess = testInjector.resolve("childProcess"); const fs = testInjector.resolve("fs"); @@ -172,7 +172,7 @@ describe("pnpm-package-manager", () => { it("maps ignoreScripts to --ignore-scripts and drops internal options pnpm rejects", async () => { const testInjector = createTestInjector(); - const pnpm = testInjector.resolve("pnpm"); + const pnpm = testInjector.resolve("pnpm"); const childProcess = testInjector.resolve("childProcess"); @@ -191,7 +191,7 @@ describe("pnpm-package-manager", () => { it("spawns non-interactive installs with stdin closed", async () => { const testInjector = createTestInjector(); - const pnpm = testInjector.resolve("pnpm"); + const pnpm = testInjector.resolve("pnpm"); const childProcess = testInjector.resolve("childProcess"); @@ -213,7 +213,7 @@ describe("pnpm-package-manager", () => { it("appends the package name when installing a single package", async () => { const testInjector = createTestInjector(); - const pnpm = testInjector.resolve("pnpm"); + const pnpm = testInjector.resolve("pnpm"); const childProcess = testInjector.resolve("childProcess"); @@ -231,7 +231,7 @@ describe("pnpm-package-manager", () => { describe("getCachePath", () => { it("uses the configured cache directory when pnpm reports one", async () => { const testInjector = createTestInjector(); - const pnpm = testInjector.resolve("pnpm"); + const pnpm = testInjector.resolve("pnpm"); const childProcess = testInjector.resolve("childProcess"); childProcess.execResponses["pnpm config get cache"] = "/custom/cache\n"; @@ -243,7 +243,7 @@ describe("pnpm-package-manager", () => { 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 pnpm = testInjector.resolve("pnpm"); const childProcess = testInjector.resolve("childProcess"); childProcess.execResponses["pnpm config get cache"] = "undefined\n"; diff --git a/test/services/extensibility-service.ts b/test/services/extensibility-service.ts index a6b1970ee5..e5acaf36d8 100644 --- a/test/services/extensibility-service.ts +++ b/test/services/extensibility-service.ts @@ -2,12 +2,12 @@ import { ExtensibilityService } from "../../lib/services/extensibility-service"; import { Yok } from "../../lib/common/yok"; import * as stubs from "../stubs"; import { assert } from "chai"; -import { NodePackageManager } from "../../lib/node-package-manager"; -import { PackageManager } from "../../lib/package-manager"; -import { YarnPackageManager } from "../../lib/yarn-package-manager"; -import { Yarn2PackageManager } from "../../lib/yarn2-package-manager"; -import { PnpmPackageManager } from "../../lib/pnpm-package-manager"; -import { BunPackageManager } from "../../lib/bun-package-manager"; +import { NPM } from "../../lib/package-managers/npm"; +import { PackageManager } from "../../lib/package-managers"; +import { Yarn } from "../../lib/package-managers/yarn"; +import { Yarn2 } from "../../lib/package-managers/yarn2"; +import { PNPM } from "../../lib/package-managers/pnpm"; +import { Bun } from "../../lib/package-managers/bun"; import * as constants from "../../lib/constants"; import { ChildProcess } from "../../lib/common/child-process"; import { CommandsDelimiters } from "../../lib/common/constants"; @@ -75,11 +75,11 @@ describe("extensibilityService", () => { testInjector.register("userSettingsService", { getSettingValue: async (settingName: string): Promise => undefined, }); - testInjector.register("npm", NodePackageManager); - testInjector.register("yarn", YarnPackageManager); - testInjector.register("yarn2", Yarn2PackageManager); - testInjector.register("pnpm", PnpmPackageManager); - testInjector.register("bun", BunPackageManager); + testInjector.register("npm", NPM); + testInjector.register("yarn", Yarn); + testInjector.register("yarn2", Yarn2); + testInjector.register("pnpm", PNPM); + testInjector.register("bun", Bun); testInjector.register("settingsService", SettingsService); testInjector.register("requireService", { require: (pathToRequire: string): any => undefined, From 1157cae5d74da3670d0dcca57dd94839c20745ff Mon Sep 17 00:00:00 2001 From: Igor Randjelovic Date: Wed, 16 Sep 2026 11:28:12 +0200 Subject: [PATCH 2/6] refactor: type package manager install and uninstall options Replace the untyped npm flag bag passed to install/uninstall with IPackageInstallOptions and IPackageUninstallOptions (save, dev, optional, exact, silent, ignoreScripts plus the CLI-internal options). Each package manager declares how it spells each option, and options a manager has no flag for are dropped instead of leaking npm syntax onto its command line. This fixes yarn berry receiving --save-dev / --save-exact (silently dropped, so platforms landed in dependencies) and --ignore-scripts (an unknown option that aborted the install); it now gets --dev, --exact and --mode=skip-build. bun receives its own --dev / --exact instead of npm's. Also settle the implementation class names on NpmPackageManager, YarnPackageManager, Yarn2PackageManager, PnpmPackageManager and BunPackageManager. --- PublicAPI.md | 6 +- lib/commands/install.ts | 2 +- lib/commands/plugin/create-plugin.ts | 3 +- lib/commands/preview.ts | 6 +- lib/commands/test-init.ts | 9 +- lib/constants.ts | 7 - lib/contracts/package-manager.ts | 11 +- lib/declarations.d.ts | 51 +++- lib/package-managers/base-package-manager.ts | 56 +++- lib/package-managers/bun.ts | 34 ++- lib/package-managers/index.ts | 11 +- lib/package-managers/npm.ts | 40 ++- .../package-installation-manager.ts | 21 +- lib/package-managers/pnpm.ts | 37 ++- lib/package-managers/yarn.ts | 29 ++- lib/package-managers/yarn2.ts | 35 +-- lib/services/extensibility-service.ts | 10 +- lib/services/platform/add-platform-service.ts | 5 +- lib/services/plugins-service.ts | 4 +- test/bun-package-manager.ts | 8 +- test/controllers/add-platform-controller.ts | 20 +- test/ios-project-service.ts | 8 +- test/node-package-manager.ts | 8 +- test/package-installation-manager.ts | 10 +- test/package-manager-flags.ts | 241 ++++++++++++++++++ test/plugins-service.ts | 20 +- test/pnpm-package-manager.ts | 35 ++- test/project-templates-service.ts | 4 +- test/services/extensibility-service.ts | 29 ++- test/stubs.ts | 4 +- 30 files changed, 556 insertions(+), 208 deletions(-) create mode 100644 test/package-manager-flags.ts diff --git a/PublicAPI.md b/PublicAPI.md index b59cba2fe3..0d59215163 100644 --- a/PublicAPI.md +++ b/PublicAPI.md @@ -479,7 +479,7 @@ tns.settingsService.setSettings({ userAgentName: "myUserAgent", profileDir: "cus `npm` module provides a way to interact with npm specifically the use of install, uninstall, search and view commands. ### install -Installs specified package. Note that you can use the third argument in order to pass different options to the installation like `ignore-scripts`, `save` or `save-exact` which work exactly like they would if you would execute npm from the command line and pass them as `--` flags. +Installs specified package. The third argument takes package-manager-agnostic options (`dev`, `exact`, `save`, `optional`, `silent`, `ignoreScripts`); the selected package manager maps them onto its own command line flags. * Auxiliary interfaces: ```TypeScript /** @@ -533,11 +533,11 @@ Uninstalls a specified package. /** * Uninstalls a dependency * @param {string} packageName The name of the dependency. - * @param {IDictionary} config Additional options that can be passed to manipulate uninstallation. + * @param {IPackageUninstallOptions} options Package-manager-agnostic uninstallation options (`save`). * @param {string} path The destination of the uninstallation. * @return {Promise} The output of the uninstallation. */ -uninstall(packageName: string, config?: IDictionary, path?: string): Promise; +uninstall(packageName: string, options?: IPackageUninstallOptions, path?: string): Promise; ``` * Usage: diff --git a/lib/commands/install.ts b/lib/commands/install.ts index 69ab52f2b2..ba60986a2b 100644 --- a/lib/commands/install.ts +++ b/lib/commands/install.ts @@ -95,7 +95,7 @@ async function installModule( } await $packageManager.install(moduleName, projectDir, { - "save-dev": true, + dev: true, disableNpmInstall: context.options.disableNpmInstall, frameworkPath: context.options.frameworkPath, ignoreScripts: context.options.ignoreScripts, diff --git a/lib/commands/plugin/create-plugin.ts b/lib/commands/plugin/create-plugin.ts index 62ee77059e..a350d995a8 100644 --- a/lib/commands/plugin/create-plugin.ts +++ b/lib/commands/plugin/create-plugin.ts @@ -180,8 +180,7 @@ export class CreatePluginCommand extends Command({ const cwd = path.join(projectDir, "src"); try { spinner.start(); - const npmOptions: any = { silent: true }; - await this.$packageManager.install(cwd, cwd, npmOptions); + await this.$packageManager.install(cwd, cwd, { silent: true }); } finally { spinner.stop(); } diff --git a/lib/commands/preview.ts b/lib/commands/preview.ts index 0ef938950b..ce1444dd0b 100644 --- a/lib/commands/preview.ts +++ b/lib/commands/preview.ts @@ -53,9 +53,9 @@ export class PreviewCommand extends Command({ `${PREVIEW_CLI_PACKAGE}@latest`, this.$projectData.projectDir, { - "save-dev": true, - "save-exact": true, - } as any, + dev: true, + exact: true, + }, ); } diff --git a/lib/commands/test-init.ts b/lib/commands/test-init.ts index dc070f4e2d..9dd5879e3b 100644 --- a/lib/commands/test-init.ts +++ b/lib/commands/test-init.ts @@ -129,9 +129,8 @@ export class TestInitCommand extends Command({ await this.$packageManager.install(moduleToInstall, projectDir, { // Packages with native code must land in "dependencies" — the CLI // integrates plugin platform files (pods, aars) only from there. - ...(mod.saveInDependencies ? { save: true } : { "save-dev": true }), - "save-exact": true, - optional: false, + dev: !mod.saveInDependencies, + exact: true, disableNpmInstall: this.$options.disableNpmInstall, frameworkPath: this.$options.frameworkPath, ignoreScripts: this.$options.ignoreScripts, @@ -186,8 +185,8 @@ export class TestInitCommand extends Command({ `${peerDependency}@${dependencyVersion}`, projectDir, { - "save-dev": true, - "save-exact": true, + dev: true, + exact: true, disableNpmInstall: false, frameworkPath: this.$options.frameworkPath, ignoreScripts: this.$options.ignoreScripts, diff --git a/lib/constants.ts b/lib/constants.ts index 7420893fab..1c97df4171 100644 --- a/lib/constants.ts +++ b/lib/constants.ts @@ -126,13 +126,6 @@ export const TemplatesV2PackageJsonKeysToRemove: Array = [ "nativescript", ]; -export class SaveOptions { - static PRODUCTION = "save"; - static DEV = "save-dev"; - static OPTIONAL = "save-optional"; - static EXACT = "save-exact"; -} - export class ReleaseType { static MAJOR = "major"; static PREMAJOR = "premajor"; diff --git a/lib/contracts/package-manager.ts b/lib/contracts/package-manager.ts index cae1a8eed2..ef58e3e10f 100644 --- a/lib/contracts/package-manager.ts +++ b/lib/contracts/package-manager.ts @@ -1,7 +1,8 @@ import { Contract } from "../common/di/contract"; import type { IDictionary } from "../common/declarations"; import type { - INodePackageManagerInstallOptions, + IPackageInstallOptions, + IPackageUninstallOptions, INpmInstallResultInfo, INpmPackageNameParts, INpmsResult, @@ -17,25 +18,25 @@ export abstract class PackageManager { * Installs dependency * @param {string} packageName The name of the dependency - can be a path, a url or a string. * @param {string} pathToSave The destination of the installation. - * @param {INodePackageManagerInstallOptions} config Additional options that can be passed to manipulate installation. + * @param {IPackageInstallOptions} options Package-manager-agnostic installation options. * @return {Promise} Information about installed package. */ abstract install( packageName: string, pathToSave: string, - config: INodePackageManagerInstallOptions, + options: IPackageInstallOptions, ): Promise; /** * Uninstalls a dependency * @param {string} packageName The name of the dependency. - * @param {IDictionary} config Additional options that can be passed to manipulate uninstallation. + * @param {IPackageUninstallOptions} options Package-manager-agnostic uninstallation options. * @param {string} path The destination of the uninstallation. * @return {Promise} The output of the uninstallation. */ abstract uninstall( packageName: string, - config?: IDictionary, + options?: IPackageUninstallOptions, path?: string, ): Promise; diff --git a/lib/declarations.d.ts b/lib/declarations.d.ts index 466f3830fe..c51b2a8ce9 100644 --- a/lib/declarations.d.ts +++ b/lib/declarations.d.ts @@ -27,25 +27,25 @@ interface INodePackageManager { * Installs dependency * @param {string} packageName The name of the dependency - can be a path, a url or a string. * @param {string} pathToSave The destination of the installation. - * @param {INodePackageManagerInstallOptions} config Additional options that can be passed to manipulate installation. + * @param {IPackageInstallOptions} options Package-manager-agnostic installation options. * @return {Promise} Information about installed package. */ install( packageName: string, pathToSave: string, - config: INodePackageManagerInstallOptions, + options: IPackageInstallOptions, ): Promise; /** * Uninstalls a dependency * @param {string} packageName The name of the dependency. - * @param {IDictionary} config Additional options that can be passed to manipulate uninstallation. + * @param {IPackageUninstallOptions} options Package-manager-agnostic uninstallation options. * @param {string} path The destination of the uninstallation. * @return {Promise} The output of the uninstallation. */ uninstall( packageName: string, - config?: IDictionary, + options?: IPackageUninstallOptions, path?: string, ): Promise; @@ -167,18 +167,42 @@ interface IPackageInstallationManager { } /** - * Describes options that can be passed to manipulate package installation. + * Package-manager-agnostic installation options. Each package manager maps + * these onto its own command line flags; options a manager has no flag for + * are dropped rather than passed through. */ -interface INodePackageManagerInstallOptions - extends INpmInstallConfigurationOptions, IDictionary { - /** - * Destination of the installation. - * @type {string} - * @optional - */ +interface IPackageInstallOptions { + /** + * Record the package in package.json. Every supported package manager + * does this by default, so only `false` changes behaviour. + */ + save?: boolean; + /** Record the package under devDependencies. */ + dev?: boolean; + /** Record the package under optionalDependencies. */ + optional?: boolean; + /** Pin the exact resolved version instead of a semver range. */ + exact?: boolean; + /** Suppress the package manager's own output. */ + silent?: boolean; + /** Do not run lifecycle scripts. */ + ignoreScripts?: boolean; + /** Skip the installation entirely (the --disable-npm-install CLI flag). */ + disableNpmInstall?: boolean; + /** Local runtime location (the --frameworkPath CLI flag). */ + frameworkPath?: string; + /** Destination of the installation (the --path CLI flag). */ path?: string; } +/** + * Package-manager-agnostic uninstallation options. + */ +interface IPackageUninstallOptions { + /** Remove the package from package.json. */ + save?: boolean; +} + /** * Describes information about dependency packages. */ @@ -396,7 +420,8 @@ interface INpmInstallResultInfo { interface INpmInstallOptions { pathToSave?: string; version?: string; - dependencyType?: string; + /** Record the package under devDependencies. */ + dev?: boolean; } /** diff --git a/lib/package-managers/base-package-manager.ts b/lib/package-managers/base-package-manager.ts index ab177a6a12..789b9c7ab2 100644 --- a/lib/package-managers/base-package-manager.ts +++ b/lib/package-managers/base-package-manager.ts @@ -1,7 +1,8 @@ import { isInteractive } from "../common/helpers"; import { INodePackageManager, - INodePackageManagerInstallOptions, + IPackageInstallOptions, + IPackageUninstallOptions, INpmInstallResultInfo, INpmsResult, INpmPackageNameParts, @@ -13,15 +14,33 @@ import { IHostInfo, } from "../common/declarations"; +/** + * How one package manager spells each IPackageInstallOptions flag on its + * command line. A missing entry means the manager has no such flag and the + * option is dropped rather than passed through. + */ +export interface IPackageManagerFlags { + save?: string; + noSave?: string; + dev?: string; + optional?: string; + exact?: string; + silent?: string; + ignoreScripts?: string; +} + export abstract class BasePackageManager implements INodePackageManager { + protected abstract readonly installFlags: IPackageManagerFlags; + protected abstract readonly uninstallFlags: IPackageManagerFlags; + public abstract install( packageName: string, pathToSave: string, - config: INodePackageManagerInstallOptions, + options: IPackageInstallOptions, ): Promise; public abstract uninstall( packageName: string, - config?: IDictionary, + options?: IPackageUninstallOptions, path?: string, ): Promise; public abstract view(packageName: string, config: Object): Promise; @@ -133,6 +152,37 @@ export abstract class BasePackageManager implements INodePackageManager { }; } + protected getInstallFlags(options: IPackageInstallOptions): string[] { + return this.mapFlags(options, this.installFlags); + } + + protected getUninstallFlags(options: IPackageUninstallOptions): string[] { + return this.mapFlags(options, this.uninstallFlags); + } + + private mapFlags( + options: IPackageInstallOptions, + flags: IPackageManagerFlags, + ): string[] { + const result: string[] = []; + if (!options) { + return result; + } + const push = (flag?: string) => { + if (flag) { + result.push(flag); + } + }; + if (options.save === true) push(flags.save); + if (options.save === false) push(flags.noSave); + if (options.dev) push(flags.dev); + if (options.optional) push(flags.optional); + if (options.exact) push(flags.exact); + if (options.silent) push(flags.silent); + if (options.ignoreScripts) push(flags.ignoreScripts); + return result; + } + protected getFlagsString(config: any, asArray: boolean): any { const array: Array = []; for (const flag in config) { diff --git a/lib/package-managers/bun.ts b/lib/package-managers/bun.ts index 0bf36bb0d7..ddb46464b4 100644 --- a/lib/package-managers/bun.ts +++ b/lib/package-managers/bun.ts @@ -4,7 +4,8 @@ import { exported, cache } from "../common/decorators"; import { CACACHE_DIRECTORY_NAME } from "../constants"; import * as _ from "lodash"; import { - INodePackageManagerInstallOptions, + IPackageInstallOptions, + IPackageUninstallOptions, INpmInstallResultInfo, INpmsResult, } from "../declarations"; @@ -17,7 +18,21 @@ import { } from "../common/declarations"; import { injector } from "../common/yok"; -export class Bun extends BasePackageManager { +export class BunPackageManager extends BasePackageManager { + protected readonly installFlags = { + save: "--save", + noSave: "--no-save", + dev: "--dev", + optional: "--optional", + exact: "--exact", + silent: "--silent", + ignoreScripts: "--ignore-scripts", + }; + protected readonly uninstallFlags = { + save: "--save", + noSave: "--no-save", + }; + constructor( $childProcess: IChildProcess, private $errors: IErrors, @@ -34,19 +49,16 @@ export class Bun extends BasePackageManager { public async install( packageName: string, pathToSave: string, - config: INodePackageManagerInstallOptions + options: IPackageInstallOptions ): Promise { - if (config.disableNpmInstall) { + if (options.disableNpmInstall) { return; } - if (config.ignoreScripts) { - config["ignore-scripts"] = true; - } const packageJsonPath = path.join(pathToSave, "package.json"); const jsonContentBefore = this.$fs.readJson(packageJsonPath); - const flags = this.getFlagsString(config, true); + const flags = this.getInstallFlags(options); let params = ["install"]; const isInstallingAllDependencies = packageName === pathToSave; if (!isInstallingAllDependencies) { @@ -73,10 +85,10 @@ export class Bun extends BasePackageManager { @exported("bun") public async uninstall( packageName: string, - config?: any, + options?: IPackageUninstallOptions, cwd?: string ): Promise { - const flags = this.getFlagsString(config, false); + const flags = this.getUninstallFlags(options).join(" "); return this.$childProcess.exec(`bun remove ${packageName} ${flags}`, { cwd, }); @@ -152,4 +164,4 @@ export class Bun extends BasePackageManager { } } -injector.register("bun", Bun); +injector.register("bun", BunPackageManager); diff --git a/lib/package-managers/index.ts b/lib/package-managers/index.ts index 3230e14bd8..668f30e9cb 100644 --- a/lib/package-managers/index.ts +++ b/lib/package-managers/index.ts @@ -5,7 +5,8 @@ import { IPackageManager, INodePackageManager, IOptions, - INodePackageManagerInstallOptions, + IPackageInstallOptions, + IPackageUninstallOptions, INpmInstallResultInfo, INpmsResult, INpmPackageNameParts, @@ -50,18 +51,18 @@ export class PackageManager implements IPackageManager { public install( packageName: string, pathToSave: string, - config: INodePackageManagerInstallOptions + options: IPackageInstallOptions ): Promise { - return this.packageManager.install(packageName, pathToSave, config); + return this.packageManager.install(packageName, pathToSave, options); } @exported("packageManager") @invokeInit() public uninstall( packageName: string, - config?: IDictionary, + options?: IPackageUninstallOptions, path?: string ): Promise { - return this.packageManager.uninstall(packageName, config, path); + return this.packageManager.uninstall(packageName, options, path); } @exported("packageManager") @invokeInit() diff --git a/lib/package-managers/npm.ts b/lib/package-managers/npm.ts index 1e508f7c27..839ffae6d4 100644 --- a/lib/package-managers/npm.ts +++ b/lib/package-managers/npm.ts @@ -4,7 +4,8 @@ import { exported, cache } from "../common/decorators"; import { CACACHE_DIRECTORY_NAME } from "../constants"; import * as _ from "lodash"; import { - INodePackageManagerInstallOptions, + IPackageInstallOptions, + IPackageUninstallOptions, INpmInstallResultInfo, INpmsResult, } from "../declarations"; @@ -17,7 +18,21 @@ import { } from "../common/declarations"; import { injector } from "../common/yok"; -export class NPM extends BasePackageManager { +export class NpmPackageManager extends BasePackageManager { + protected readonly installFlags = { + save: "--save", + noSave: "--no-save", + dev: "--save-dev", + optional: "--save-optional", + exact: "--save-exact", + silent: "--silent", + ignoreScripts: "--ignore-scripts", + }; + protected readonly uninstallFlags = { + save: "--save", + noSave: "--no-save", + }; + constructor( $childProcess: IChildProcess, private $errors: IErrors, @@ -34,19 +49,16 @@ export class NPM extends BasePackageManager { public async install( packageName: string, pathToSave: string, - config: INodePackageManagerInstallOptions + options: IPackageInstallOptions ): Promise { - if (config.disableNpmInstall) { + if (options.disableNpmInstall) { return; } - if (config.ignoreScripts) { - config["ignore-scripts"] = true; - } const packageJsonPath = join(pathToSave, "package.json"); const jsonContentBefore = this.$fs.readJson(packageJsonPath); - const flags = this.getFlagsString(config, true); + const flags = this.getInstallFlags(options); let params = ["install"]; const isInstallingAllDependencies = packageName === pathToSave; if (!isInstallingAllDependencies) { @@ -62,11 +74,11 @@ export class NPM extends BasePackageManager { const etcExistsPriorToInstallation = this.$fs.exists(etcDirectoryLocation); //TODO: plamen5kov: workaround is here for a reason (remove whole file later) - if (config.path) { + if (options.path) { let relativePathFromCwdToSource = ""; - if (config.frameworkPath) { + if (options.frameworkPath) { relativePathFromCwdToSource = relative( - config.frameworkPath, + options.frameworkPath, pathToSave ); if (this.$fs.exists(relativePathFromCwdToSource)) { @@ -103,10 +115,10 @@ export class NPM extends BasePackageManager { @exported("npm") public async uninstall( packageName: string, - config?: any, + options?: IPackageUninstallOptions, path?: string ): Promise { - const flags = this.getFlagsString(config, false); + const flags = this.getUninstallFlags(options).join(" "); return this.$childProcess.exec(`npm uninstall ${packageName} ${flags}`, { cwd: path, }); @@ -172,4 +184,4 @@ export class NPM extends BasePackageManager { } } -injector.register("npm", NPM); +injector.register("npm", NpmPackageManager); diff --git a/lib/package-managers/package-installation-manager.ts b/lib/package-managers/package-installation-manager.ts index d34fe6290f..edc5f95250 100644 --- a/lib/package-managers/package-installation-manager.ts +++ b/lib/package-managers/package-installation-manager.ts @@ -3,6 +3,7 @@ import * as constants from "../constants"; import { INpmInstallOptions, INpmInstallResultInfo, + IPackageInstallOptions, IPackageInstallationManager, IPackageManager, IStaticConfig, @@ -152,13 +153,13 @@ export class PackageInstallationManager implements IPackageInstallationManager { try { const pathToSave = projectDir; const version = (opts && opts.version) || null; - const dependencyType = (opts && opts.dependencyType) || null; + const dev = !!(opts && opts.dev); return await this.installCore( packageToInstall, pathToSave, version, - dependencyType + dev ); } catch (error) { this.$logger.trace(error); @@ -277,7 +278,7 @@ export class PackageInstallationManager implements IPackageInstallationManager { packageName: string, pathToSave: string, version: string, - dependencyType: string + dev: boolean ): Promise { const possiblePackageName = path.resolve(packageName); if (this.$fs.exists(possiblePackageName)) { @@ -290,7 +291,7 @@ export class PackageInstallationManager implements IPackageInstallationManager { packageName, pathToSave, version, - dependencyType + dev ); const installedPackageName = installResultInfo.name; @@ -307,17 +308,17 @@ export class PackageInstallationManager implements IPackageInstallationManager { packageName: string, pathToSave: string, version: string, - dependencyType: string + dev: boolean ): Promise { this.$logger.info(`Installing ${packageName}`); packageName = packageName + (version ? `@${version}` : ""); - const npmOptions: any = { silent: true, "save-exact": true }; - - if (dependencyType) { - npmOptions[dependencyType] = true; - } + const npmOptions: IPackageInstallOptions = { + silent: true, + exact: true, + dev, + }; return await this.$packageManager.install( packageName, diff --git a/lib/package-managers/pnpm.ts b/lib/package-managers/pnpm.ts index c14814b1d2..137f26c556 100644 --- a/lib/package-managers/pnpm.ts +++ b/lib/package-managers/pnpm.ts @@ -4,7 +4,8 @@ import { BasePackageManager } from "./base-package-manager"; import { exported } from "../common/decorators"; import { CACACHE_DIRECTORY_NAME } from "../constants"; import { - INodePackageManagerInstallOptions, + IPackageInstallOptions, + IPackageUninstallOptions, INpmInstallResultInfo, INpmsResult, } from "../declarations"; @@ -18,7 +19,16 @@ import { } from "../common/declarations"; import { injector } from "../common/yok"; -export class PNPM extends BasePackageManager { +export class PnpmPackageManager extends BasePackageManager { + protected readonly installFlags = { + dev: "--save-dev", + optional: "--save-optional", + exact: "--save-exact", + silent: "--silent", + ignoreScripts: "--ignore-scripts", + }; + protected readonly uninstallFlags = {}; + constructor( $childProcess: IChildProcess, private $errors: IErrors, @@ -35,25 +45,16 @@ export class PNPM extends BasePackageManager { public async install( packageName: string, pathToSave: string, - config: INodePackageManagerInstallOptions, + options: IPackageInstallOptions, ): Promise { - if (config.disableNpmInstall) { + if (options.disableNpmInstall) { return; } - delete config.dev; // temporary fix for unsupported yarn flag - 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); + const flags = this.getInstallFlags(options); let params = ["i"]; if (!this.projectManagesOwnHoisting(pathToSave)) { // With pnpm's default isolated layout some imports won't be found, so @@ -87,12 +88,10 @@ export class PNPM extends BasePackageManager { @exported("pnpm") public uninstall( packageName: string, - config?: IDictionary, + options?: IPackageUninstallOptions, cwd?: string, ): Promise { - // pnpm does not want save option in remove. It saves it by default - delete config["save"]; - const flags = this.getFlagsString(config, false); + const flags = this.getUninstallFlags(options).join(" "); return this.$childProcess.exec(`pnpm remove ${packageName} ${flags}`, { cwd, }); @@ -199,4 +198,4 @@ export class PNPM extends BasePackageManager { } } -injector.register("pnpm", PNPM); +injector.register("pnpm", PnpmPackageManager); diff --git a/lib/package-managers/yarn.ts b/lib/package-managers/yarn.ts index 253f5efbd5..dd1b916b66 100644 --- a/lib/package-managers/yarn.ts +++ b/lib/package-managers/yarn.ts @@ -3,7 +3,8 @@ import * as _ from "lodash"; import { BasePackageManager } from "./base-package-manager"; import { exported } from "../common/decorators"; import { - INodePackageManagerInstallOptions, + IPackageInstallOptions, + IPackageUninstallOptions, INpmInstallResultInfo, INpmsResult, } from "../declarations"; @@ -17,7 +18,16 @@ import { } from "../common/declarations"; import { injector } from "../common/yok"; -export class Yarn extends BasePackageManager { +export class YarnPackageManager extends BasePackageManager { + protected readonly installFlags = { + dev: "--dev", + optional: "--optional", + exact: "--exact", + silent: "--silent", + ignoreScripts: "--ignore-scripts", + }; + protected readonly uninstallFlags = {}; + constructor( $childProcess: IChildProcess, private $errors: IErrors, @@ -34,19 +44,16 @@ export class Yarn extends BasePackageManager { public async install( packageName: string, pathToSave: string, - config: INodePackageManagerInstallOptions + options: IPackageInstallOptions ): Promise { - if (config.disableNpmInstall) { + if (options.disableNpmInstall) { return; } - if (config.ignoreScripts) { - config["ignore-scripts"] = true; - } const packageJsonPath = path.join(pathToSave, "package.json"); const jsonContentBefore = this.$fs.readJson(packageJsonPath); - const flags = this.getFlagsString(config, true); + const flags = this.getInstallFlags(options); let params = []; const isInstallingAllDependencies = packageName === pathToSave; if (!isInstallingAllDependencies) { @@ -72,10 +79,10 @@ export class Yarn extends BasePackageManager { @exported("yarn") public uninstall( packageName: string, - config?: IDictionary, + options?: IPackageUninstallOptions, cwd?: string ): Promise { - const flags = this.getFlagsString(config, false); + const flags = this.getUninstallFlags(options).join(" "); return this.$childProcess.exec(`yarn remove ${packageName} ${flags}`, { cwd, }); @@ -147,4 +154,4 @@ export class Yarn extends BasePackageManager { } } -injector.register("yarn", Yarn); +injector.register("yarn", YarnPackageManager); diff --git a/lib/package-managers/yarn2.ts b/lib/package-managers/yarn2.ts index d717cdf793..1e4d1736aa 100644 --- a/lib/package-managers/yarn2.ts +++ b/lib/package-managers/yarn2.ts @@ -3,7 +3,8 @@ import * as _ from "lodash"; import { BasePackageManager } from "./base-package-manager"; import { exported } from "../common/decorators"; import { - INodePackageManagerInstallOptions, + IPackageInstallOptions, + IPackageUninstallOptions, INpmInstallResultInfo, INpmsResult, } from "../declarations"; @@ -17,7 +18,18 @@ import { } from "../common/declarations"; import { injector } from "../common/yok"; -export class Yarn2 extends BasePackageManager { +export class Yarn2PackageManager extends BasePackageManager { + protected readonly installFlags = { + dev: "--dev", + optional: "--optional", + exact: "--exact", + silent: "--silent", + // yarn berry has no --ignore-scripts; skip-build is the mode that + // installs without running any build scripts. + ignoreScripts: "--mode=skip-build", + }; + protected readonly uninstallFlags = {}; + private $hostInfo_: IHostInfo; constructor( $childProcess: IChildProcess, @@ -46,23 +58,16 @@ export class Yarn2 extends BasePackageManager { public async install( packageName: string, pathToSave: string, - config: INodePackageManagerInstallOptions + options: IPackageInstallOptions ): Promise { - if (config.disableNpmInstall) { + if (options.disableNpmInstall) { return; } - if (config.ignoreScripts) { - config["ignore-scripts"] = true; - } const packageJsonPath = path.join(pathToSave, "package.json"); const jsonContentBefore = this.$fs.readJson(packageJsonPath); - // remove unsupported flags - // todo: refactor all package managers to map typed flags to the actual flags - const cleanedConfig = _.omit(config, ["save-dev", "save-exact"]); - - const flags = this.getFlagsString(cleanedConfig, true); + const flags = this.getInstallFlags(options); let params = []; const isInstallingAllDependencies = packageName === pathToSave; if (!isInstallingAllDependencies) { @@ -88,10 +93,10 @@ export class Yarn2 extends BasePackageManager { @exported("yarn2") public uninstall( packageName: string, - config?: IDictionary, + options?: IPackageUninstallOptions, cwd?: string ): Promise { - const flags = this.getFlagsString(config, false); + const flags = this.getUninstallFlags(options).join(" "); return this.$childProcess.exec(`yarn remove ${packageName} ${flags}`, { cwd, }); @@ -165,4 +170,4 @@ export class Yarn2 extends BasePackageManager { } } -injector.register("yarn2", Yarn2); +injector.register("yarn2", Yarn2PackageManager); diff --git a/lib/services/extensibility-service.ts b/lib/services/extensibility-service.ts index 9c2884c534..4a9b123c9d 100644 --- a/lib/services/extensibility-service.ts +++ b/lib/services/extensibility-service.ts @@ -4,7 +4,11 @@ import { cache } from "../common/decorators"; import * as constants from "../constants"; import { createRegExp, regExpEscape } from "../common/helpers"; import { reportDeprecation } from "../common/deprecation"; -import { INodePackageManager, INpmsSingleResultData } from "../declarations"; +import { + INodePackageManager, + INpmsSingleResultData, + IPackageInstallOptions, +} from "../declarations"; import { IDictionary, IFileSystem, @@ -119,9 +123,9 @@ export class ExtensibilityService implements IExtensibilityService { await this.assertPackageJsonExists(); - const npmOpts: any = { + const npmOpts: IPackageInstallOptions = { save: true, - ["save-exact"]: true, + exact: true, }; const localPath = path.resolve(extensionName); diff --git a/lib/services/platform/add-platform-service.ts b/lib/services/platform/add-platform-service.ts index b93aba4ea7..28c45e45db 100644 --- a/lib/services/platform/add-platform-service.ts +++ b/lib/services/platform/add-platform-service.ts @@ -120,9 +120,8 @@ export class AddPlatformService implements IAddPlatformService { { silent: true, dev: true, - "save-dev": true, - "save-exact": true, - } as any + exact: true, + } ); if (!installedPackage.name) { diff --git a/lib/services/plugins-service.ts b/lib/services/plugins-service.ts index 43c843e59a..8717ed8bab 100644 --- a/lib/services/plugins-service.ts +++ b/lib/services/plugins-service.ts @@ -17,7 +17,7 @@ import { } from "../definitions/platform"; import { IProjectDataService, IProjectData } from "../definitions/project"; import { - INodePackageManagerInstallOptions, + IPackageInstallOptions, INodePackageManager, IOptions, IDependencyData, @@ -59,7 +59,7 @@ export class PluginsService implements IPluginsService { return this.$injector.resolve("projectDataService"); } - private get npmInstallOptions(): INodePackageManagerInstallOptions { + private get npmInstallOptions(): IPackageInstallOptions { return _.merge( { disableNpmInstall: this.$options.disableNpmInstall, diff --git a/test/bun-package-manager.ts b/test/bun-package-manager.ts index 8569831ab2..46fdfeb16b 100644 --- a/test/bun-package-manager.ts +++ b/test/bun-package-manager.ts @@ -1,7 +1,7 @@ import { Yok } from "../lib/common/yok"; import * as stubs from "./stubs"; import { assert } from "chai"; -import { Bun } from "../lib/package-managers/bun"; +import { BunPackageManager } from "../lib/package-managers/bun"; import { IInjector } from "../lib/common/definitions/yok"; function createTestInjector(configuration: {} = {}): IInjector { @@ -12,7 +12,7 @@ function createTestInjector(configuration: {} = {}): IInjector { injector.register("childProcess", stubs.ChildProcessStub); injector.register("httpClient", {}); injector.register("fs", stubs.FileSystemStub); - injector.register("bun", Bun); + injector.register("bun", BunPackageManager); injector.register("pacoteService", { manifest: () => Promise.resolve(), }); @@ -50,7 +50,7 @@ describe("node-package-manager", () => { ].forEach((testCase) => { it(testCase.name, async () => { const testInjector = createTestInjector(); - const npm = testInjector.resolve("bun"); + const npm = testInjector.resolve("bun"); const templateNameParts = await npm.getPackageNameParts( testCase.templateFullName ); @@ -85,7 +85,7 @@ describe("node-package-manager", () => { ].forEach((testCase) => { it(testCase.name, async () => { const testInjector = createTestInjector(); - const npm = testInjector.resolve("bun"); + const npm = testInjector.resolve("bun"); const templateFullName = await npm.getPackageFullName({ name: testCase.templateName, version: testCase.templateVersion, diff --git a/test/controllers/add-platform-controller.ts b/test/controllers/add-platform-controller.ts index fa869f692e..be90db2cbf 100644 --- a/test/controllers/add-platform-controller.ts +++ b/test/controllers/add-platform-controller.ts @@ -6,11 +6,11 @@ import { format } from "util"; import * as _ from "lodash"; import { AddPlaformErrors } from "../../lib/constants"; import { PackageManager } from "../../lib/package-managers"; -import { NPM } from "../../lib/package-managers/npm"; -import { Yarn } from "../../lib/package-managers/yarn"; -import { Yarn2 } from "../../lib/package-managers/yarn2"; -import { PNPM } from "../../lib/package-managers/pnpm"; -import { Bun } from "../../lib/package-managers/bun"; +import { NpmPackageManager } from "../../lib/package-managers/npm"; +import { YarnPackageManager } from "../../lib/package-managers/yarn"; +import { Yarn2PackageManager } from "../../lib/package-managers/yarn2"; +import { PnpmPackageManager } from "../../lib/package-managers/pnpm"; +import { BunPackageManager } from "../../lib/package-managers/bun"; import { MobileHelper } from "../../lib/common/mobile/mobile-helper"; let actualMessage: string = null; @@ -29,11 +29,11 @@ function createInjector(data?: { latestFrameworkVersion: string }) { trackEventActionInGoogleAnalytics: () => ({}), }); injector.register("packageManager", PackageManager); - injector.register("npm", NPM); - injector.register("yarn", Yarn); - injector.register("yarn2", Yarn2); - injector.register("pnpm", PNPM); - injector.register("bun", Bun); + injector.register("npm", NpmPackageManager); + injector.register("yarn", YarnPackageManager); + injector.register("yarn2", Yarn2PackageManager); + injector.register("pnpm", PnpmPackageManager); + injector.register("bun", BunPackageManager); injector.register("userSettingsService", { getSettingValue: async (settingName: string): Promise => undefined, diff --git a/test/ios-project-service.ts b/test/ios-project-service.ts index 399557963e..09b124c0fe 100644 --- a/test/ios-project-service.ts +++ b/test/ios-project-service.ts @@ -25,8 +25,8 @@ import { AndroidDeviceDiscovery } from "../lib/common/mobile/mobile-core/android import { Utils } from "../lib/common/utils"; import { CocoaPodsService } from "../lib/services/cocoapods-service"; import { PackageManager } from "../lib/package-managers"; -import { NPM } from "../lib/package-managers/npm"; -import { Yarn } from "../lib/package-managers/yarn"; +import { NpmPackageManager } from "../lib/package-managers/npm"; +import { YarnPackageManager } from "../lib/package-managers/yarn"; import { assert } from "chai"; import { SettingsService } from "../lib/common/test/unit-tests/stubs"; @@ -183,8 +183,8 @@ function createTestInjector( }); testInjector.register("packageManager", PackageManager); testInjector.register("projectConfigService", ProjectConfigServiceStub); - testInjector.register("npm", NPM); - testInjector.register("yarn", Yarn); + testInjector.register("npm", NpmPackageManager); + testInjector.register("yarn", YarnPackageManager); testInjector.register("xcconfigService", XcconfigService); testInjector.register("settingsService", SettingsService); testInjector.register("httpClient", {}); diff --git a/test/node-package-manager.ts b/test/node-package-manager.ts index 79af0c71d0..36d1eb65a9 100644 --- a/test/node-package-manager.ts +++ b/test/node-package-manager.ts @@ -1,7 +1,7 @@ import { Yok } from "../lib/common/yok"; import * as stubs from "./stubs"; import { assert } from "chai"; -import { NPM } from "../lib/package-managers/npm"; +import { NpmPackageManager } from "../lib/package-managers/npm"; import { IInjector } from "../lib/common/definitions/yok"; function createTestInjector(configuration: {} = {}): IInjector { @@ -12,7 +12,7 @@ function createTestInjector(configuration: {} = {}): IInjector { injector.register("childProcess", stubs.ChildProcessStub); injector.register("httpClient", {}); injector.register("fs", stubs.FileSystemStub); - injector.register("npm", NPM); + injector.register("npm", NpmPackageManager); injector.register("pacoteService", { manifest: () => Promise.resolve(), }); @@ -52,7 +52,7 @@ describe("node-package-manager", () => { ].forEach((testCase) => { it(testCase.name, async () => { const testInjector = createTestInjector(); - const npm = testInjector.resolve("npm"); + const npm = testInjector.resolve("npm"); const templateNameParts = await npm.getPackageNameParts( testCase.templateFullName ); @@ -87,7 +87,7 @@ describe("node-package-manager", () => { ].forEach((testCase) => { it(testCase.name, async () => { const testInjector = createTestInjector(); - const npm = testInjector.resolve("npm"); + const npm = testInjector.resolve("npm"); const templateFullName = await npm.getPackageFullName({ name: testCase.templateName, version: testCase.templateVersion, diff --git a/test/package-installation-manager.ts b/test/package-installation-manager.ts index c1a5a462a6..23bceea684 100644 --- a/test/package-installation-manager.ts +++ b/test/package-installation-manager.ts @@ -46,11 +46,11 @@ function createTestInjector(): IInjector { testInjector.register("userSettingsService", { getSettingValue: async (settingName: string): Promise => undefined, }); - testInjector.register("npm", NpmLib.NPM); - testInjector.register("yarn", YarnLib.Yarn); - testInjector.register("yarn2", Yarn2Lib.Yarn2); - testInjector.register("pnpm", PnpmLib.PNPM); - testInjector.register("bun", BunLib.Bun); + testInjector.register("npm", NpmLib.NpmPackageManager); + testInjector.register("yarn", YarnLib.YarnPackageManager); + testInjector.register("yarn2", Yarn2Lib.Yarn2PackageManager); + testInjector.register("pnpm", PnpmLib.PnpmPackageManager); + testInjector.register("bun", BunLib.BunPackageManager); testInjector.register("packageManager", PackageManagerLib.PackageManager); testInjector.register("projectConfigService", ProjectConfigServiceStub); testInjector.register( diff --git a/test/package-manager-flags.ts b/test/package-manager-flags.ts new file mode 100644 index 0000000000..a2bbb0c9fb --- /dev/null +++ b/test/package-manager-flags.ts @@ -0,0 +1,241 @@ +import * as path from "path"; +import { Yok } from "../lib/common/yok"; +import * as stubs from "./stubs"; +import { assert } from "chai"; +import { NpmPackageManager } from "../lib/package-managers/npm"; +import { YarnPackageManager } from "../lib/package-managers/yarn"; +import { Yarn2PackageManager } from "../lib/package-managers/yarn2"; +import { PnpmPackageManager } from "../lib/package-managers/pnpm"; +import { BunPackageManager } from "../lib/package-managers/bun"; +import { + INodePackageManager, + IPackageInstallOptions, +} from "../lib/declarations"; +import { IInjector } from "../lib/common/definitions/yok"; + +class RecordingChildProcessStub extends stubs.ChildProcessStub { + public spawnedArgs: string[][] = []; + public execCommands: string[] = []; + + public async exec( + command: string, + options?: any, + execOptions?: any, + ): Promise { + this.execCommands.push(command); + return super.exec(command, options, execOptions); + } + + public async spawnFromEvent( + command: string, + args: string[], + event: string, + options?: any, + spawnFromEventOptions?: any, + ): Promise { + this.spawnedArgs.push(args); + return super.spawnFromEvent( + command, + args, + event, + options, + spawnFromEventOptions, + ); + } +} + +class NoFilesFileSystemStub extends stubs.FileSystemStub { + exists(filePath: string): boolean { + return false; + } +} + +const managers: { name: string; ctor: any }[] = [ + { name: "npm", ctor: NpmPackageManager }, + { name: "yarn", ctor: YarnPackageManager }, + { name: "yarn2", ctor: Yarn2PackageManager }, + { name: "pnpm", ctor: PnpmPackageManager }, + { name: "bun", ctor: BunPackageManager }, +]; + +function createTestInjector(name: string, ctor: any): 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", NoFilesFileSystemStub); + injector.register(name, ctor); + injector.register("pacoteService", { + manifest: () => Promise.resolve({ name: "left-pad", version: "1.3.0" }), + }); + + return injector; +} + +async function installArgs( + name: string, + ctor: any, + options: IPackageInstallOptions, +): Promise { + const injector = createTestInjector(name, ctor); + const manager = injector.resolve(name); + const childProcess = + injector.resolve("childProcess"); + await manager.install("left-pad", projectDir, options); + return childProcess.spawnedArgs[0]; +} + +async function uninstallCommand( + name: string, + ctor: any, + save: boolean, +): Promise { + const injector = createTestInjector(name, ctor); + const manager = injector.resolve(name); + const childProcess = + injector.resolve("childProcess"); + await manager.uninstall("left-pad", { save }, projectDir); + return childProcess.execCommands[0].trim(); +} + +const projectDir = path.join("/tmp", "some-project"); + +const allOptions: IPackageInstallOptions = { + save: true, + dev: true, + optional: true, + exact: true, + silent: true, + ignoreScripts: true, +}; + +describe("package manager flag mapping", () => { + const expectedInstallArgs: { [name: string]: string[] } = { + npm: [ + "install", + "left-pad", + "--save", + "--save-dev", + "--save-optional", + "--save-exact", + "--silent", + "--ignore-scripts", + ], + yarn: [ + "add", + "left-pad", + "--dev", + "--optional", + "--exact", + "--silent", + "--ignore-scripts", + ], + yarn2: [ + "add", + "left-pad", + "--dev", + "--optional", + "--exact", + "--silent", + "--mode=skip-build", + ], + pnpm: [ + "i", + "--shamefully-hoist", + "left-pad", + "--save-dev", + "--save-optional", + "--save-exact", + "--silent", + "--ignore-scripts", + ], + bun: [ + "install", + "left-pad", + "--save", + "--dev", + "--optional", + "--exact", + "--silent", + "--ignore-scripts", + ], + }; + + for (const { name, ctor } of managers) { + describe(name, () => { + it("maps every install option to its own flag", async () => { + const args = await installArgs(name, ctor, allOptions); + assert.deepEqual(args, expectedInstallArgs[name]); + }); + + it("passes no option flags when no options are set", async () => { + const args = await installArgs(name, ctor, {}); + const expected = expectedInstallArgs[name].filter( + (arg) => !arg.startsWith("--") || arg === "--shamefully-hoist", + ); + assert.deepEqual(args, expected); + }); + + it("never passes CLI-internal options to the command line", async () => { + const args = await installArgs(name, ctor, { + path: "/some/path", + frameworkPath: "/some/framework", + }); + for (const arg of args) { + assert.notMatch(arg, /path|framework/i); + } + }); + + it("skips the install when disableNpmInstall is set", async () => { + const injector = createTestInjector(name, ctor); + const manager = injector.resolve(name); + const childProcess = + injector.resolve("childProcess"); + await manager.install("left-pad", projectDir, { + disableNpmInstall: true, + }); + assert.lengthOf(childProcess.spawnedArgs, 0); + }); + }); + } + + describe("save: false", () => { + it("maps to --no-save where the package manager supports it", async () => { + assert.include( + await installArgs("npm", NpmPackageManager, { save: false }), + "--no-save", + ); + assert.include( + await installArgs("bun", BunPackageManager, { save: false }), + "--no-save", + ); + }); + + it("is dropped where the package manager has no such flag", async () => { + for (const { name, ctor } of managers.filter( + (m) => m.name !== "npm" && m.name !== "bun", + )) { + const args = await installArgs(name, ctor, { save: false }); + assert.notInclude(args, "--no-save", name); + } + }); + }); + + describe("uninstall", () => { + const expected: { [name: string]: string } = { + npm: "npm uninstall left-pad --save", + yarn: "yarn remove left-pad", + yarn2: "yarn remove left-pad", + pnpm: "pnpm remove left-pad", + bun: "bun remove left-pad --save", + }; + + for (const { name, ctor } of managers) { + it(`${name} maps save onto its own remove command`, async () => { + assert.equal(await uninstallCommand(name, ctor, true), expected[name]); + }); + } + }); +}); diff --git a/test/plugins-service.ts b/test/plugins-service.ts index 3372674ed3..4c7eb6c57e 100644 --- a/test/plugins-service.ts +++ b/test/plugins-service.ts @@ -2,11 +2,11 @@ import { Yok } from "../lib/common/yok"; import * as stubs from "./stubs"; import { PackageManager } from "../lib/package-managers"; import { PackageInstallationManager } from "../lib/package-managers/package-installation-manager"; -import { NPM } from "../lib/package-managers/npm"; -import { Yarn } from "../lib/package-managers/yarn"; -import { Yarn2 } from "../lib/package-managers/yarn2"; -import { PNPM } from "../lib/package-managers/pnpm"; -import { Bun } from "../lib/package-managers/bun"; +import { NpmPackageManager } from "../lib/package-managers/npm"; +import { YarnPackageManager } from "../lib/package-managers/yarn"; +import { Yarn2PackageManager } from "../lib/package-managers/yarn2"; +import { PnpmPackageManager } from "../lib/package-managers/pnpm"; +import { BunPackageManager } from "../lib/package-managers/bun"; import { ProjectData } from "../lib/project-data"; import { ChildProcess } from "../lib/common/child-process"; import { Options } from "../lib/options"; @@ -75,11 +75,11 @@ function createTestInjector() { "projectConfigService", stubs.PackageInstallationManagerStub, ); - testInjector.register("npm", NPM); - testInjector.register("yarn", Yarn); - testInjector.register("yarn2", Yarn2); - testInjector.register("pnpm", PNPM); - testInjector.register("bun", Bun); + testInjector.register("npm", NpmPackageManager); + testInjector.register("yarn", YarnPackageManager); + testInjector.register("yarn2", Yarn2PackageManager); + testInjector.register("pnpm", PnpmPackageManager); + testInjector.register("bun", BunPackageManager); testInjector.register("fs", FileSystem); // const fileSystemStub = new stubs.FileSystemStub(); // fileSystemStub.exists = (fileName: string) => { diff --git a/test/pnpm-package-manager.ts b/test/pnpm-package-manager.ts index 6c6a4645bb..16f8b02ac8 100644 --- a/test/pnpm-package-manager.ts +++ b/test/pnpm-package-manager.ts @@ -3,7 +3,7 @@ import { Yok } from "../lib/common/yok"; import * as stubs from "./stubs"; import { assert } from "chai"; import { setIsInteractive } from "../lib/common/helpers"; -import { PNPM } from "../lib/package-managers/pnpm"; +import { PnpmPackageManager } from "../lib/package-managers/pnpm"; import { IInjector } from "../lib/common/definitions/yok"; class RecordingChildProcessStub extends stubs.ChildProcessStub { @@ -66,7 +66,7 @@ function createTestInjector(): IInjector { injector.register("childProcess", RecordingChildProcessStub); injector.register("httpClient", {}); injector.register("fs", SelectiveFileSystemStub); - injector.register("pnpm", PNPM); + injector.register("pnpm", PnpmPackageManager); injector.register("pacoteService", { manifest: () => Promise.resolve({ name: "left-pad", version: "1.3.0" }), }); @@ -80,7 +80,7 @@ describe("pnpm-package-manager", () => { describe("install", () => { it("passes --shamefully-hoist when the project has no pnpm layout config", async () => { const testInjector = createTestInjector(); - const pnpm = testInjector.resolve("pnpm"); + const pnpm = testInjector.resolve("pnpm"); const childProcess = testInjector.resolve("childProcess"); @@ -94,7 +94,7 @@ describe("pnpm-package-manager", () => { it("omits --shamefully-hoist when a pnpm-workspace.yaml governs the project", async () => { const testInjector = createTestInjector(); - const pnpm = testInjector.resolve("pnpm"); + const pnpm = testInjector.resolve("pnpm"); const childProcess = testInjector.resolve("childProcess"); const fs = testInjector.resolve("fs"); @@ -107,7 +107,7 @@ describe("pnpm-package-manager", () => { it("omits --shamefully-hoist when an ancestor pnpm-workspace.yaml governs the project", async () => { const testInjector = createTestInjector(); - const pnpm = testInjector.resolve("pnpm"); + const pnpm = testInjector.resolve("pnpm"); const childProcess = testInjector.resolve("childProcess"); const fs = testInjector.resolve("fs"); @@ -120,7 +120,7 @@ describe("pnpm-package-manager", () => { it("omits --shamefully-hoist when an .npmrc sets a layout key", async () => { const testInjector = createTestInjector(); - const pnpm = testInjector.resolve("pnpm"); + const pnpm = testInjector.resolve("pnpm"); const childProcess = testInjector.resolve("childProcess"); const fs = testInjector.resolve("fs"); @@ -137,7 +137,7 @@ describe("pnpm-package-manager", () => { ["hoist-pattern[]", "public-hoist-pattern[]"].forEach((layoutKey) => { it(`omits --shamefully-hoist when an .npmrc sets array-valued ${layoutKey}`, async () => { const testInjector = createTestInjector(); - const pnpm = testInjector.resolve("pnpm"); + const pnpm = testInjector.resolve("pnpm"); const childProcess = testInjector.resolve("childProcess"); const fs = testInjector.resolve("fs"); @@ -146,7 +146,7 @@ describe("pnpm-package-manager", () => { fs.textFiles[npmrcPath] = `registry=https://example.com\n${layoutKey}=*types*\n`; - await pnpm.install(projectDir, projectDir, {} as any); + await pnpm.install(projectDir, projectDir, {}); assert.deepEqual(childProcess.spawnedArgs[0], ["i"]); }); @@ -154,7 +154,7 @@ describe("pnpm-package-manager", () => { it("keeps --shamefully-hoist when an .npmrc has no layout key", async () => { const testInjector = createTestInjector(); - const pnpm = testInjector.resolve("pnpm"); + const pnpm = testInjector.resolve("pnpm"); const childProcess = testInjector.resolve("childProcess"); const fs = testInjector.resolve("fs"); @@ -172,7 +172,7 @@ describe("pnpm-package-manager", () => { it("maps ignoreScripts to --ignore-scripts and drops internal options pnpm rejects", async () => { const testInjector = createTestInjector(); - const pnpm = testInjector.resolve("pnpm"); + const pnpm = testInjector.resolve("pnpm"); const childProcess = testInjector.resolve("childProcess"); @@ -180,7 +180,7 @@ describe("pnpm-package-manager", () => { ignoreScripts: true, path: "/some/path", frameworkPath: "/some/framework", - } as any); + }); const args = childProcess.spawnedArgs[0]; assert.include(args, "--ignore-scripts"); @@ -191,13 +191,13 @@ describe("pnpm-package-manager", () => { it("spawns non-interactive installs with stdin closed", async () => { const testInjector = createTestInjector(); - const pnpm = testInjector.resolve("pnpm"); + const pnpm = testInjector.resolve("pnpm"); const childProcess = testInjector.resolve("childProcess"); setIsInteractive(() => false); try { - await pnpm.install(projectDir, projectDir, {} as any); + await pnpm.install(projectDir, projectDir, {}); } finally { setIsInteractive(undefined); } @@ -213,17 +213,16 @@ describe("pnpm-package-manager", () => { it("appends the package name when installing a single package", async () => { const testInjector = createTestInjector(); - const pnpm = testInjector.resolve("pnpm"); + const pnpm = testInjector.resolve("pnpm"); const childProcess = testInjector.resolve("childProcess"); - await pnpm.install("left-pad", projectDir, { save: true } as any); + await pnpm.install("left-pad", projectDir, { save: true }); assert.deepEqual(childProcess.spawnedArgs[0], [ "i", "--shamefully-hoist", "left-pad", - "--save", ]); }); }); @@ -231,7 +230,7 @@ describe("pnpm-package-manager", () => { describe("getCachePath", () => { it("uses the configured cache directory when pnpm reports one", async () => { const testInjector = createTestInjector(); - const pnpm = testInjector.resolve("pnpm"); + const pnpm = testInjector.resolve("pnpm"); const childProcess = testInjector.resolve("childProcess"); childProcess.execResponses["pnpm config get cache"] = "/custom/cache\n"; @@ -243,7 +242,7 @@ describe("pnpm-package-manager", () => { 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 pnpm = testInjector.resolve("pnpm"); const childProcess = testInjector.resolve("childProcess"); childProcess.execResponses["pnpm config get cache"] = "undefined\n"; diff --git a/test/project-templates-service.ts b/test/project-templates-service.ts index 10abc12520..6635526130 100644 --- a/test/project-templates-service.ts +++ b/test/project-templates-service.ts @@ -6,7 +6,7 @@ import * as path from "path"; import * as constants from "../lib/constants"; import { INpmInstallResultInfo, - INodePackageManagerInstallOptions, + IPackageInstallOptions, INpmPackageNameParts, INpmInstallOptions, } from "../lib/declarations"; @@ -38,7 +38,7 @@ function createTestInjector( public async install( packageName: string, pathToSave: string, - config: INodePackageManagerInstallOptions + config: IPackageInstallOptions ): Promise { if (configuration.shouldNpmInstallThrow) { throw new Error("NPM install throws error."); diff --git a/test/services/extensibility-service.ts b/test/services/extensibility-service.ts index e5acaf36d8..0ea1ba29af 100644 --- a/test/services/extensibility-service.ts +++ b/test/services/extensibility-service.ts @@ -2,12 +2,12 @@ import { ExtensibilityService } from "../../lib/services/extensibility-service"; import { Yok } from "../../lib/common/yok"; import * as stubs from "../stubs"; import { assert } from "chai"; -import { NPM } from "../../lib/package-managers/npm"; +import { NpmPackageManager } from "../../lib/package-managers/npm"; import { PackageManager } from "../../lib/package-managers"; -import { Yarn } from "../../lib/package-managers/yarn"; -import { Yarn2 } from "../../lib/package-managers/yarn2"; -import { PNPM } from "../../lib/package-managers/pnpm"; -import { Bun } from "../../lib/package-managers/bun"; +import { YarnPackageManager } from "../../lib/package-managers/yarn"; +import { Yarn2PackageManager } from "../../lib/package-managers/yarn2"; +import { PnpmPackageManager } from "../../lib/package-managers/pnpm"; +import { BunPackageManager } from "../../lib/package-managers/bun"; import * as constants from "../../lib/constants"; import { ChildProcess } from "../../lib/common/child-process"; import { CommandsDelimiters } from "../../lib/common/constants"; @@ -75,11 +75,11 @@ describe("extensibilityService", () => { testInjector.register("userSettingsService", { getSettingValue: async (settingName: string): Promise => undefined, }); - testInjector.register("npm", NPM); - testInjector.register("yarn", Yarn); - testInjector.register("yarn2", Yarn2); - testInjector.register("pnpm", PNPM); - testInjector.register("bun", Bun); + testInjector.register("npm", NpmPackageManager); + testInjector.register("yarn", YarnPackageManager); + testInjector.register("yarn2", Yarn2PackageManager); + testInjector.register("pnpm", PnpmPackageManager); + testInjector.register("bun", BunPackageManager); testInjector.register("settingsService", SettingsService); testInjector.register("requireService", { require: (pathToRequire: string): any => undefined, @@ -245,15 +245,16 @@ describe("extensibilityService", () => { ); }); - it("passes save and save-exact options to npm install", async () => { + it("passes save and exact options to the package manager", async () => { const extensionName = "extension1"; const argsPassedToNpmInstall = await getArgsPassedToNpmInstallDuringInstallExtensionCall( extensionName, ); - const expectedNpmConfg: any = { save: true }; - expectedNpmConfg["save-exact"] = true; - assert.deepStrictEqual(argsPassedToNpmInstall.config, expectedNpmConfg); + assert.deepStrictEqual(argsPassedToNpmInstall.config, { + save: true, + exact: true, + }); }); it("passes full path to extensions dir for installation", async () => { diff --git a/test/stubs.ts b/test/stubs.ts index 8011d1c0be..371295ca24 100644 --- a/test/stubs.ts +++ b/test/stubs.ts @@ -17,7 +17,7 @@ import { INpmInstallOptions, INodePackageManager, INpmInstallResultInfo, - INodePackageManagerInstallOptions, + IPackageInstallOptions, INpmPackageNameParts, INpmsResult, IAndroidToolsInfoData, @@ -460,7 +460,7 @@ export class NodePackageManagerStub implements INodePackageManager { public async install( packageName: string, pathToSave: string, - config: INodePackageManagerInstallOptions, + options: IPackageInstallOptions, ): Promise { return { name: packageName, From f3f4e1b44a1aae44285dbf7224933d99d2db9739 Mon Sep 17 00:00:00 2001 From: Igor Randjelovic Date: Wed, 16 Sep 2026 11:40:01 +0200 Subject: [PATCH 3/6] refactor: manager-agnostic view/search and resolution-based package lookups view() now takes an optional registry field instead of an npm flag bag, and search() takes only the keywords; each package manager spells the field selection itself (yarn berry uses --fields). The generic npm flag-string builder is gone with them, so nothing in the package manager layer emits npm syntax on behalf of another manager any more. Project-facing package lookups go through Node resolution from the project directory instead of joining node_modules/ by hand: test-init peer dependency discovery, the installed path returned by PackageInstallationManager, the local inspector check, the core modules short-import scan in doctor, and the installed core modules versions in versions-service. plugins-service no longer creates an empty node_modules directory before enumerating dependencies. --- PublicAPI.md | 21 ++++---- lib/commands/test-init.ts | 7 +-- lib/contracts/package-manager.ts | 19 +++---- lib/declarations.d.ts | 13 +++-- lib/package-managers/base-package-manager.ts | 51 ++---------------- lib/package-managers/bun.ts | 19 +++---- lib/package-managers/index.ts | 14 ++--- lib/package-managers/npm.ts | 15 ++---- .../package-installation-manager.ts | 38 +++---------- lib/package-managers/pnpm.ts | 19 ++----- lib/package-managers/yarn.ts | 16 ++---- lib/package-managers/yarn2.ts | 18 +++---- lib/services/android-plugin-build-service.ts | 8 ++- lib/services/doctor-service.ts | 21 ++++---- lib/services/plugins-service.ts | 15 +----- lib/services/versions-service.ts | 32 +++++------ test/package-installation-manager.ts | 6 +-- test/services/android-plugin-build-service.ts | 6 +-- test/services/doctor-service.ts | 53 ++++++++++++++++--- test/stubs.ts | 4 +- 20 files changed, 158 insertions(+), 237 deletions(-) diff --git a/PublicAPI.md b/PublicAPI.md index 0d59215163..351d537418 100644 --- a/PublicAPI.md +++ b/PublicAPI.md @@ -556,16 +556,15 @@ Searches for a package using keywords. ```TypeScript /** * Searches for a package. - * @param {string[]} filter Keywords with which to perform the search. - * @param {IDictionary} config Additional options that can be passed to manipulate search. - * @return {Promise} The output of the uninstallation. + * @param {string[]} filter Keywords with which to perform the search. + * @return {Promise} The raw search output. */ -search(filter: string[], config: IDictionary): Promise; +search(filter: string[]): Promise; ``` * Usage: ```JavaScript -tns.npm.search(["nativescript", "cloud"], { silent: true }).then(output => { +tns.npm.search(["nativescript", "cloud"]).then(output => { console.log(`Found: ${output}`); }, err => { console.log("An error occurred during searching", err); @@ -578,17 +577,17 @@ Provides information about a given package. * Definition ```TypeScript /** - * Provides information about a given package. - * @param {string} packageName The name of the package. - * @param {IDictionary} config Additional options that can be passed to manipulate view. - * @return {Promise} Object, containing information about the package. + * Provides registry information about a package. + * @param {string} packageName The name of the package, optionally with a version. + * @param {string} field Optional single registry field (e.g. "versions" or "dist-tags") to return instead of the whole document. + * @return {Promise} The parsed registry data. */ -view(packageName: string, config: Object): Promise; +view(packageName: string, field?: string): Promise; ``` * Usage: ```JavaScript -tns.npm.view(["nativescript"], {}).then(result => { +tns.npm.view("nativescript").then(result => { console.log(`${result.name}'s latest version is ${result["dist-tags"].latest}`); }, err => { console.log("An error occurred during viewing", err); diff --git a/lib/commands/test-init.ts b/lib/commands/test-init.ts index 9dd5879e3b..8f33e62b01 100644 --- a/lib/commands/test-init.ts +++ b/lib/commands/test-init.ts @@ -2,6 +2,7 @@ import * as path from "path"; import * as _ from "lodash"; import { TESTING_FRAMEWORKS, ProjectTypes } from "../constants"; import { fromWindowsRelativePathToUnix } from "../common/helpers"; +import { resolvePackageJSONPath } from "../helpers/package-path-helper"; import { IProjectData, ITestInitializationService, @@ -137,9 +138,9 @@ export class TestInitCommand extends Command({ path: this.$options.path, }); - const modulePath = path.join(projectDir, "node_modules", mod.name); - const modulePackageJsonPath = path.join(modulePath, "package.json"); - const modulePackageJsonContent = this.$fs.readJson(modulePackageJsonPath); + const modulePackageJsonContent = this.$fs.readJson( + resolvePackageJSONPath(mod.name, { paths: [projectDir] }), + ); const modulePeerDependencies = modulePackageJsonContent.peerDependencies || {}; const modulePeerDependenciesMeta = diff --git a/lib/contracts/package-manager.ts b/lib/contracts/package-manager.ts index ef58e3e10f..e5b09c8b57 100644 --- a/lib/contracts/package-manager.ts +++ b/lib/contracts/package-manager.ts @@ -1,5 +1,4 @@ import { Contract } from "../common/di/contract"; -import type { IDictionary } from "../common/declarations"; import type { IPackageInstallOptions, IPackageUninstallOptions, @@ -42,11 +41,11 @@ export abstract class PackageManager { /** * Provides information about a given package. - * @param {string} packageName The name of the package. - * @param {IDictionary} config Additional options that can be passed to manipulate view. - * @return {Promise} Object, containing information about the package. + * @param {string} packageName The name of the package, optionally with a version. + * @param {string} field @optional A single registry field (e.g. "versions" or "dist-tags") to return instead of the whole document. + * @return {Promise} The parsed registry data, or null when it cannot be parsed. */ - abstract view(packageName: string, config: Object): Promise; + abstract view(packageName: string, field?: string): Promise; /** * Checks if the specified string is name of a packaged published in the NPM registry. @@ -75,14 +74,10 @@ export abstract class PackageManager { /** * Searches for a package. - * @param {string[]} filter Keywords with which to perform the search. - * @param {IDictionary} config Additional options that can be passed to manipulate search. - * @return {Promise} The output of the uninstallation. + * @param {string[]} filter Keywords with which to perform the search. + * @return {Promise} The raw search output. */ - abstract search( - filter: string[], - config: IDictionary, - ): Promise; + abstract search(filter: string[]): Promise; /** * Searches for npm packages in npms by keyword. diff --git a/lib/declarations.d.ts b/lib/declarations.d.ts index c51b2a8ce9..cc3b1f33fe 100644 --- a/lib/declarations.d.ts +++ b/lib/declarations.d.ts @@ -55,7 +55,13 @@ interface INodePackageManager { * @param {IDictionary} config Additional options that can be passed to manipulate view. * @return {Promise} Object, containing information about the package. */ - view(packageName: string, config: Object): Promise; + /** + * Provides registry information about a package. + * @param {string} packageName The name of the package, optionally with a version. + * @param {string} field @optional A single registry field (e.g. "versions" or "dist-tags") to return instead of the whole document. + * @return {Promise} The parsed registry data, or null when it cannot be parsed. + */ + view(packageName: string, field?: string): Promise; /** * Checks if the specified string is name of a packaged published in the NPM registry. @@ -84,10 +90,7 @@ interface INodePackageManager { * @param {IDictionary} config Additional options that can be passed to manipulate search. * @return {Promise} The output of the uninstallation. */ - search( - filter: string[], - config: IDictionary, - ): Promise; + search(filter: string[]): Promise; /** * Searches for npm packages in npms by keyword. diff --git a/lib/package-managers/base-package-manager.ts b/lib/package-managers/base-package-manager.ts index 789b9c7ab2..3bcb7e52da 100644 --- a/lib/package-managers/base-package-manager.ts +++ b/lib/package-managers/base-package-manager.ts @@ -7,12 +7,7 @@ import { INpmsResult, INpmPackageNameParts, } from "../declarations"; -import { - IDictionary, - IChildProcess, - IFileSystem, - IHostInfo, -} from "../common/declarations"; +import { IChildProcess, IFileSystem, IHostInfo } from "../common/declarations"; /** * How one package manager spells each IPackageInstallOptions flag on its @@ -43,11 +38,8 @@ export abstract class BasePackageManager implements INodePackageManager { options?: IPackageUninstallOptions, path?: string, ): Promise; - public abstract view(packageName: string, config: Object): Promise; - public abstract search( - filter: string[], - config: IDictionary, - ): Promise; + public abstract view(packageName: string, field?: string): Promise; + public abstract search(filter: string[]): Promise; public abstract searchNpms(keyword: string): Promise; public abstract getRegistryPackageData(packageName: string): Promise; public abstract getCachePath(): Promise; @@ -70,7 +62,7 @@ export abstract class BasePackageManager implements INodePackageManager { } try { - const viewResult = await this.view(packageName, { name: true }); + const viewResult = await this.view(packageName, "name"); // `npm view nonExistingPackageName` will return `nativescript` // if executed in the root dir of the CLI (npm 6.4.1) @@ -183,41 +175,6 @@ export abstract class BasePackageManager implements INodePackageManager { return result; } - protected getFlagsString(config: any, asArray: boolean): any { - const array: Array = []; - for (const flag in config) { - if ( - flag === "global" && - this.packageManager !== "yarn" && - this.packageManager !== "yarn2" - ) { - array.push(`--${flag}`); - array.push(`${config[flag]}`); - } else if (config[flag]) { - if ( - flag === "dist-tags" || - flag === "versions" || - flag === "name" || - flag === "gradle" || - flag === "version_info" - ) { - if (this.packageManager === "yarn2") { - array.push(`--fields ${flag}`); - } else { - array.push(` ${flag}`); - } - continue; - } - array.push(`--${flag}`); - } - } - if (asArray) { - return array; - } - - return array.join(" "); - } - private isTgz(packageName: string): boolean { return packageName.indexOf(".tgz") >= 0; } diff --git a/lib/package-managers/bun.ts b/lib/package-managers/bun.ts index ddb46464b4..65911014a5 100644 --- a/lib/package-managers/bun.ts +++ b/lib/package-managers/bun.ts @@ -94,17 +94,13 @@ export class BunPackageManager extends BasePackageManager { }); } - // Bun does not have a `view` command; use npm. @exported("bun") - public async view(packageName: string, config: Object): Promise { - const wrappedConfig = _.extend({}, config, { json: true }); // always require view response as JSON - - const flags = this.getFlagsString(wrappedConfig, false); + // Bun does not have a `view` command; use npm. + public async view(packageName: string, field?: string): Promise { + const args = [packageName, field, "--json"].filter(Boolean).join(" "); let viewResult: any; try { - viewResult = await this.$childProcess.exec( - `npm view ${packageName} ${flags}` - ); + viewResult = await this.$childProcess.exec(`npm view ${args}`); } catch (e) { this.$errors.fail(e.message); } @@ -116,11 +112,10 @@ export class BunPackageManager extends BasePackageManager { } } - // Bun does not have a `search` command; use npm. @exported("bun") - public async search(filter: string[], config: any): Promise { - const flags = this.getFlagsString(config, false); - return this.$childProcess.exec(`npm search ${filter.join(" ")} ${flags}`); + // Bun does not have a `search` command; use npm. + public async search(filter: string[]): Promise { + return this.$childProcess.exec(`npm search ${filter.join(" ")}`); } public async searchNpms(keyword: string): Promise { diff --git a/lib/package-managers/index.ts b/lib/package-managers/index.ts index 668f30e9cb..cfbd6bbce7 100644 --- a/lib/package-managers/index.ts +++ b/lib/package-managers/index.ts @@ -14,7 +14,6 @@ import { import { IErrors, IUserSettingsService, - IDictionary, } from "../common/declarations"; import { injector } from "../common/yok"; import { IProjectConfigService } from "../definitions/project"; @@ -66,16 +65,13 @@ export class PackageManager implements IPackageManager { } @exported("packageManager") @invokeInit() - public view(packageName: string, config: Object): Promise { - return this.packageManager.view(packageName, config); + public view(packageName: string, field?: string): Promise { + return this.packageManager.view(packageName, field); } @exported("packageManager") @invokeInit() - public search( - filter: string[], - config: IDictionary - ): Promise { - return this.packageManager.search(filter, config); + public search(filter: string[]): Promise { + return this.packageManager.search(filter); } @invokeInit() @@ -122,7 +118,7 @@ export class PackageManager implements IPackageManager { } try { - const result = await this.view(packageName, { "dist-tags": true }); + const result = await this.view(packageName, "dist-tags"); version = result[tag]; } catch (err) { this.$logger.trace( diff --git a/lib/package-managers/npm.ts b/lib/package-managers/npm.ts index 839ffae6d4..9b55fe3336 100644 --- a/lib/package-managers/npm.ts +++ b/lib/package-managers/npm.ts @@ -125,21 +125,16 @@ export class NpmPackageManager extends BasePackageManager { } @exported("npm") - public async search(filter: string[], config: any): Promise { - const flags = this.getFlagsString(config, false); - return this.$childProcess.exec(`npm search ${filter.join(" ")} ${flags}`); + public async search(filter: string[]): Promise { + return this.$childProcess.exec(`npm search ${filter.join(" ")}`); } @exported("npm") - public async view(packageName: string, config: Object): Promise { - const wrappedConfig = _.extend({}, config, { json: true }); // always require view response as JSON - - const flags = this.getFlagsString(wrappedConfig, false); + public async view(packageName: string, field?: string): Promise { + const args = [packageName, field, "--json"].filter(Boolean).join(" "); let viewResult: any; try { - viewResult = await this.$childProcess.exec( - `npm view ${packageName} ${flags}` - ); + viewResult = await this.$childProcess.exec(`npm view ${args}`); } catch (e) { this.$errors.fail(e.message); } diff --git a/lib/package-managers/package-installation-manager.ts b/lib/package-managers/package-installation-manager.ts index edc5f95250..7fe8d9722a 100644 --- a/lib/package-managers/package-installation-manager.ts +++ b/lib/package-managers/package-installation-manager.ts @@ -1,5 +1,6 @@ import * as path from "path"; import * as constants from "../constants"; +import { resolvePackagePath } from "../helpers/package-path-helper"; import { INpmInstallOptions, INpmInstallResultInfo, @@ -67,9 +68,7 @@ export class PackageInstallationManager implements IPackageInstallationManager { packageName: string, versionRange: string ): Promise { - const data = await this.$packageManager.view(packageName, { - versions: true, - }); + const data = await this.$packageManager.view(packageName, "versions"); let versions; @@ -190,14 +189,11 @@ export class PackageInstallationManager implements IPackageInstallationManager { inspectorNpmPackageName: string, projectDir: string ): Promise { - const inspectorPath = path.join( - projectDir, - constants.NODE_MODULES_FOLDER_NAME, - inspectorNpmPackageName - ); - // local installation takes precedence over cache - if (this.inspectorAlreadyInstalled(inspectorPath)) { + const inspectorPath = resolvePackagePath(inspectorNpmPackageName, { + paths: [projectDir], + }); + if (inspectorPath) { return inspectorPath; } @@ -266,14 +262,6 @@ export class PackageInstallationManager implements IPackageInstallationManager { } } - private inspectorAlreadyInstalled(pathToInspector: string): Boolean { - if (this.$fs.exists(pathToInspector)) { - return true; - } - - return false; - } - private async installCore( packageName: string, pathToSave: string, @@ -293,15 +281,7 @@ export class PackageInstallationManager implements IPackageInstallationManager { version, dev ); - const installedPackageName = installResultInfo.name; - - const pathToInstalledPackage = path.join( - pathToSave, - "node_modules", - installedPackageName - ); - - return pathToInstalledPackage; + return resolvePackagePath(installResultInfo.name, { paths: [pathToSave] }); } private async npmInstall( @@ -335,9 +315,7 @@ export class PackageInstallationManager implements IPackageInstallationManager { packageName: string, version: string ): Promise { - let data: any = await this.$packageManager.view(packageName, { - "dist-tags": true, - }); + let data: any = await this.$packageManager.view(packageName, "dist-tags"); data = data?.["dist-tags"] ?? data; this.$logger.trace("Using version %s. ", data[version]); diff --git a/lib/package-managers/pnpm.ts b/lib/package-managers/pnpm.ts index 137f26c556..1321e50a9e 100644 --- a/lib/package-managers/pnpm.ts +++ b/lib/package-managers/pnpm.ts @@ -15,7 +15,6 @@ import { IFileSystem, IHostInfo, Server, - IDictionary, } from "../common/declarations"; import { injector } from "../common/yok"; @@ -98,15 +97,11 @@ export class PnpmPackageManager extends BasePackageManager { } @exported("pnpm") - public async view(packageName: string, config: Object): Promise { - const wrappedConfig = _.extend({}, config, { json: true }); - - const flags = this.getFlagsString(wrappedConfig, false); + public async view(packageName: string, field?: string): Promise { + const args = [packageName, field, "--json"].filter(Boolean).join(" "); let viewResult: any; try { - viewResult = await this.$childProcess.exec( - `pnpm info ${packageName} ${flags}`, - ); + viewResult = await this.$childProcess.exec(`pnpm info ${args}`); } catch (e) { this.$errors.fail(e.message); } @@ -119,12 +114,8 @@ export class PnpmPackageManager extends BasePackageManager { } @exported("pnpm") - public search( - filter: string[], - config: IDictionary, - ): Promise { - const flags = this.getFlagsString(config, false); - return this.$childProcess.exec(`pnpm search ${filter.join(" ")} ${flags}`); + public async search(filter: string[]): Promise { + return this.$childProcess.exec(`pnpm search ${filter.join(" ")}`); } public async searchNpms(keyword: string): Promise { diff --git a/lib/package-managers/yarn.ts b/lib/package-managers/yarn.ts index dd1b916b66..aecffc77ca 100644 --- a/lib/package-managers/yarn.ts +++ b/lib/package-managers/yarn.ts @@ -14,7 +14,6 @@ import { IFileSystem, IHostInfo, Server, - IDictionary, } from "../common/declarations"; import { injector } from "../common/yok"; @@ -89,15 +88,11 @@ export class YarnPackageManager extends BasePackageManager { } @exported("yarn") - public async view(packageName: string, config: Object): Promise { - const wrappedConfig = _.extend({}, config, { json: true }); - - const flags = this.getFlagsString(wrappedConfig, false); + public async view(packageName: string, field?: string): Promise { + const args = [packageName, field, "--json"].filter(Boolean).join(" "); let viewResult: any; try { - viewResult = await this.$childProcess.exec( - `yarn info ${packageName} ${flags}` - ); + viewResult = await this.$childProcess.exec(`yarn info ${args}`); } catch (e) { this.$errors.fail(e.message); } @@ -111,10 +106,7 @@ export class YarnPackageManager extends BasePackageManager { } @exported("yarn") - public search( - filter: string[], - config: IDictionary - ): Promise { + public search(filter: string[]): Promise { this.$errors.fail( "Method not implemented. Yarn does not support searching for packages in the registry." ); diff --git a/lib/package-managers/yarn2.ts b/lib/package-managers/yarn2.ts index 1e4d1736aa..4cbb9df362 100644 --- a/lib/package-managers/yarn2.ts +++ b/lib/package-managers/yarn2.ts @@ -14,7 +14,6 @@ import { IFileSystem, IHostInfo, Server, - IDictionary, } from "../common/declarations"; import { injector } from "../common/yok"; @@ -103,15 +102,13 @@ export class Yarn2PackageManager extends BasePackageManager { } @exported("yarn2") - public async view(packageName: string, config: Object): Promise { - const wrappedConfig = _.extend({}, config, { json: true }); - - const flags = this.getFlagsString(wrappedConfig, false); + public async view(packageName: string, field?: string): Promise { + const args = [packageName, field && `--fields ${field}`, "--json"] + .filter(Boolean) + .join(" "); let viewResult: any; try { - viewResult = await this.$childProcess.exec( - `yarn npm info ${packageName} ${flags}` - ); + viewResult = await this.$childProcess.exec(`yarn npm info ${args}`); } catch (e) { this.$errors.fail(e.message); } @@ -125,10 +122,7 @@ export class Yarn2PackageManager extends BasePackageManager { } @exported("yarn2") - public search( - filter: string[], - config: IDictionary - ): Promise { + public search(filter: string[]): Promise { this.$errors.fail( "Method not implemented. Yarn does not support searching for packages in the registry." ); diff --git a/lib/services/android-plugin-build-service.ts b/lib/services/android-plugin-build-service.ts index 88098d55b0..9dfd557271 100644 --- a/lib/services/android-plugin-build-service.ts +++ b/lib/services/android-plugin-build-service.ts @@ -492,9 +492,7 @@ export class AndroidPluginBuildService implements IAndroidPluginBuildService { this.$projectData.nsConfig?.android?.runtimePackageName || SCOPED_ANDROID_RUNTIME_NAME; try { - let result = await this.$packageManager.view(packageName, { - "dist-tags": true, - }); + let result = await this.$packageManager.view(packageName, "dist-tags"); result = result?.["dist-tags"] ?? result; runtimeVersion = result.latest; } catch (err) { @@ -590,7 +588,7 @@ export class AndroidPluginBuildService implements IAndroidPluginBuildService { try { let output = await this.$packageManager.view( `${packageName}@${runtimeVersion}`, - { version_info: true }, + "version_info", ); output = output?.["version_info"] ?? output; @@ -605,7 +603,7 @@ export class AndroidPluginBuildService implements IAndroidPluginBuildService { */ output = await this.$packageManager.view( `${packageName}@${runtimeVersion}`, - { gradle: true }, + "gradle", ); output = output?.["gradle"] ?? output; diff --git a/lib/services/doctor-service.ts b/lib/services/doctor-service.ts index d57018efec..f5d188aef6 100644 --- a/lib/services/doctor-service.ts +++ b/lib/services/doctor-service.ts @@ -1,13 +1,10 @@ import { EOL } from "os"; import * as path from "path"; +import { resolvePackagePath } from "../helpers/package-path-helper"; import * as _ from "lodash"; import * as helpers from "../common/helpers"; import { cache } from "../common/decorators"; -import { - TrackActionNames, - NODE_MODULES_FOLDER_NAME, - TNS_CORE_MODULES_NAME, -} from "../constants"; +import { TrackActionNames, TNS_CORE_MODULES_NAME } from "../constants"; import { DoctorService } from "../contracts/doctor-service"; import { doctor, constants } from "@nativescript/doctor"; import { IProjectDataService } from "../definitions/project"; @@ -278,6 +275,9 @@ export class DoctorServiceImpl implements DoctorService { ): { file: string; line: string }[] { const shortImportRegExp = this.getShortImportRegExp(projectDir); const shortImports: { file: string; line: string }[] = []; + if (!shortImportRegExp) { + return shortImports; + } for (const file of files) { const fileContent = this.$fs.readText(file); @@ -305,11 +305,12 @@ export class DoctorServiceImpl implements DoctorService { } private getShortImportRegExp(projectDir: string): RegExp { - const pathToTnsCoreModules = path.join( - projectDir, - NODE_MODULES_FOLDER_NAME, - TNS_CORE_MODULES_NAME, - ); + const pathToTnsCoreModules = resolvePackagePath(TNS_CORE_MODULES_NAME, { + paths: [projectDir], + }); + if (!pathToTnsCoreModules) { + return null; + } const coreModulesSubDirs = this.$fs .readDirectory(pathToTnsCoreModules) .filter((entry) => diff --git a/lib/services/plugins-service.ts b/lib/services/plugins-service.ts index 8717ed8bab..7ba2bc593c 100644 --- a/lib/services/plugins-service.ts +++ b/lib/services/plugins-service.ts @@ -84,7 +84,7 @@ export class PluginsService implements IPluginsService { ) {} public async add(plugin: string, projectData: IProjectData): Promise { - await this.ensure(projectData); + await this.ensureAllDependenciesAreInstalled(projectData); const possiblePackageName = path.resolve(plugin); if ( possiblePackageName.indexOf(".tgz") !== -1 && @@ -702,10 +702,6 @@ This framework comes from ${dependencyName} plugin, which is installed multiple })); } - private getNodeModulesPath(projectDir: string): string { - return path.join(projectDir, "node_modules"); - } - private getPackageJsonFilePath(projectDir: string): string { return path.join(projectDir, "package.json"); } @@ -754,17 +750,10 @@ This framework comes from ${dependencyName} plugin, which is installed multiple }; } - private async ensure(projectData: IProjectData): Promise { - await this.ensureAllDependenciesAreInstalled(projectData); - this.$fs.ensureDirectoryExists( - this.getNodeModulesPath(projectData.projectDir), - ); - } - private async getAllInstalledModules( projectData: IProjectData, ): Promise { - await this.ensure(projectData); + await this.ensureAllDependenciesAreInstalled(projectData); const nodeModules = this.getDependencies(projectData.projectDir); return _.map(nodeModules, (nodeModuleName) => diff --git a/lib/services/versions-service.ts b/lib/services/versions-service.ts index 3710ab52bc..e9e93a0f6d 100644 --- a/lib/services/versions-service.ts +++ b/lib/services/versions-service.ts @@ -2,6 +2,7 @@ import * as constants from "../constants"; import * as helpers from "../common/helpers"; import * as semver from "semver"; import * as path from "path"; +import { resolvePackagePath } from "../helpers/package-path-helper"; import { IVersionsService, IPackageInstallationManager } from "../declarations"; import { IProjectData, IProjectDataService } from "../definitions/project"; import { IPluginsService, IBasePluginData } from "../definitions/plugins"; @@ -63,18 +64,12 @@ class VersionsService implements IVersionsService { const versionInformations: IVersionInformation[] = []; if (this.projectData) { - const nodeModulesPath = path.join( - this.projectData.projectDir, - constants.NODE_MODULES_FOLDER_NAME - ); - const scopedPackagePath = path.join( - nodeModulesPath, - constants.SCOPED_TNS_CORE_MODULES - ); - const tnsCoreModulesPath = path.join( - nodeModulesPath, - constants.TNS_CORE_MODULES_NAME - ); + const resolve = (packageName: string) => + resolvePackagePath(packageName, { + paths: [this.projectData.projectDir], + }); + let scopedPackagePath = resolve(constants.SCOPED_TNS_CORE_MODULES); + let tnsCoreModulesPath = resolve(constants.TNS_CORE_MODULES_NAME); const dependsOnNonScopedPackage = !!this.projectData.dependencies[ constants.TNS_CORE_MODULES_NAME @@ -83,18 +78,19 @@ class VersionsService implements IVersionsService { constants.SCOPED_TNS_CORE_MODULES ]; - // ensure the dependencies are installed, so we can get their actual versions from node_modules + // ensure the dependencies are installed, so we can read their actual versions if ( - !this.$fs.exists(nodeModulesPath) || - (dependsOnNonScopedPackage && !this.$fs.exists(tnsCoreModulesPath)) || - (dependsOnScopedPackage && !this.$fs.exists(scopedPackagePath)) + (dependsOnNonScopedPackage && !tnsCoreModulesPath) || + (dependsOnScopedPackage && !scopedPackagePath) ) { await this.$pluginsService.ensureAllDependenciesAreInstalled( this.projectData ); + scopedPackagePath = resolve(constants.SCOPED_TNS_CORE_MODULES); + tnsCoreModulesPath = resolve(constants.TNS_CORE_MODULES_NAME); } - if (dependsOnNonScopedPackage && this.$fs.exists(tnsCoreModulesPath)) { + if (dependsOnNonScopedPackage && tnsCoreModulesPath) { const currentTnsCoreModulesVersion = this.$fs.readJson( path.join(tnsCoreModulesPath, constants.PACKAGE_JSON_FILE_NAME) ).version; @@ -102,7 +98,7 @@ class VersionsService implements IVersionsService { versionInformations.push(nativescriptCoreModulesInfo); } - if (dependsOnScopedPackage && this.$fs.exists(scopedPackagePath)) { + if (dependsOnScopedPackage && scopedPackagePath) { const scopedModulesInformation: IVersionInformation = { componentName: constants.SCOPED_TNS_CORE_MODULES, latestVersion: await this.$packageInstallationManager.getLatestVersion( diff --git a/test/package-installation-manager.ts b/test/package-installation-manager.ts index 23bceea684..7a1622fb90 100644 --- a/test/package-installation-manager.ts +++ b/test/package-installation-manager.ts @@ -67,12 +67,12 @@ function mockNpm( latestVersion: string ) { testInjector.register("npm", { - view: async (packageName: string, config: any): Promise => { - if (config.versions) { + view: async (packageName: string, field?: string): Promise => { + if (field === "versions") { return versions; } - throw new Error(`Unable to find propertyName ${config}.`); + throw new Error(`Unable to find propertyName ${field}.`); }, }); } diff --git a/test/services/android-plugin-build-service.ts b/test/services/android-plugin-build-service.ts index c2107cf4c1..fdd314138d 100644 --- a/test/services/android-plugin-build-service.ts +++ b/test/services/android-plugin-build-service.ts @@ -149,9 +149,9 @@ describe("androidPluginBuildService", () => { return result; }, - view: async (packageName: string, config: any): Promise => { + view: async (packageName: string, field?: string): Promise => { let result: any = null; - if (config && config.gradle) { + if (field === "gradle") { const packageNameParts = packageName.split("@"); const packageVersion = packageNameParts[packageNameParts.length - 1]; switch (packageVersion) { @@ -170,7 +170,7 @@ describe("androidPluginBuildService", () => { } } - if (config && config["dist-tags"]) { + if (field === "dist-tags") { result = { latest: "4.1.2", }; diff --git a/test/services/doctor-service.ts b/test/services/doctor-service.ts index 862480c02c..ec02c0f882 100644 --- a/test/services/doctor-service.ts +++ b/test/services/doctor-service.ts @@ -3,6 +3,8 @@ import { Yok } from "../../lib/common/yok"; import { LoggerStub, FileSystemStub } from "../stubs"; import { assert } from "chai"; import * as path from "path"; +import * as os from "os"; +import * as nodeFs from "fs"; import * as sinon from "sinon"; import * as _ from "lodash"; import { IProjectDataService } from "../../lib/definitions/project"; @@ -365,15 +367,54 @@ const Observable = require("tns-core-modules-widgets/data/observable").Observabl } }; - testData.forEach(({ filesContents, expectedShortImports }) => { - fs.readText = (filePath) => filesContents[filePath]; + const projectDir = nodeFs.mkdtempSync( + path.join(os.tmpdir(), "ns-doctor-service-"), + ); + const coreModulesDir = path.join( + projectDir, + "node_modules", + "tns-core-modules", + ); + nodeFs.mkdirSync(coreModulesDir, { recursive: true }); + nodeFs.writeFileSync( + path.join(coreModulesDir, "package.json"), + JSON.stringify({ name: "tns-core-modules", version: "6.0.0" }), + ); + + try { + testData.forEach(({ filesContents, expectedShortImports }) => { + fs.readText = (filePath) => filesContents[filePath]; + + const shortImports = doctorService.getDeprecatedShortImportsInFiles( + _.keys(filesContents), + projectDir, + ); + assert.deepStrictEqual(shortImports, expectedShortImports); + }); + } finally { + nodeFs.rmSync(projectDir, { recursive: true, force: true }); + } + }); + it("getDeprecatedShortImportsInFiles returns no results when tns-core-modules is not installed", () => { + const testInjector = createTestInjector(); + const doctorService = + testInjector.resolve("doctorService"); + const fs = testInjector.resolve("fs"); + fs.readText = () => 'const application = require("application");'; + + const projectDir = nodeFs.mkdtempSync( + path.join(os.tmpdir(), "ns-doctor-service-"), + ); + try { const shortImports = doctorService.getDeprecatedShortImportsInFiles( - _.keys(filesContents), - "projectDir", + ["file1"], + projectDir, ); - assert.deepStrictEqual(shortImports, expectedShortImports); - }); + assert.deepStrictEqual(shortImports, []); + } finally { + nodeFs.rmSync(projectDir, { recursive: true, force: true }); + } }); }); diff --git a/test/stubs.ts b/test/stubs.ts index 371295ca24..4793c06cbb 100644 --- a/test/stubs.ts +++ b/test/stubs.ts @@ -476,11 +476,11 @@ export class NodePackageManagerStub implements INodePackageManager { return ""; } - public async search(filter: string[], config: any): Promise { + public async search(filter: string[]): Promise { return ""; } - public async view(packageName: string, config: Object): Promise { + public async view(packageName: string, field?: string): Promise { return {}; } From c8d2a28a53cbd8926dc4c995f9ec63424b909223 Mon Sep 17 00:00:00 2001 From: Igor Randjelovic Date: Wed, 16 Sep 2026 12:10:16 +0200 Subject: [PATCH 4/6] feat(package-managers): resolve installed packages through the package manager Add getInstalledPackagePath(packageName, fromDir) to the package manager contract. The base implementation walks node_modules the way Node does; a package manager with a different on-disk layout can override it. Services whose call chains are already async now ask the package manager where a package lives instead of resolving it themselves: plugins-service, doctor short-import scan, versions-service, prepare-controller's runtime package.json lookup, android-plugin-build-service's local gradle versions, the preview command, test-init and PackageInstallationManager. Sites reached only from synchronous code (getRuntimePackage in project-data-service, the bundler executable lookup, the vitest and karma readiness checks, the transitive walk in node-modules-dependencies-builder) keep using the resolution helper directly. --- PublicAPI.md | 20 ++++++ lib/commands/preview.ts | 12 ++-- lib/commands/test-init.ts | 7 +- lib/contracts/doctor-service.ts | 4 +- lib/contracts/package-manager.ts | 11 +++ lib/controllers/prepare-controller.ts | 23 ++++--- lib/declarations.d.ts | 11 +++ lib/package-managers/base-package-manager.ts | 8 +++ lib/package-managers/index.ts | 9 +++ .../package-installation-manager.ts | 13 ++-- lib/services/android-plugin-build-service.ts | 20 +++--- lib/services/doctor-service.ts | 28 ++++---- lib/services/plugins-service.ts | 68 ++++++++----------- lib/services/versions-service.ts | 23 ++++--- test/contracts.ts | 2 +- test/controllers/prepare-controller.ts | 3 + test/package-manager-flags.ts | 31 +++++++++ test/services/android-plugin-build-service.ts | 6 ++ test/services/doctor-service.ts | 68 ++++++++----------- test/stubs.ts | 7 ++ 20 files changed, 239 insertions(+), 135 deletions(-) diff --git a/PublicAPI.md b/PublicAPI.md index 351d537418..02dfc2055f 100644 --- a/PublicAPI.md +++ b/PublicAPI.md @@ -571,6 +571,26 @@ tns.npm.search(["nativescript", "cloud"]).then(output => { }); ``` +### getInstalledPackagePath +Locates a package the way the selected package manager laid it out on disk, so callers never have to assume a `node_modules` layout. + +* Definition: +```TypeScript +/** + * @param {string} packageName The name of the package. + * @param {string} fromDir The directory whose dependencies are searched, usually the project directory. + * @return {Promise} The absolute path of the package directory, or null when it is not installed. + */ +getInstalledPackagePath(packageName: string, fromDir: string): Promise; +``` + +* Usage: +```JavaScript +tns.packageManager.getInstalledPackagePath("@nativescript/core", "/tmp/myProject").then(pathToPackage => { + console.log(pathToPackage ? `Installed at ${pathToPackage}` : "Not installed"); +}); +``` + ### view Provides information about a given package. diff --git a/lib/commands/preview.ts b/lib/commands/preview.ts index ce1444dd0b..7b5b183915 100644 --- a/lib/commands/preview.ts +++ b/lib/commands/preview.ts @@ -1,4 +1,3 @@ -import { resolvePackagePath } from "@rigor789/resolve-package-path"; import * as path from "path"; import { color } from "../color"; import { IChildProcess, IErrors } from "../common/declarations"; @@ -38,7 +37,7 @@ export class PreviewCommand extends Command({ await this.installLatestPreviewCLI(); } - const previewCLIPath = this.getPreviewCLIPath(); + const previewCLIPath = await this.getPreviewCLIPath(); if (!previewCLIPath) { await this.failMissingPreviewCLI(); @@ -59,10 +58,11 @@ export class PreviewCommand extends Command({ ); } - private getPreviewCLIPath(): string { - return resolvePackagePath(PREVIEW_CLI_PACKAGE, { - paths: [this.$projectData.projectDir], - }); + private getPreviewCLIPath(): Promise { + return this.$packageManager.getInstalledPackagePath( + PREVIEW_CLI_PACKAGE, + this.$projectData.projectDir, + ); } private async failMissingPreviewCLI(): Promise { diff --git a/lib/commands/test-init.ts b/lib/commands/test-init.ts index 8f33e62b01..015559ff36 100644 --- a/lib/commands/test-init.ts +++ b/lib/commands/test-init.ts @@ -2,7 +2,6 @@ import * as path from "path"; import * as _ from "lodash"; import { TESTING_FRAMEWORKS, ProjectTypes } from "../constants"; import { fromWindowsRelativePathToUnix } from "../common/helpers"; -import { resolvePackageJSONPath } from "../helpers/package-path-helper"; import { IProjectData, ITestInitializationService, @@ -138,8 +137,12 @@ export class TestInitCommand extends Command({ path: this.$options.path, }); + const modulePath = await this.$packageManager.getInstalledPackagePath( + mod.name, + projectDir, + ); const modulePackageJsonContent = this.$fs.readJson( - resolvePackageJSONPath(mod.name, { paths: [projectDir] }), + path.join(modulePath, "package.json"), ); const modulePeerDependencies = modulePackageJsonContent.peerDependencies || {}; diff --git a/lib/contracts/doctor-service.ts b/lib/contracts/doctor-service.ts index 9a64fbd7a4..4c5a42840e 100644 --- a/lib/contracts/doctor-service.ts +++ b/lib/contracts/doctor-service.ts @@ -34,5 +34,7 @@ export abstract class DoctorService { }): Promise; /** Checks and notifies users of deprecated short imports in their app. */ - abstract checkForDeprecatedShortImportsInAppDir(projectDir: string): void; + abstract checkForDeprecatedShortImportsInAppDir( + projectDir: string, + ): Promise; } diff --git a/lib/contracts/package-manager.ts b/lib/contracts/package-manager.ts index e5b09c8b57..2619e3297d 100644 --- a/lib/contracts/package-manager.ts +++ b/lib/contracts/package-manager.ts @@ -99,6 +99,17 @@ export abstract class PackageManager { */ abstract getCachePath(): Promise; + /** + * Locates a package the way the package manager laid it out on disk. + * @param {string} packageName The name of the package. + * @param {string} fromDir The directory whose dependencies are searched, usually the project directory. + * @return {Promise} The absolute path of the package directory, or null when it is not installed. + */ + abstract getInstalledPackagePath( + packageName: string, + fromDir: string, + ): Promise; + /** * Gets the name of the package manager used for the current process. * It can be read from the user settings or by passing -- option. diff --git a/lib/controllers/prepare-controller.ts b/lib/controllers/prepare-controller.ts index 4cc149ba04..f2e21afb1f 100644 --- a/lib/controllers/prepare-controller.ts +++ b/lib/controllers/prepare-controller.ts @@ -25,7 +25,11 @@ import { SupportedPlatform, TrackActionNames, } from "../constants"; -import { IOptions, IWatchIgnoreListService } from "../declarations"; +import { + IOptions, + IWatchIgnoreListService, + IPackageManager, +} from "../declarations"; import { INodeModulesDependenciesBuilder, IPlatformController, @@ -40,7 +44,6 @@ import { IProjectDataService, IProjectService, } from "../definitions/project"; -import { resolvePackageJSONPath } from "@rigor789/resolve-package-path"; interface IPlatformWatcherData { hasWebpackCompilerProcess: boolean; @@ -82,6 +85,7 @@ export class PrepareController private $markingModeService: IMarkingModeService, private $projectConfigService: IProjectConfigService, private $projectService: IProjectService, + private $packageManager: IPackageManager, ) { super(); } @@ -490,16 +494,15 @@ export class PrepareController SCOPED_ANDROID_RUNTIME_NAME; } // try reading from installed runtime first before reading from the npm registry... - const installedRuntimePackageJSONPath = resolvePackageJSONPath( - runtimePackageName, - { - paths: [projectData.projectDir], - }, - ); + const installedRuntimePath = + await this.$packageManager.getInstalledPackagePath( + runtimePackageName, + projectData.projectDir, + ); - if (installedRuntimePackageJSONPath) { + if (installedRuntimePath) { installedRuntimePackageJSON = this.$fs.readJson( - installedRuntimePackageJSONPath, + path.join(installedRuntimePath, "package.json"), ); } const packageData: any = { diff --git a/lib/declarations.d.ts b/lib/declarations.d.ts index cc3b1f33fe..c7c9c54126 100644 --- a/lib/declarations.d.ts +++ b/lib/declarations.d.ts @@ -111,6 +111,17 @@ interface INodePackageManager { * @returns {string} The full path to npm cache directory */ getCachePath(): Promise; + + /** + * Locates a package the way the package manager laid it out on disk. + * @param {string} packageName The name of the package. + * @param {string} fromDir The directory whose dependencies are searched, usually the project directory. + * @return {Promise} The absolute path of the package directory, or null when it is not installed. + */ + getInstalledPackagePath( + packageName: string, + fromDir: string, + ): Promise; } /** @deprecated Kept so existing annotations compile; use the {@link PackageManager} contract. */ diff --git a/lib/package-managers/base-package-manager.ts b/lib/package-managers/base-package-manager.ts index 3bcb7e52da..45a23d4e18 100644 --- a/lib/package-managers/base-package-manager.ts +++ b/lib/package-managers/base-package-manager.ts @@ -1,4 +1,5 @@ import { isInteractive } from "../common/helpers"; +import { resolvePackagePath } from "../helpers/package-path-helper"; import { INodePackageManager, IPackageInstallOptions, @@ -144,6 +145,13 @@ export abstract class BasePackageManager implements INodePackageManager { }; } + public async getInstalledPackagePath( + packageName: string, + fromDir: string, + ): Promise { + return resolvePackagePath(packageName, { paths: [fromDir] }) || null; + } + protected getInstallFlags(options: IPackageInstallOptions): string[] { return this.mapFlags(options, this.installFlags); } diff --git a/lib/package-managers/index.ts b/lib/package-managers/index.ts index cfbd6bbce7..d19f5f530c 100644 --- a/lib/package-managers/index.ts +++ b/lib/package-managers/index.ts @@ -108,6 +108,15 @@ export class PackageManager implements IPackageManager { return this.packageManager.getCachePath(); } + @exported("packageManager") + @invokeInit() + public getInstalledPackagePath( + packageName: string, + fromDir: string + ): Promise { + return this.packageManager.getInstalledPackagePath(packageName, fromDir); + } + public async getTagVersion( packageName: string, tag: string diff --git a/lib/package-managers/package-installation-manager.ts b/lib/package-managers/package-installation-manager.ts index 7fe8d9722a..ffa2b04e8b 100644 --- a/lib/package-managers/package-installation-manager.ts +++ b/lib/package-managers/package-installation-manager.ts @@ -1,6 +1,5 @@ import * as path from "path"; import * as constants from "../constants"; -import { resolvePackagePath } from "../helpers/package-path-helper"; import { INpmInstallOptions, INpmInstallResultInfo, @@ -190,9 +189,10 @@ export class PackageInstallationManager implements IPackageInstallationManager { projectDir: string ): Promise { // local installation takes precedence over cache - const inspectorPath = resolvePackagePath(inspectorNpmPackageName, { - paths: [projectDir], - }); + const inspectorPath = await this.$packageManager.getInstalledPackagePath( + inspectorNpmPackageName, + projectDir + ); if (inspectorPath) { return inspectorPath; } @@ -281,7 +281,10 @@ export class PackageInstallationManager implements IPackageInstallationManager { version, dev ); - return resolvePackagePath(installResultInfo.name, { paths: [pathToSave] }); + return this.$packageManager.getInstalledPackagePath( + installResultInfo.name, + pathToSave + ); } private async npmInstall( diff --git a/lib/services/android-plugin-build-service.ts b/lib/services/android-plugin-build-service.ts index 9dfd557271..31a4cecf96 100644 --- a/lib/services/android-plugin-build-service.ts +++ b/lib/services/android-plugin-build-service.ts @@ -36,7 +36,6 @@ import { IFilesHashService } from "../definitions/files-hash-service"; import { IInjector } from "../common/definitions/yok"; import { injector } from "../common/yok"; import * as _ from "lodash"; -import { resolvePackageJSONPath } from "@rigor789/resolve-package-path"; import { cwd } from "process"; export class AndroidPluginBuildService implements IAndroidPluginBuildService { @@ -507,7 +506,7 @@ export class AndroidPluginBuildService implements IAndroidPluginBuildService { return runtimeVersion; } - private getLocalGradleVersions(): IRuntimeGradleVersions { + private async getLocalGradleVersions(): Promise { // partial interface of the runtime package.json // including new 8.2+ format and legacy interface IRuntimePackageJSON { @@ -527,19 +526,18 @@ export class AndroidPluginBuildService implements IAndroidPluginBuildService { this.$projectData.nsConfig?.android?.runtimePackageName || SCOPED_ANDROID_RUNTIME_NAME; // try reading from installed runtime first before reading from the npm registry... - const installedRuntimePackageJSONPath = resolvePackageJSONPath( - packageName, - { - paths: [this.$projectData.projectDir], - }, - ); + const installedRuntimePath = + await this.$packageManager.getInstalledPackagePath( + packageName, + this.$projectData.projectDir, + ); - if (!installedRuntimePackageJSONPath) { + if (!installedRuntimePath) { return null; } const installedRuntimePackageJSON: IRuntimePackageJSON = this.$fs.readJson( - installedRuntimePackageJSONPath, + path.join(installedRuntimePath, "package.json"), ); if (!installedRuntimePackageJSON) { @@ -575,7 +573,7 @@ export class AndroidPluginBuildService implements IAndroidPluginBuildService { versions: { gradle: string; gradleAndroid: string }; } = null; - const localVersionInfo = this.getLocalGradleVersions(); + const localVersionInfo = await this.getLocalGradleVersions(); if (localVersionInfo) { return localVersionInfo; diff --git a/lib/services/doctor-service.ts b/lib/services/doctor-service.ts index f5d188aef6..3a2b355713 100644 --- a/lib/services/doctor-service.ts +++ b/lib/services/doctor-service.ts @@ -1,6 +1,5 @@ import { EOL } from "os"; import * as path from "path"; -import { resolvePackagePath } from "../helpers/package-path-helper"; import * as _ from "lodash"; import * as helpers from "../common/helpers"; import { cache } from "../common/decorators"; @@ -8,7 +7,7 @@ import { TrackActionNames, TNS_CORE_MODULES_NAME } from "../constants"; import { DoctorService } from "../contracts/doctor-service"; import { doctor, constants } from "@nativescript/doctor"; import { IProjectDataService } from "../definitions/project"; -import { IVersionsService, IOptions } from "../declarations"; +import { IVersionsService, IOptions, IPackageManager } from "../declarations"; import { IPlatformEnvironmentRequirements } from "../definitions/platform"; import { IAnalyticsService, @@ -70,6 +69,7 @@ export class DoctorServiceImpl implements DoctorService { private $terminalSpinnerService: ITerminalSpinnerService, private $versionsService: IVersionsService, private $settingsService: ISettingsService, + private $packageManager: IPackageManager, ) {} public async printWarnings(configOptions?: { @@ -139,7 +139,7 @@ export class DoctorServiceImpl implements DoctorService { } // todo: check for deprecated imports from `tns-core-modules` - this.checkForDeprecatedShortImportsInAppDir(configOptions.projectDir); + await this.checkForDeprecatedShortImportsInAppDir(configOptions.projectDir); await this.$injector .resolve( @@ -241,12 +241,14 @@ export class DoctorServiceImpl implements DoctorService { return !hasWarnings; } - public checkForDeprecatedShortImportsInAppDir(projectDir: string): void { + public async checkForDeprecatedShortImportsInAppDir( + projectDir: string, + ): Promise { if (projectDir) { try { const files = this.$projectDataService.getAppExecutableFiles(projectDir); - const shortImports = this.getDeprecatedShortImportsInFiles( + const shortImports = await this.getDeprecatedShortImportsInFiles( files, projectDir, ); @@ -269,11 +271,11 @@ export class DoctorServiceImpl implements DoctorService { } } - protected getDeprecatedShortImportsInFiles( + protected async getDeprecatedShortImportsInFiles( files: string[], projectDir: string, - ): { file: string; line: string }[] { - const shortImportRegExp = this.getShortImportRegExp(projectDir); + ): Promise<{ file: string; line: string }[]> { + const shortImportRegExp = await this.getShortImportRegExp(projectDir); const shortImports: { file: string; line: string }[] = []; if (!shortImportRegExp) { return shortImports; @@ -304,10 +306,12 @@ export class DoctorServiceImpl implements DoctorService { return shortImports; } - private getShortImportRegExp(projectDir: string): RegExp { - const pathToTnsCoreModules = resolvePackagePath(TNS_CORE_MODULES_NAME, { - paths: [projectDir], - }); + private async getShortImportRegExp(projectDir: string): Promise { + const pathToTnsCoreModules = + await this.$packageManager.getInstalledPackagePath( + TNS_CORE_MODULES_NAME, + projectDir, + ); if (!pathToTnsCoreModules) { return null; } diff --git a/lib/services/plugins-service.ts b/lib/services/plugins-service.ts index 7ba2bc593c..dffe4497b5 100644 --- a/lib/services/plugins-service.ts +++ b/lib/services/plugins-service.ts @@ -32,10 +32,6 @@ import { IFilesHashService } from "../definitions/files-hash-service"; import * as _ from "lodash"; import { IInjector } from "../common/definitions/yok"; import { injector } from "../common/yok"; -import { - resolvePackagePath, - resolvePackageJSONPath, -} from "../helpers/package-path-helper"; import { color } from "../color"; export class PluginsService implements IPluginsService { @@ -100,7 +96,7 @@ export class PluginsService implements IPluginsService { this.npmInstallOptions, ) ).name; - const pathToRealNpmPackageJson = this.getPackageJsonFilePathForModule( + const pathToRealNpmPackageJson = await this.getPackageJsonFilePathForModule( name, projectData.projectDir, ); @@ -148,7 +144,7 @@ export class PluginsService implements IPluginsService { platformData: IPlatformData, ): Promise => { const pluginData = this.convertToPluginData( - this.getNodeModuleData(pluginName, projectData.projectDir), + await this.getNodeModuleData(pluginName, projectData.projectDir), projectData.projectDir, ); @@ -298,24 +294,18 @@ export class PluginsService implements IPluginsService { _.keys(packageJsonContent.devDependencies), ); - const notInstalledDependencies = allDependencies - .map((dep) => { - this.$logger.trace(`Checking if ${dep} is installed...`); - const pathToPackage = resolvePackagePath(dep, { - paths: [projectData.projectDir], - }); - - if (pathToPackage) { - // return false if the dependency is installed - we'll filter out boolean values - // and end up with an array of dep names that are not installed if we end up - // inside the catch block. - return false; - } - + const notInstalledDependencies: string[] = []; + for (const dep of allDependencies) { + this.$logger.trace(`Checking if ${dep} is installed...`); + const pathToPackage = await this.$packageManager.getInstalledPackagePath( + dep, + projectData.projectDir, + ); + if (!pathToPackage) { this.$logger.trace(`${dep} is not installed, or couldn't be found`); - return dep; - }) - .filter(Boolean); + notInstalledDependencies.push(dep); + } + } if (this.$options.force || notInstalledDependencies.length) { this.$logger.trace( @@ -635,9 +625,7 @@ This framework comes from ${dependencyName} plugin, which is installed multiple pluginData.version = cacheData.version; pluginData.fullPath = (cacheData).directory || - path.dirname( - this.getPackageJsonFilePathForModule(cacheData.name, projectDir), - ); + (cacheData).fullPath; pluginData.isPlugin = !!cacheData.nativescript; pluginData.pluginPlatformsFolderPath = (platform: string) => { if (this.$mobileHelper.isvisionOSPlatform(platform)) { @@ -706,14 +694,15 @@ This framework comes from ${dependencyName} plugin, which is installed multiple return path.join(projectDir, "package.json"); } - private getPackageJsonFilePathForModule( + private async getPackageJsonFilePathForModule( moduleName: string, projectDir: string, - ): string { - const pathToJsonFile = resolvePackageJSONPath(moduleName, { - paths: [projectDir], - }); - return pathToJsonFile; + ): Promise { + const pathToModule = await this.$packageManager.getInstalledPackagePath( + moduleName, + projectDir, + ); + return pathToModule && path.join(pathToModule, "package.json"); } private getDependencies(projectDir: string): string[] { @@ -721,13 +710,13 @@ This framework comes from ${dependencyName} plugin, which is installed multiple return _.keys(require(packageJsonFilePath).dependencies); } - private getNodeModuleData( + private async getNodeModuleData( module: string, projectDir: string, - ): INodeModuleData { + ): Promise { // module can be modulePath or moduleName if (!this.$fs.exists(module) || path.basename(module) !== "package.json") { - const resolvedPath = this.getPackageJsonFilePathForModule( + const resolvedPath = await this.getPackageJsonFilePathForModule( module, projectDir, ); @@ -756,9 +745,12 @@ This framework comes from ${dependencyName} plugin, which is installed multiple await this.ensureAllDependenciesAreInstalled(projectData); const nodeModules = this.getDependencies(projectData.projectDir); - return _.map(nodeModules, (nodeModuleName) => - this.getNodeModuleData(nodeModuleName, projectData.projectDir), - ).filter(Boolean); + const modules = await Promise.all( + nodeModules.map((nodeModuleName) => + this.getNodeModuleData(nodeModuleName, projectData.projectDir), + ), + ); + return modules.filter(Boolean); } private async executeNpmCommand( diff --git a/lib/services/versions-service.ts b/lib/services/versions-service.ts index e9e93a0f6d..dc14694ca1 100644 --- a/lib/services/versions-service.ts +++ b/lib/services/versions-service.ts @@ -2,8 +2,11 @@ import * as constants from "../constants"; import * as helpers from "../common/helpers"; import * as semver from "semver"; import * as path from "path"; -import { resolvePackagePath } from "../helpers/package-path-helper"; -import { IVersionsService, IPackageInstallationManager } from "../declarations"; +import { + IVersionsService, + IPackageInstallationManager, + IPackageManager, +} from "../declarations"; import { IProjectData, IProjectDataService } from "../definitions/project"; import { IPluginsService, IBasePluginData } from "../definitions/plugins"; import { IFileSystem, IVersionInformation } from "../common/declarations"; @@ -29,6 +32,7 @@ class VersionsService implements IVersionsService { constructor( private $fs: IFileSystem, private $packageInstallationManager: IPackageInstallationManager, + private $packageManager: IPackageManager, private $injector: IInjector, private $logger: ILogger, private $staticConfig: Config.IStaticConfig, @@ -65,11 +69,12 @@ class VersionsService implements IVersionsService { if (this.projectData) { const resolve = (packageName: string) => - resolvePackagePath(packageName, { - paths: [this.projectData.projectDir], - }); - let scopedPackagePath = resolve(constants.SCOPED_TNS_CORE_MODULES); - let tnsCoreModulesPath = resolve(constants.TNS_CORE_MODULES_NAME); + this.$packageManager.getInstalledPackagePath( + packageName, + this.projectData.projectDir + ); + let scopedPackagePath = await resolve(constants.SCOPED_TNS_CORE_MODULES); + let tnsCoreModulesPath = await resolve(constants.TNS_CORE_MODULES_NAME); const dependsOnNonScopedPackage = !!this.projectData.dependencies[ constants.TNS_CORE_MODULES_NAME @@ -86,8 +91,8 @@ class VersionsService implements IVersionsService { await this.$pluginsService.ensureAllDependenciesAreInstalled( this.projectData ); - scopedPackagePath = resolve(constants.SCOPED_TNS_CORE_MODULES); - tnsCoreModulesPath = resolve(constants.TNS_CORE_MODULES_NAME); + scopedPackagePath = await resolve(constants.SCOPED_TNS_CORE_MODULES); + tnsCoreModulesPath = await resolve(constants.TNS_CORE_MODULES_NAME); } if (dependsOnNonScopedPackage && tnsCoreModulesPath) { diff --git a/test/contracts.ts b/test/contracts.ts index e07581e01e..1be08d6c2a 100644 --- a/test/contracts.ts +++ b/test/contracts.ts @@ -60,7 +60,7 @@ describe("contracts tranche", () => { async canExecuteLocalBuild(): Promise { return true; } - checkForDeprecatedShortImportsInAppDir(): void {} + async checkForDeprecatedShortImportsInAppDir(): Promise {} } const injector = new Injector([provide(DoctorService, StubDoctorService)]); diff --git a/test/controllers/prepare-controller.ts b/test/controllers/prepare-controller.ts index e3982de1e4..050e8bf5b0 100644 --- a/test/controllers/prepare-controller.ts +++ b/test/controllers/prepare-controller.ts @@ -51,6 +51,9 @@ function createTestInjector(data: { hasNativeChanges: boolean }): IInjector { injector.register("mobileHelper", MobileHelper); injector.register("prepareController", PrepareController); + injector.register("packageManager", { + getInstalledPackagePath: async (): Promise => null, + }); injector.register("nodeModulesDependenciesBuilder", { getProductionDependencies: () => [], diff --git a/test/package-manager-flags.ts b/test/package-manager-flags.ts index a2bbb0c9fb..27e9a9ac11 100644 --- a/test/package-manager-flags.ts +++ b/test/package-manager-flags.ts @@ -223,6 +223,37 @@ describe("package manager flag mapping", () => { }); }); + describe("getInstalledPackagePath", () => { + const repoRoot = path.join(__dirname, "..", ".."); + + for (const { name, ctor } of managers) { + it(`${name} resolves an installed package from the given directory`, async () => { + const manager = createTestInjector( + name, + ctor, + ).resolve(name); + const resolved = await manager.getInstalledPackagePath( + "lodash", + repoRoot, + ); + assert.equal(resolved, path.join(repoRoot, "node_modules", "lodash")); + }); + + it(`${name} returns null for a package that is not installed`, async () => { + const manager = createTestInjector( + name, + ctor, + ).resolve(name); + assert.isNull( + await manager.getInstalledPackagePath( + "definitely-not-installed-package", + repoRoot, + ), + ); + }); + } + }); + describe("uninstall", () => { const expected: { [name: string]: string } = { npm: "npm uninstall left-pad --save", diff --git a/test/services/android-plugin-build-service.ts b/test/services/android-plugin-build-service.ts index fdd314138d..0f8332d5c9 100644 --- a/test/services/android-plugin-build-service.ts +++ b/test/services/android-plugin-build-service.ts @@ -10,6 +10,7 @@ import * as FsLib from "../../lib/common/file-system"; import * as path from "path"; import * as stubs from "../stubs"; import { mkdtempSync } from "fs"; +import { resolvePackagePath } from "../../lib/helpers/package-path-helper"; import { tmpdir } from "os"; import { IFileSystem, @@ -128,6 +129,11 @@ describe("androidPluginBuildService", () => { addProjectRuntime?: boolean; }): any { return { + getInstalledPackagePath: async ( + packageName: string, + fromDir: string, + ): Promise => + resolvePackagePath(packageName, { paths: [fromDir] }) || null, getRegistryPackageData: async (packageName: string): Promise => { const result: any = []; result["dist-tags"] = { latest: "4.1.2" }; diff --git a/test/services/doctor-service.ts b/test/services/doctor-service.ts index ec02c0f882..efe7654871 100644 --- a/test/services/doctor-service.ts +++ b/test/services/doctor-service.ts @@ -3,12 +3,10 @@ import { Yok } from "../../lib/common/yok"; import { LoggerStub, FileSystemStub } from "../stubs"; import { assert } from "chai"; import * as path from "path"; -import * as os from "os"; -import * as nodeFs from "fs"; import * as sinon from "sinon"; import * as _ from "lodash"; import { IProjectDataService } from "../../lib/definitions/project"; -import { IVersionsService } from "../../lib/declarations"; +import { IVersionsService, IPackageManager } from "../../lib/declarations"; import { ICheckEnvironmentRequirementsInput, ICheckEnvironmentRequirementsOutput, @@ -47,6 +45,7 @@ class DoctorServiceInheritor extends DoctorService { $terminalSpinnerService: ITerminalSpinnerService, $versionsService: IVersionsService, $settingsService: ISettingsService, + $packageManager: IPackageManager, ) { super( $analyticsService, @@ -59,13 +58,14 @@ class DoctorServiceInheritor extends DoctorService { $terminalSpinnerService, $versionsService, $settingsService, + $packageManager, ); } public getDeprecatedShortImportsInFiles( files: string[], projectDir: string, - ): { file: string; line: string }[] { + ): Promise<{ file: string; line: string }[]> { return super.getDeprecatedShortImportsInFiles(files, projectDir); } } @@ -95,6 +95,15 @@ describe("doctorService", () => { }, }); testInjector.register("versionsService", {}); + testInjector.register("packageManager", { + getInstalledPackagePath: async ( + packageName: string, + fromDir: string, + ): Promise => + packageName === "tns-core-modules" + ? path.join(fromDir, "node_modules", packageName) + : null, + }); testInjector.register("settingsService", { getProfileDir: (): string => "", }); @@ -351,7 +360,7 @@ const Observable = require("tns-core-modules-widgets/data/observable").Observabl }, ]; - it("getDeprecatedShortImportsInFiles returns correct results", () => { + it("getDeprecatedShortImportsInFiles returns correct results", async () => { const testInjector = createTestInjector(); const doctorService = testInjector.resolve("doctorService"); @@ -367,54 +376,33 @@ const Observable = require("tns-core-modules-widgets/data/observable").Observabl } }; - const projectDir = nodeFs.mkdtempSync( - path.join(os.tmpdir(), "ns-doctor-service-"), - ); - const coreModulesDir = path.join( - projectDir, - "node_modules", - "tns-core-modules", - ); - nodeFs.mkdirSync(coreModulesDir, { recursive: true }); - nodeFs.writeFileSync( - path.join(coreModulesDir, "package.json"), - JSON.stringify({ name: "tns-core-modules", version: "6.0.0" }), - ); + for (const { filesContents, expectedShortImports } of testData) { + fs.readText = (filePath) => filesContents[filePath]; - try { - testData.forEach(({ filesContents, expectedShortImports }) => { - fs.readText = (filePath) => filesContents[filePath]; - - const shortImports = doctorService.getDeprecatedShortImportsInFiles( + const shortImports = + await doctorService.getDeprecatedShortImportsInFiles( _.keys(filesContents), - projectDir, + "projectDir", ); - assert.deepStrictEqual(shortImports, expectedShortImports); - }); - } finally { - nodeFs.rmSync(projectDir, { recursive: true, force: true }); + assert.deepStrictEqual(shortImports, expectedShortImports); } }); - it("getDeprecatedShortImportsInFiles returns no results when tns-core-modules is not installed", () => { + it("getDeprecatedShortImportsInFiles returns no results when tns-core-modules is not installed", async () => { const testInjector = createTestInjector(); + const packageManager = testInjector.resolve("packageManager"); + packageManager.getInstalledPackagePath = async (): Promise => + null; const doctorService = testInjector.resolve("doctorService"); const fs = testInjector.resolve("fs"); fs.readText = () => 'const application = require("application");'; - const projectDir = nodeFs.mkdtempSync( - path.join(os.tmpdir(), "ns-doctor-service-"), + const shortImports = await doctorService.getDeprecatedShortImportsInFiles( + ["file1"], + "projectDir", ); - try { - const shortImports = doctorService.getDeprecatedShortImportsInFiles( - ["file1"], - projectDir, - ); - assert.deepStrictEqual(shortImports, []); - } finally { - nodeFs.rmSync(projectDir, { recursive: true, force: true }); - } + assert.deepStrictEqual(shortImports, []); }); }); diff --git a/test/stubs.ts b/test/stubs.ts index 4793c06cbb..533714ee1f 100644 --- a/test/stubs.ts +++ b/test/stubs.ts @@ -457,6 +457,13 @@ export class PackageInstallationManagerStub implements IPackageInstallationManag export class NodePackageManagerStub implements INodePackageManager { constructor() {} + public async getInstalledPackagePath( + packageName: string, + fromDir: string, + ): Promise { + return null; + } + public async install( packageName: string, pathToSave: string, From 14a41fa58466e37a16595f5ddcf45c45581d9753 Mon Sep 17 00:00:00 2001 From: Igor Randjelovic Date: Wed, 16 Sep 2026 12:45:35 +0200 Subject: [PATCH 5/6] refactor: resolve bundler, test runner and extension packages via the package manager The bundler executable lookup, the vitest and karma readiness checks and the extensibility service's "is this extension installed?" check now ask the package manager where a package lives. Their tests stub that one method instead of faking directory listings or patching Node's module resolution. getRuntimePackage in project-data-service and the transitive walk in node-modules-dependencies-builder stay on the resolution helper: both feed synchronous code paths (getPlatformData, getAllProductionPlugins) with dozens of callers, and threading async through those is a separate change. --- lib/commands/test.ts | 2 +- lib/definitions/project.d.ts | 2 +- .../bundler/bundler-compiler-service.ts | 50 ++++++------- lib/services/extensibility-service.ts | 7 +- lib/services/test-execution-service.ts | 11 +-- lib/services/vitest-execution-service.ts | 20 +++-- test/extension-manifests.ts | 6 ++ .../bundler/bundler-compiler-service.ts | 1 + test/services/extensibility-service.ts | 74 ++++++++----------- test/services/test-execution-service.ts | 59 ++++++--------- 10 files changed, 111 insertions(+), 121 deletions(-) diff --git a/lib/commands/test.ts b/lib/commands/test.ts index 0829bb97ec..6b7f1743e2 100644 --- a/lib/commands/test.ts +++ b/lib/commands/test.ts @@ -106,7 +106,7 @@ async function canExecuteTestCommand( if ($vitestExecutionService.isVitestProject($projectData)) { const canStartTestRun = - $vitestExecutionService.canStartTestRun($projectData); + await $vitestExecutionService.canStartTestRun($projectData); if (!canStartTestRun) { $errors.fail({ formatStr: diff --git a/lib/definitions/project.d.ts b/lib/definitions/project.d.ts index cd2a931617..468bf6c66d 100644 --- a/lib/definitions/project.d.ts +++ b/lib/definitions/project.d.ts @@ -522,7 +522,7 @@ interface ITestExecutionService { interface IVitestExecutionService { isVitestProject(projectData: IProjectData): boolean; - canStartTestRun(projectData: IProjectData): boolean; + canStartTestRun(projectData: IProjectData): Promise; startTestRun(platform: string, projectData: IProjectData): Promise; } diff --git a/lib/services/bundler/bundler-compiler-service.ts b/lib/services/bundler/bundler-compiler-service.ts index ccf7df4bb7..b613860817 100644 --- a/lib/services/bundler/bundler-compiler-service.ts +++ b/lib/services/bundler/bundler-compiler-service.ts @@ -35,10 +35,6 @@ import { import { ICleanupService } from "../../definitions/cleanup-service"; import { ViteHmrPortService } from "../../contracts/vite-hmr-port-service"; import { injector } from "../../common/yok"; -import { - resolvePackagePath, - resolvePackageJSONPath, -} from "../../helpers/package-path-helper"; // todo: move out of here interface IBundlerMessage { @@ -573,10 +569,13 @@ export class BundlerCompilerService additionalNodeArgs.unshift("--max_old_space_size=4096"); } + const bundlerExecutablePath = + await this.getBundlerExecutablePath(projectData); + const isModernBundler = await this.isModernBundler(projectData); const args = [ ...additionalNodeArgs, - this.getBundlerExecutablePath(projectData), - isVite || this.isModernBundler(projectData) ? "build" : null, + bundlerExecutablePath, + isVite || isModernBundler ? "build" : null, `--config=${projectData.bundlerConfigPath}`, ...envParams, ].filter(Boolean); @@ -726,7 +725,7 @@ export class BundlerCompilerService // go after `--` so vite's CLI doesn't choke on unknown options. const args = [ ...additionalNodeArgs, - this.getBundlerExecutablePath(projectData), + await this.getBundlerExecutablePath(projectData), "serve", `--config=${projectData.bundlerConfigPath}`, `--mode=development`, @@ -1166,21 +1165,24 @@ export class BundlerCompilerService }); } - private getBundlerExecutablePath(projectData: IProjectData): string { + private async getBundlerExecutablePath( + projectData: IProjectData, + ): Promise { const bundler = this.getBundler(); + const resolve = (packageName: string) => + this.$packageManager.getInstalledPackagePath( + packageName, + projectData.projectDir, + ); if (bundler === "vite") { - const packagePath = resolvePackagePath(`vite`, { - paths: [projectData.projectDir], - }); + const packagePath = await resolve("vite"); if (packagePath) { return path.resolve(packagePath, "bin", "vite.js"); } - } else if (this.isModernBundler(projectData)) { - const packagePath = resolvePackagePath(this.getBundlerPackageName(), { - paths: [projectData.projectDir], - }); + } else if (await this.isModernBundler(projectData)) { + const packagePath = await resolve(this.getBundlerPackageName()); if (packagePath) { return path.resolve(packagePath, "dist", "bin", "index.js"); @@ -1200,9 +1202,7 @@ export class BundlerCompilerService ); } - const packagePath = resolvePackagePath("webpack", { - paths: [projectData.projectDir], - }); + const packagePath = await resolve("webpack"); if (!packagePath) { return ""; @@ -1224,21 +1224,21 @@ export class BundlerCompilerService ); } - private isModernBundler(projectData: IProjectData): boolean { + private async isModernBundler(projectData: IProjectData): Promise { const bundler = this.getBundler(); switch (bundler) { case "rspack": return true; default: - const packageJSONPath = resolvePackageJSONPath( + const packagePath = await this.$packageManager.getInstalledPackagePath( this.getBundlerPackageName(), - { - paths: [projectData.projectDir], - }, + projectData.projectDir, ); - if (packageJSONPath) { - const packageData = this.$fs.readJson(packageJSONPath); + if (packagePath) { + const packageData = this.$fs.readJson( + path.join(packagePath, "package.json"), + ); const ver = semver.coerce(packageData.version); if (semver.satisfies(ver, ">= 5.0.0")) { diff --git a/lib/services/extensibility-service.ts b/lib/services/extensibility-service.ts index 4a9b123c9d..aee3ef6cbf 100644 --- a/lib/services/extensibility-service.ts +++ b/lib/services/extensibility-service.ts @@ -512,11 +512,12 @@ export class ExtensibilityService implements IExtensibilityService { extensionName: string, ): Promise { this.$logger.trace(`Asserting extension ${extensionName} is installed.`); - const installedExtensions = this.$fs.readDirectory( - path.join(this.pathToExtensions, constants.NODE_MODULES_FOLDER_NAME), + const installedPath = await this.$packageManager.getInstalledPackagePath( + extensionName, + this.pathToExtensions, ); - if (installedExtensions.indexOf(extensionName) === -1) { + if (!installedPath) { this.$logger.trace( `Extension ${extensionName} is not installed, starting installation.`, ); diff --git a/lib/services/test-execution-service.ts b/lib/services/test-execution-service.ts index 8ba4eda276..e640dcf1fe 100644 --- a/lib/services/test-execution-service.ts +++ b/lib/services/test-execution-service.ts @@ -7,14 +7,13 @@ import { IProjectDataService, IProjectData, } from "../definitions/project"; -import { IConfiguration, IOptions } from "../declarations"; +import { IConfiguration, IOptions, IPackageManager } from "../declarations"; import { IPluginsService } from "../definitions/plugins"; import { Server, IFileSystem, IChildProcess } from "../common/declarations"; import { ErrorCodes } from "../common/enums"; import * as _ from "lodash"; import { injector } from "../common/yok"; import { ICommandParameter } from "../common/definitions/commands"; -import { resolvePackagePath } from "../helpers/package-path-helper"; interface IKarmaConfigOptions { debugBrk: boolean; @@ -36,6 +35,7 @@ export class TestExecutionService implements ITestExecutionService { private $pluginsService: IPluginsService, private $projectDataService: IProjectDataService, private $childProcess: IChildProcess, + private $packageManager: IPackageManager, ) {} public platform: string; @@ -144,9 +144,10 @@ export class TestExecutionService implements ITestExecutionService { } }); - const pathToKarma = resolvePackagePath("karma", { - paths: [projectData.projectDir], - }); + const pathToKarma = await this.$packageManager.getInstalledPackagePath( + "karma", + projectData.projectDir, + ); canStartKarmaServer = canStartKarmaServer && !!pathToKarma; diff --git a/lib/services/vitest-execution-service.ts b/lib/services/vitest-execution-service.ts index f4811780e8..41b5593791 100644 --- a/lib/services/vitest-execution-service.ts +++ b/lib/services/vitest-execution-service.ts @@ -1,9 +1,8 @@ import * as path from "path"; import { IProjectData, IVitestExecutionService } from "../definitions/project"; -import { IOptions } from "../declarations"; +import { IOptions, IPackageManager } from "../declarations"; import { IChildProcess, IErrors, IFileSystem } from "../common/declarations"; import { injector } from "../common/yok"; -import { resolvePackagePath } from "../helpers/package-path-helper"; const VITEST_CONFIG_FILES = [ "vitest.config.mts", @@ -19,16 +18,17 @@ export class VitestExecutionService implements IVitestExecutionService { private $fs: IFileSystem, private $logger: ILogger, private $options: IOptions, + private $packageManager: IPackageManager, ) {} public isVitestProject(projectData: IProjectData): boolean { return !!this.getConfigPath(projectData); } - public canStartTestRun(projectData: IProjectData): boolean { + public async canStartTestRun(projectData: IProjectData): Promise { return ( this.isVitestProject(projectData) && - !!resolvePackagePath("vitest", { paths: [projectData.projectDir] }) + !!(await this.getVitestPackagePath(projectData)) ); } @@ -36,9 +36,7 @@ export class VitestExecutionService implements IVitestExecutionService { platform: string, projectData: IProjectData, ): Promise { - const vitestPackagePath = resolvePackagePath("vitest", { - paths: [projectData.projectDir], - }); + const vitestPackagePath = await this.getVitestPackagePath(projectData); if (!vitestPackagePath) { this.$errors.fail( "Unable to find 'vitest' in the project. Run '$ ns test init --framework vitest' first.", @@ -90,6 +88,14 @@ export class VitestExecutionService implements IVitestExecutionService { } return null; } + + private getVitestPackagePath(projectData: IProjectData): Promise { + return this.$packageManager.getInstalledPackagePath( + "vitest", + projectData.projectDir, + ); + } + } injector.register("vitestExecutionService", VitestExecutionService); diff --git a/test/extension-manifests.ts b/test/extension-manifests.ts index 9a213310d9..faaba51cfa 100644 --- a/test/extension-manifests.ts +++ b/test/extension-manifests.ts @@ -1,4 +1,5 @@ import { assert } from "chai"; +import { resolvePackagePath } from "../lib/helpers/package-path-helper"; import * as fs from "fs"; import * as os from "os"; import * as path from "path"; @@ -184,6 +185,11 @@ describe("extension manifests", () => { install: async (): Promise => { throw new Error("Extensions are expected to be installed already."); }, + getInstalledPackagePath: async ( + packageName: string, + fromDir: string, + ): Promise => + resolvePackagePath(packageName, { paths: [fromDir] }) || null, uninstall: async (): Promise => undefined, searchNpms: async (): Promise => ({ results: [] }), getRegistryPackageData: async (): Promise => ({}), diff --git a/test/services/bundler/bundler-compiler-service.ts b/test/services/bundler/bundler-compiler-service.ts index 1ec3a6b057..36fe0bf39b 100644 --- a/test/services/bundler/bundler-compiler-service.ts +++ b/test/services/bundler/bundler-compiler-service.ts @@ -58,6 +58,7 @@ function createTestInjector( const testInjector = new Yok(); testInjector.register("packageManager", { getPackageManagerName: async () => packageManager, + getInstalledPackagePath: async (): Promise => null, }); testInjector.register("bundlerCompilerService", BundlerCompilerService); testInjector.register("childProcess", {}); diff --git a/test/services/extensibility-service.ts b/test/services/extensibility-service.ts index 0ea1ba29af..eae286db8d 100644 --- a/test/services/extensibility-service.ts +++ b/test/services/extensibility-service.ts @@ -87,6 +87,17 @@ describe("extensibilityService", () => { return testInjector; }; + const stubInstalledExtensions = ( + testInjector: IInjector, + resolve: (extensionName: string, fromDir: string) => string, + ): void => { + const packageManager = testInjector.resolve("packageManager"); + packageManager.getInstalledPackagePath = async ( + packageName: string, + fromDir: string, + ): Promise => resolve(packageName, fromDir); + }; + const getExpectedInstallationPathForExtension = ( testInjector: IInjector, extensionName: string, @@ -320,14 +331,9 @@ describe("extensibilityService", () => { const fs: IFileSystem = testInjector.resolve("fs"); const extensionNames = ["extension1", "extension2", "extension3"]; fs.exists = (pathToCheck: string): boolean => true; - fs.readDirectory = (dir: string): string[] => { - assert.deepStrictEqual( - path.basename(dir), - constants.NODE_MODULES_FOLDER_NAME, - ); - // Simulates extensions are installed in node_modules - return extensionNames; - }; + stubInstalledExtensions(testInjector, (name, fromDir) => + path.join(fromDir, constants.NODE_MODULES_FOLDER_NAME, name), + ); mockFsReadJson(testInjector, extensionNames); @@ -359,20 +365,15 @@ describe("extensibilityService", () => { fs.exists = (pathToCheck: string): boolean => path.basename(pathToCheck) !== extensionNames[0]; - let isFirstReadDirExecution = true; - fs.readDirectory = (dir: string): string[] => { - assert.deepStrictEqual( - path.basename(dir), - constants.NODE_MODULES_FOLDER_NAME, - ); - // Simulates extensions are installed in node_modules - if (isFirstReadDirExecution) { - isFirstReadDirExecution = false; - return extensionNames.filter((ext) => ext !== "extension1"); - } else { - return extensionNames; + // extension1 is missing until the service installs it + let isExtension1Installed = false; + stubInstalledExtensions(testInjector, (name, fromDir) => { + if (name === "extension1" && !isExtension1Installed) { + isExtension1Installed = true; + return null; } - }; + return path.join(fromDir, constants.NODE_MODULES_FOLDER_NAME, name); + }); mockFsReadJson(testInjector, extensionNames); @@ -415,14 +416,9 @@ describe("extensibilityService", () => { const fs: IFileSystem = testInjector.resolve("fs"); const extensionNames = ["extension1", "extension2", "extension3"]; fs.exists = (pathToCheck: string): boolean => true; - fs.readDirectory = (dir: string): string[] => { - assert.deepStrictEqual( - path.basename(dir), - constants.NODE_MODULES_FOLDER_NAME, - ); - // Simulates extensions are installed in node_modules - return extensionNames; - }; + stubInstalledExtensions(testInjector, (name, fromDir) => + path.join(fromDir, constants.NODE_MODULES_FOLDER_NAME, name), + ); mockFsReadJson(testInjector, extensionNames); @@ -469,7 +465,7 @@ describe("extensibilityService", () => { } }); - it("rejects all promises when unable to read node_modules dir (simulate EPERM error)", async () => { + it("rejects all promises when the package manager cannot locate extensions (simulate EPERM error)", async () => { const testInjector = getTestInjector(); const extensionNames = ["extension1", "extension2", "extension3"]; const fs: IFileSystem = testInjector.resolve("fs"); @@ -480,14 +476,10 @@ describe("extensibilityService", () => { mockFsReadJson(testInjector, extensionNames); let isReadDirCalled = false; - fs.readDirectory = (dir: string): string[] => { + stubInstalledExtensions(testInjector, () => { isReadDirCalled = true; - assert.deepStrictEqual( - path.basename(dir), - constants.NODE_MODULES_FOLDER_NAME, - ); throw new Error(expectedErrorMessage); - }; + }); const extensibilityService: IExtensibilityService = testInjector.resolve(ExtensibilityService); @@ -532,14 +524,10 @@ describe("extensibilityService", () => { mockFsReadJson(testInjector, extensionNames); let isReadDirCalled = false; - fs.readDirectory = (dir: string): string[] => { + stubInstalledExtensions(testInjector, () => { isReadDirCalled = true; - assert.deepStrictEqual( - path.basename(dir), - constants.NODE_MODULES_FOLDER_NAME, - ); - return []; - }; + return null; + }); let isNpmInstallCalled = false; const npm: INodePackageManager = testInjector.resolve("npm"); diff --git a/test/services/test-execution-service.ts b/test/services/test-execution-service.ts index ad6ad5c62a..7daa0d3f7c 100644 --- a/test/services/test-execution-service.ts +++ b/test/services/test-execution-service.ts @@ -8,10 +8,21 @@ import { IDictionary } from "../../lib/common/declarations"; const karmaPluginName = "karma"; const unitTestsPluginName = "@nativescript/unit-test-runner"; -function getTestExecutionService(): ITestExecutionService { +function getTestExecutionService( + installedPackages: string[], +): ITestExecutionService { const injector = new InjectorStub(); injector.register("testExecutionService", TestExecutionService); injector.register("runController", {}); + injector.register("packageManager", { + getInstalledPackagePath: async ( + packageName: string, + fromDir: string, + ): Promise => + installedPackages.indexOf(packageName) !== -1 + ? `${fromDir}/node_modules/${packageName}` + : null, + }); return injector.resolve("testExecutionService"); } @@ -28,8 +39,7 @@ function getDependenciesObj(deps: string[]): IDictionary { describe("testExecutionService", () => { const testCases = [ { - name: - "should return false when the project has no dependencies and dev dependencies", + name: "should return false when the project has no dependencies and dev dependencies", expectedCanStartKarmaServer: false, projectData: { dependencies: {}, devDependencies: {} }, }, @@ -50,8 +60,7 @@ describe("testExecutionService", () => { }, }, { - name: - "should return true when the project has the required plugins as dependencies", + name: "should return true when the project has the required plugins as dependencies", expectedCanStartKarmaServer: true, projectData: { dependencies: getDependenciesObj([ @@ -62,8 +71,7 @@ describe("testExecutionService", () => { }, }, { - name: - "should return true when the project has the required plugins as dev dependencies", + name: "should return true when the project has the required plugins as dev dependencies", expectedCanStartKarmaServer: true, projectData: { dependencies: {}, @@ -74,8 +82,7 @@ describe("testExecutionService", () => { }, }, { - name: - "should return true when the project has the required plugins as dev and normal dependencies", + name: "should return true when the project has the required plugins as dev and normal dependencies", expectedCanStartKarmaServer: true, projectData: { dependencies: getDependenciesObj([karmaPluginName]), @@ -87,35 +94,15 @@ describe("testExecutionService", () => { describe("canStartKarmaServer", () => { _.each(testCases, (testCase: any) => { it(`${testCase.name}`, async () => { - const testExecutionService = getTestExecutionService(); - - // todo: cleanup monkey-patch with a friendlier syntax (util?) - // MOCK require.resolve - const Module = require("module"); - const originalResolveFilename = Module._resolveFilename; - - Module._resolveFilename = function (...args: any) { - if ( - args[0].startsWith(karmaPluginName) && - (testCase.projectData.dependencies[karmaPluginName] || - testCase.projectData.devDependencies[karmaPluginName]) - ) { - // override with a "random" built-in module to - // ensure the module can be resolved - args[0] = "fs"; - } + const installedPackages = _.keys({ + ...testCase.projectData.dependencies, + ...testCase.projectData.devDependencies, + }); + const testExecutionService = getTestExecutionService(installedPackages); - return originalResolveFilename.apply(this, args); - }; - // END MOCK - - const canStartKarmaServer = await testExecutionService.canStartKarmaServer( - testCase.projectData - ); + const canStartKarmaServer = + await testExecutionService.canStartKarmaServer(testCase.projectData); assert.equal(canStartKarmaServer, testCase.expectedCanStartKarmaServer); - - // restore mock - Module._resolveFilename = originalResolveFilename; }); }); }); From 2000f91e1079e7a2a9d2ce8f2050c69681aedd91 Mon Sep 17 00:00:00 2001 From: Igor Randjelovic Date: Wed, 16 Sep 2026 13:04:40 +0200 Subject: [PATCH 6/6] refactor(package-managers): select the package manager and resolve packages synchronously Nothing about locating an installed package is asynchronous; the only async link was the dispatcher reading the "packageManager" user setting through the settings lock. JsonFileSettingsService gains a lock-free getSettingValueSync for settings that only change through explicit user commands, and the dispatcher now picks its implementation lazily and synchronously, dropping the @cache/@invokeInit init dance. getInstalledPackagePath is therefore synchronous on the contract, which unwinds the async that had been threaded through doctor, plugins-service, the bundler, the test runners, preview and android-plugin-build-service, and lets the last two direct users of the resolution helper move onto the contract: getRuntimePackage in project-data-service (resolved lazily via the injector, as the service is constructed everywhere) and the transitive walk in node-modules-dependencies-builder. --- PublicAPI.md | 9 +- lib/commands/preview.ts | 4 +- lib/commands/test-init.ts | 2 +- lib/commands/test.ts | 2 +- .../json-file-settings-service.d.ts | 5 ++ .../services/json-file-settings-service.ts | 22 +++++ .../services/json-file-settings-service.ts | 31 +++++++ lib/contracts/doctor-service.ts | 4 +- lib/contracts/package-manager.ts | 4 +- lib/controllers/prepare-controller.ts | 9 +- lib/declarations.d.ts | 7 +- lib/definitions/project.d.ts | 2 +- lib/package-managers/base-package-manager.ts | 5 +- lib/package-managers/index.ts | 90 +++++++++---------- .../package-installation-manager.ts | 2 +- lib/services/android-plugin-build-service.ts | 13 ++- .../bundler/bundler-compiler-service.ts | 23 +++-- lib/services/doctor-service.ts | 25 +++--- lib/services/extensibility-service.ts | 2 +- lib/services/plugins-service.ts | 27 +++--- lib/services/project-data-service.ts | 17 ++-- lib/services/test-execution-service.ts | 2 +- lib/services/user-settings-service.ts | 4 + lib/services/versions-service.ts | 8 +- lib/services/vitest-execution-service.ts | 8 +- .../node-modules-dependencies-builder.ts | 22 +++-- test/contracts.ts | 2 +- test/controllers/add-platform-controller.ts | 1 + test/controllers/prepare-controller.ts | 2 +- test/extension-manifests.ts | 5 +- test/ios-project-service.ts | 1 + test/package-installation-manager.ts | 1 + test/package-manager-flags.ts | 7 +- test/plugins-service.ts | 1 + test/services/android-plugin-build-service.ts | 5 +- .../bundler/bundler-compiler-service.ts | 2 +- test/services/doctor-service.ts | 14 ++- test/services/extensibility-service.ts | 5 +- test/services/test-execution-service.ts | 5 +- test/stubs.ts | 5 +- .../node-modules-dependencies-builder.ts | 5 ++ 41 files changed, 218 insertions(+), 192 deletions(-) diff --git a/PublicAPI.md b/PublicAPI.md index 02dfc2055f..dd6d8f6151 100644 --- a/PublicAPI.md +++ b/PublicAPI.md @@ -579,16 +579,15 @@ Locates a package the way the selected package manager laid it out on disk, so c /** * @param {string} packageName The name of the package. * @param {string} fromDir The directory whose dependencies are searched, usually the project directory. - * @return {Promise} The absolute path of the package directory, or null when it is not installed. + * @return {string} The absolute path of the package directory, or null when it is not installed. */ -getInstalledPackagePath(packageName: string, fromDir: string): Promise; +getInstalledPackagePath(packageName: string, fromDir: string): string; ``` * Usage: ```JavaScript -tns.packageManager.getInstalledPackagePath("@nativescript/core", "/tmp/myProject").then(pathToPackage => { - console.log(pathToPackage ? `Installed at ${pathToPackage}` : "Not installed"); -}); +const pathToPackage = tns.packageManager.getInstalledPackagePath("@nativescript/core", "/tmp/myProject"); +console.log(pathToPackage ? `Installed at ${pathToPackage}` : "Not installed"); ``` ### view diff --git a/lib/commands/preview.ts b/lib/commands/preview.ts index 7b5b183915..f4f45ca6bf 100644 --- a/lib/commands/preview.ts +++ b/lib/commands/preview.ts @@ -37,7 +37,7 @@ export class PreviewCommand extends Command({ await this.installLatestPreviewCLI(); } - const previewCLIPath = await this.getPreviewCLIPath(); + const previewCLIPath = this.getPreviewCLIPath(); if (!previewCLIPath) { await this.failMissingPreviewCLI(); @@ -58,7 +58,7 @@ export class PreviewCommand extends Command({ ); } - private getPreviewCLIPath(): Promise { + private getPreviewCLIPath(): string { return this.$packageManager.getInstalledPackagePath( PREVIEW_CLI_PACKAGE, this.$projectData.projectDir, diff --git a/lib/commands/test-init.ts b/lib/commands/test-init.ts index 015559ff36..e5d4e2dbf2 100644 --- a/lib/commands/test-init.ts +++ b/lib/commands/test-init.ts @@ -137,7 +137,7 @@ export class TestInitCommand extends Command({ path: this.$options.path, }); - const modulePath = await this.$packageManager.getInstalledPackagePath( + const modulePath = this.$packageManager.getInstalledPackagePath( mod.name, projectDir, ); diff --git a/lib/commands/test.ts b/lib/commands/test.ts index 6b7f1743e2..0829bb97ec 100644 --- a/lib/commands/test.ts +++ b/lib/commands/test.ts @@ -106,7 +106,7 @@ async function canExecuteTestCommand( if ($vitestExecutionService.isVitestProject($projectData)) { const canStartTestRun = - await $vitestExecutionService.canStartTestRun($projectData); + $vitestExecutionService.canStartTestRun($projectData); if (!canStartTestRun) { $errors.fail({ formatStr: diff --git a/lib/common/definitions/json-file-settings-service.d.ts b/lib/common/definitions/json-file-settings-service.d.ts index 3c20e52e83..8fea9c1b63 100644 --- a/lib/common/definitions/json-file-settings-service.d.ts +++ b/lib/common/definitions/json-file-settings-service.d.ts @@ -13,6 +13,11 @@ interface IJsonFileSettingsService { settingName: string, cacheOpts?: ICacheTimeoutOpts ): Promise; + /** + * Reads a setting without taking the settings lock. Suitable for values that + * only change through explicit user commands, where a torn read is harmless. + */ + getSettingValueSync(settingName: string): T; saveSetting( key: string, value: T, diff --git a/lib/common/services/json-file-settings-service.ts b/lib/common/services/json-file-settings-service.ts index d58da1e8fa..81d0e38e43 100644 --- a/lib/common/services/json-file-settings-service.ts +++ b/lib/common/services/json-file-settings-service.ts @@ -56,6 +56,28 @@ export class JsonFileSettingsService implements IJsonFileSettingsService { ); } + public getSettingValueSync(settingName: string): T { + if (!this.jsonSettingsData && this.$fs.exists(this.jsonSettingsFilePath)) { + try { + this.jsonSettingsData = parseJson( + this.$fs.readText(this.jsonSettingsFilePath) + ); + } catch (err) { + this.$logger.trace( + `Error while trying to parse ${this.jsonSettingsFilePath}. Err is: ${err}` + ); + return null; + } + } + + if (this.jsonSettingsData && _.has(this.jsonSettingsData, settingName)) { + const data = this.jsonSettingsData[settingName]; + return data.modifiedByCacheMechanism ? data.value : data; + } + + return null; + } + public async saveSetting( key: string, value: T, diff --git a/lib/common/test/unit-tests/services/json-file-settings-service.ts b/lib/common/test/unit-tests/services/json-file-settings-service.ts index 9dac594142..e37673c6d1 100644 --- a/lib/common/test/unit-tests/services/json-file-settings-service.ts +++ b/lib/common/test/unit-tests/services/json-file-settings-service.ts @@ -65,6 +65,37 @@ describe("jsonFileSettingsService", () => { Date.now = originalDateNow; }); + describe("getSettingValueSync", () => { + it("returns the stored value without going through the lock", () => { + const testInjector = createTestInjector(); + dataInFile[jsonFileSettingsPath] = { prop1: "value1" }; + const lockService = testInjector.resolve("lockService"); + lockService.executeActionWithLock = () => { + throw new Error("lock must not be used for sync reads"); + }; + + const jsonFileSettingsService = + testInjector.resolve( + "jsonFileSettingsService", + { jsonFileSettingsPath } + ); + assert.equal(jsonFileSettingsService.getSettingValueSync("prop1"), "value1"); + assert.isNull(jsonFileSettingsService.getSettingValueSync("missing")); + }); + + it("returns null when the settings file does not exist", () => { + const testInjector = createTestInjector(); + const fs = testInjector.resolve("fs"); + fs.exists = () => false; + const jsonFileSettingsService = + testInjector.resolve( + "jsonFileSettingsService", + { jsonFileSettingsPath } + ); + assert.isNull(jsonFileSettingsService.getSettingValueSync("prop1")); + }); + }); + describe("getSettingValue", () => { it("returns correct data without cache", async () => { dataInFile = { [jsonFileSettingsPath]: { prop1: 1 } }; diff --git a/lib/contracts/doctor-service.ts b/lib/contracts/doctor-service.ts index 4c5a42840e..9a64fbd7a4 100644 --- a/lib/contracts/doctor-service.ts +++ b/lib/contracts/doctor-service.ts @@ -34,7 +34,5 @@ export abstract class DoctorService { }): Promise; /** Checks and notifies users of deprecated short imports in their app. */ - abstract checkForDeprecatedShortImportsInAppDir( - projectDir: string, - ): Promise; + abstract checkForDeprecatedShortImportsInAppDir(projectDir: string): void; } diff --git a/lib/contracts/package-manager.ts b/lib/contracts/package-manager.ts index 2619e3297d..010fffbd46 100644 --- a/lib/contracts/package-manager.ts +++ b/lib/contracts/package-manager.ts @@ -103,12 +103,12 @@ export abstract class PackageManager { * Locates a package the way the package manager laid it out on disk. * @param {string} packageName The name of the package. * @param {string} fromDir The directory whose dependencies are searched, usually the project directory. - * @return {Promise} The absolute path of the package directory, or null when it is not installed. + * @return {string} The absolute path of the package directory, or null when it is not installed. */ abstract getInstalledPackagePath( packageName: string, fromDir: string, - ): Promise; + ): string; /** * Gets the name of the package manager used for the current process. diff --git a/lib/controllers/prepare-controller.ts b/lib/controllers/prepare-controller.ts index f2e21afb1f..fc730ddeab 100644 --- a/lib/controllers/prepare-controller.ts +++ b/lib/controllers/prepare-controller.ts @@ -494,11 +494,10 @@ export class PrepareController SCOPED_ANDROID_RUNTIME_NAME; } // try reading from installed runtime first before reading from the npm registry... - const installedRuntimePath = - await this.$packageManager.getInstalledPackagePath( - runtimePackageName, - projectData.projectDir, - ); + const installedRuntimePath = this.$packageManager.getInstalledPackagePath( + runtimePackageName, + projectData.projectDir, + ); if (installedRuntimePath) { installedRuntimePackageJSON = this.$fs.readJson( diff --git a/lib/declarations.d.ts b/lib/declarations.d.ts index c7c9c54126..a686771506 100644 --- a/lib/declarations.d.ts +++ b/lib/declarations.d.ts @@ -116,12 +116,9 @@ interface INodePackageManager { * Locates a package the way the package manager laid it out on disk. * @param {string} packageName The name of the package. * @param {string} fromDir The directory whose dependencies are searched, usually the project directory. - * @return {Promise} The absolute path of the package directory, or null when it is not installed. + * @return {string} The absolute path of the package directory, or null when it is not installed. */ - getInstalledPackagePath( - packageName: string, - fromDir: string, - ): Promise; + getInstalledPackagePath(packageName: string, fromDir: string): string; } /** @deprecated Kept so existing annotations compile; use the {@link PackageManager} contract. */ diff --git a/lib/definitions/project.d.ts b/lib/definitions/project.d.ts index 468bf6c66d..cd2a931617 100644 --- a/lib/definitions/project.d.ts +++ b/lib/definitions/project.d.ts @@ -522,7 +522,7 @@ interface ITestExecutionService { interface IVitestExecutionService { isVitestProject(projectData: IProjectData): boolean; - canStartTestRun(projectData: IProjectData): Promise; + canStartTestRun(projectData: IProjectData): boolean; startTestRun(platform: string, projectData: IProjectData): Promise; } diff --git a/lib/package-managers/base-package-manager.ts b/lib/package-managers/base-package-manager.ts index 45a23d4e18..f44a7b85c4 100644 --- a/lib/package-managers/base-package-manager.ts +++ b/lib/package-managers/base-package-manager.ts @@ -145,10 +145,7 @@ export abstract class BasePackageManager implements INodePackageManager { }; } - public async getInstalledPackagePath( - packageName: string, - fromDir: string, - ): Promise { + public getInstalledPackagePath(packageName: string, fromDir: string): string { return resolvePackagePath(packageName, { paths: [fromDir] }) || null; } diff --git a/lib/package-managers/index.ts b/lib/package-managers/index.ts index d19f5f530c..180e855841 100644 --- a/lib/package-managers/index.ts +++ b/lib/package-managers/index.ts @@ -1,4 +1,4 @@ -import { cache, exported, invokeInit } from "../common/decorators"; +import { exported } from "../common/decorators"; import { performanceLog } from "../common/decorators"; import { PackageManagers } from "../constants"; import { @@ -11,15 +11,13 @@ import { INpmsResult, INpmPackageNameParts, } from "../declarations"; -import { - IErrors, - IUserSettingsService, -} from "../common/declarations"; +import { IErrors, IUserSettingsService } from "../common/declarations"; import { injector } from "../common/yok"; import { IProjectConfigService } from "../definitions/project"; + export class PackageManager implements IPackageManager { - private packageManager: INodePackageManager; - private _packageManagerName: string; + private selected: INodePackageManager; + private selectedName: string; constructor( private $errors: IErrors, @@ -31,95 +29,79 @@ export class PackageManager implements IPackageManager { private $bun: INodePackageManager, private $logger: ILogger, private $userSettingsService: IUserSettingsService, - private $projectConfigService: IProjectConfigService + private $projectConfigService: IProjectConfigService, ) {} - @cache() - protected async init(): Promise { - this.packageManager = await this._determinePackageManager(); - } - - @invokeInit() public async getPackageManagerName(): Promise { - return this._packageManagerName; + this.packageManager; + return this.selectedName; } @exported("packageManager") @performanceLog() - @invokeInit() public install( packageName: string, pathToSave: string, - options: IPackageInstallOptions + options: IPackageInstallOptions, ): Promise { return this.packageManager.install(packageName, pathToSave, options); } + @exported("packageManager") - @invokeInit() public uninstall( packageName: string, options?: IPackageUninstallOptions, - path?: string + path?: string, ): Promise { return this.packageManager.uninstall(packageName, options, path); } + @exported("packageManager") - @invokeInit() public view(packageName: string, field?: string): Promise { return this.packageManager.view(packageName, field); } + @exported("packageManager") - @invokeInit() public search(filter: string[]): Promise { return this.packageManager.search(filter); } - @invokeInit() public searchNpms(keyword: string): Promise { return this.packageManager.searchNpms(keyword); } - @invokeInit() - public async isRegistered(packageName: string): Promise { + public isRegistered(packageName: string): Promise { return this.packageManager.isRegistered(packageName); } - @invokeInit() - public async getPackageFullName( - packageNameParts: INpmPackageNameParts + public getPackageFullName( + packageNameParts: INpmPackageNameParts, ): Promise { return this.packageManager.getPackageFullName(packageNameParts); } - @invokeInit() - public async getPackageNameParts( - fullPackageName: string + public getPackageNameParts( + fullPackageName: string, ): Promise { return this.packageManager.getPackageNameParts(fullPackageName); } - @invokeInit() public getRegistryPackageData(packageName: string): Promise { return this.packageManager.getRegistryPackageData(packageName); } - @invokeInit() public getCachePath(): Promise { return this.packageManager.getCachePath(); } @exported("packageManager") - @invokeInit() - public getInstalledPackagePath( - packageName: string, - fromDir: string - ): Promise { + public getInstalledPackagePath(packageName: string, fromDir: string): string { return this.packageManager.getInstalledPackagePath(packageName, fromDir); } public async getTagVersion( packageName: string, - tag: string + tag: string, ): Promise { let version: string = null; if (!tag) { @@ -131,7 +113,7 @@ export class PackageManager implements IPackageManager { version = result[tag]; } catch (err) { this.$logger.trace( - `Error while getting tag version from view command: ${err}` + `Error while getting tag version from view command: ${err}`, ); const registryData = await this.getRegistryPackageData(packageName); version = registryData["dist-tags"][tag]; @@ -140,13 +122,21 @@ export class PackageManager implements IPackageManager { return version; } - private async _determinePackageManager(): Promise { - let pm = null; + private get packageManager(): INodePackageManager { + if (!this.selected) { + this.selected = this.determinePackageManager(); + } + + return this.selected; + } + + private determinePackageManager(): INodePackageManager { + let pm: string = null; try { - pm = await this.$userSettingsService.getSettingValue("packageManager"); + pm = this.$userSettingsService.getSettingValueSync("packageManager"); } catch (err) { this.$errors.fail( - `Unable to read package manager config from user settings ${err}` + `Unable to read package manager config from user settings ${err}`, ); } @@ -156,7 +146,7 @@ export class PackageManager implements IPackageManager { if (configPm) { this.$logger.trace( - `Determined packageManager to use from user config is: ${configPm}` + `Determined packageManager to use from user config is: ${configPm}`, ); pm = configPm; } @@ -164,25 +154,25 @@ export class PackageManager implements IPackageManager { // ignore error, but log info this.$logger.trace( "Tried to read cli.packageManager from project config and failed. Error is: ", - err + err, ); } if (pm === PackageManagers.yarn || this.$options.yarn) { - this._packageManagerName = PackageManagers.yarn; + this.selectedName = PackageManagers.yarn; return this.$yarn; } if (pm === PackageManagers.yarn2 || this.$options.yarn2) { - this._packageManagerName = PackageManagers.yarn2; + this.selectedName = PackageManagers.yarn2; return this.$yarn2; } else if (pm === PackageManagers.pnpm || this.$options.pnpm) { - this._packageManagerName = PackageManagers.pnpm; + this.selectedName = PackageManagers.pnpm; return this.$pnpm; } else if (pm === PackageManagers.bun) { - this._packageManagerName = PackageManagers.bun; + this.selectedName = PackageManagers.bun; return this.$bun; } else { - this._packageManagerName = PackageManagers.npm; + this.selectedName = PackageManagers.npm; return this.$npm; } } diff --git a/lib/package-managers/package-installation-manager.ts b/lib/package-managers/package-installation-manager.ts index ffa2b04e8b..807c4fc724 100644 --- a/lib/package-managers/package-installation-manager.ts +++ b/lib/package-managers/package-installation-manager.ts @@ -189,7 +189,7 @@ export class PackageInstallationManager implements IPackageInstallationManager { projectDir: string ): Promise { // local installation takes precedence over cache - const inspectorPath = await this.$packageManager.getInstalledPackagePath( + const inspectorPath = this.$packageManager.getInstalledPackagePath( inspectorNpmPackageName, projectDir ); diff --git a/lib/services/android-plugin-build-service.ts b/lib/services/android-plugin-build-service.ts index 31a4cecf96..0d4021ac6e 100644 --- a/lib/services/android-plugin-build-service.ts +++ b/lib/services/android-plugin-build-service.ts @@ -506,7 +506,7 @@ export class AndroidPluginBuildService implements IAndroidPluginBuildService { return runtimeVersion; } - private async getLocalGradleVersions(): Promise { + private getLocalGradleVersions(): IRuntimeGradleVersions { // partial interface of the runtime package.json // including new 8.2+ format and legacy interface IRuntimePackageJSON { @@ -526,11 +526,10 @@ export class AndroidPluginBuildService implements IAndroidPluginBuildService { this.$projectData.nsConfig?.android?.runtimePackageName || SCOPED_ANDROID_RUNTIME_NAME; // try reading from installed runtime first before reading from the npm registry... - const installedRuntimePath = - await this.$packageManager.getInstalledPackagePath( - packageName, - this.$projectData.projectDir, - ); + const installedRuntimePath = this.$packageManager.getInstalledPackagePath( + packageName, + this.$projectData.projectDir, + ); if (!installedRuntimePath) { return null; @@ -573,7 +572,7 @@ export class AndroidPluginBuildService implements IAndroidPluginBuildService { versions: { gradle: string; gradleAndroid: string }; } = null; - const localVersionInfo = await this.getLocalGradleVersions(); + const localVersionInfo = this.getLocalGradleVersions(); if (localVersionInfo) { return localVersionInfo; diff --git a/lib/services/bundler/bundler-compiler-service.ts b/lib/services/bundler/bundler-compiler-service.ts index b613860817..2c05eb6183 100644 --- a/lib/services/bundler/bundler-compiler-service.ts +++ b/lib/services/bundler/bundler-compiler-service.ts @@ -569,9 +569,8 @@ export class BundlerCompilerService additionalNodeArgs.unshift("--max_old_space_size=4096"); } - const bundlerExecutablePath = - await this.getBundlerExecutablePath(projectData); - const isModernBundler = await this.isModernBundler(projectData); + const bundlerExecutablePath = this.getBundlerExecutablePath(projectData); + const isModernBundler = this.isModernBundler(projectData); const args = [ ...additionalNodeArgs, bundlerExecutablePath, @@ -725,7 +724,7 @@ export class BundlerCompilerService // go after `--` so vite's CLI doesn't choke on unknown options. const args = [ ...additionalNodeArgs, - await this.getBundlerExecutablePath(projectData), + this.getBundlerExecutablePath(projectData), "serve", `--config=${projectData.bundlerConfigPath}`, `--mode=development`, @@ -1165,9 +1164,7 @@ export class BundlerCompilerService }); } - private async getBundlerExecutablePath( - projectData: IProjectData, - ): Promise { + private getBundlerExecutablePath(projectData: IProjectData): string { const bundler = this.getBundler(); const resolve = (packageName: string) => this.$packageManager.getInstalledPackagePath( @@ -1176,13 +1173,13 @@ export class BundlerCompilerService ); if (bundler === "vite") { - const packagePath = await resolve("vite"); + const packagePath = resolve("vite"); if (packagePath) { return path.resolve(packagePath, "bin", "vite.js"); } - } else if (await this.isModernBundler(projectData)) { - const packagePath = await resolve(this.getBundlerPackageName()); + } else if (this.isModernBundler(projectData)) { + const packagePath = resolve(this.getBundlerPackageName()); if (packagePath) { return path.resolve(packagePath, "dist", "bin", "index.js"); @@ -1202,7 +1199,7 @@ export class BundlerCompilerService ); } - const packagePath = await resolve("webpack"); + const packagePath = resolve("webpack"); if (!packagePath) { return ""; @@ -1224,13 +1221,13 @@ export class BundlerCompilerService ); } - private async isModernBundler(projectData: IProjectData): Promise { + private isModernBundler(projectData: IProjectData): boolean { const bundler = this.getBundler(); switch (bundler) { case "rspack": return true; default: - const packagePath = await this.$packageManager.getInstalledPackagePath( + const packagePath = this.$packageManager.getInstalledPackagePath( this.getBundlerPackageName(), projectData.projectDir, ); diff --git a/lib/services/doctor-service.ts b/lib/services/doctor-service.ts index 3a2b355713..3c41698968 100644 --- a/lib/services/doctor-service.ts +++ b/lib/services/doctor-service.ts @@ -139,7 +139,7 @@ export class DoctorServiceImpl implements DoctorService { } // todo: check for deprecated imports from `tns-core-modules` - await this.checkForDeprecatedShortImportsInAppDir(configOptions.projectDir); + this.checkForDeprecatedShortImportsInAppDir(configOptions.projectDir); await this.$injector .resolve( @@ -241,14 +241,12 @@ export class DoctorServiceImpl implements DoctorService { return !hasWarnings; } - public async checkForDeprecatedShortImportsInAppDir( - projectDir: string, - ): Promise { + public checkForDeprecatedShortImportsInAppDir(projectDir: string): void { if (projectDir) { try { const files = this.$projectDataService.getAppExecutableFiles(projectDir); - const shortImports = await this.getDeprecatedShortImportsInFiles( + const shortImports = this.getDeprecatedShortImportsInFiles( files, projectDir, ); @@ -271,11 +269,11 @@ export class DoctorServiceImpl implements DoctorService { } } - protected async getDeprecatedShortImportsInFiles( + protected getDeprecatedShortImportsInFiles( files: string[], projectDir: string, - ): Promise<{ file: string; line: string }[]> { - const shortImportRegExp = await this.getShortImportRegExp(projectDir); + ): { file: string; line: string }[] { + const shortImportRegExp = this.getShortImportRegExp(projectDir); const shortImports: { file: string; line: string }[] = []; if (!shortImportRegExp) { return shortImports; @@ -306,12 +304,11 @@ export class DoctorServiceImpl implements DoctorService { return shortImports; } - private async getShortImportRegExp(projectDir: string): Promise { - const pathToTnsCoreModules = - await this.$packageManager.getInstalledPackagePath( - TNS_CORE_MODULES_NAME, - projectDir, - ); + private getShortImportRegExp(projectDir: string): RegExp { + const pathToTnsCoreModules = this.$packageManager.getInstalledPackagePath( + TNS_CORE_MODULES_NAME, + projectDir, + ); if (!pathToTnsCoreModules) { return null; } diff --git a/lib/services/extensibility-service.ts b/lib/services/extensibility-service.ts index aee3ef6cbf..54de9ba694 100644 --- a/lib/services/extensibility-service.ts +++ b/lib/services/extensibility-service.ts @@ -512,7 +512,7 @@ export class ExtensibilityService implements IExtensibilityService { extensionName: string, ): Promise { this.$logger.trace(`Asserting extension ${extensionName} is installed.`); - const installedPath = await this.$packageManager.getInstalledPackagePath( + const installedPath = this.$packageManager.getInstalledPackagePath( extensionName, this.pathToExtensions, ); diff --git a/lib/services/plugins-service.ts b/lib/services/plugins-service.ts index dffe4497b5..34880da5b2 100644 --- a/lib/services/plugins-service.ts +++ b/lib/services/plugins-service.ts @@ -96,7 +96,7 @@ export class PluginsService implements IPluginsService { this.npmInstallOptions, ) ).name; - const pathToRealNpmPackageJson = await this.getPackageJsonFilePathForModule( + const pathToRealNpmPackageJson = this.getPackageJsonFilePathForModule( name, projectData.projectDir, ); @@ -144,7 +144,7 @@ export class PluginsService implements IPluginsService { platformData: IPlatformData, ): Promise => { const pluginData = this.convertToPluginData( - await this.getNodeModuleData(pluginName, projectData.projectDir), + this.getNodeModuleData(pluginName, projectData.projectDir), projectData.projectDir, ); @@ -297,7 +297,7 @@ export class PluginsService implements IPluginsService { const notInstalledDependencies: string[] = []; for (const dep of allDependencies) { this.$logger.trace(`Checking if ${dep} is installed...`); - const pathToPackage = await this.$packageManager.getInstalledPackagePath( + const pathToPackage = this.$packageManager.getInstalledPackagePath( dep, projectData.projectDir, ); @@ -694,11 +694,11 @@ This framework comes from ${dependencyName} plugin, which is installed multiple return path.join(projectDir, "package.json"); } - private async getPackageJsonFilePathForModule( + private getPackageJsonFilePathForModule( moduleName: string, projectDir: string, - ): Promise { - const pathToModule = await this.$packageManager.getInstalledPackagePath( + ): string { + const pathToModule = this.$packageManager.getInstalledPackagePath( moduleName, projectDir, ); @@ -710,13 +710,13 @@ This framework comes from ${dependencyName} plugin, which is installed multiple return _.keys(require(packageJsonFilePath).dependencies); } - private async getNodeModuleData( + private getNodeModuleData( module: string, projectDir: string, - ): Promise { + ): INodeModuleData { // module can be modulePath or moduleName if (!this.$fs.exists(module) || path.basename(module) !== "package.json") { - const resolvedPath = await this.getPackageJsonFilePathForModule( + const resolvedPath = this.getPackageJsonFilePathForModule( module, projectDir, ); @@ -745,12 +745,11 @@ This framework comes from ${dependencyName} plugin, which is installed multiple await this.ensureAllDependenciesAreInstalled(projectData); const nodeModules = this.getDependencies(projectData.projectDir); - const modules = await Promise.all( - nodeModules.map((nodeModuleName) => + return nodeModules + .map((nodeModuleName) => this.getNodeModuleData(nodeModuleName, projectData.projectDir), - ), - ); - return modules.filter(Boolean); + ) + .filter(Boolean); } private async executeNpmCommand( diff --git a/lib/services/project-data-service.ts b/lib/services/project-data-service.ts index 31e5a2a342..d795c9cf38 100644 --- a/lib/services/project-data-service.ts +++ b/lib/services/project-data-service.ts @@ -27,6 +27,7 @@ import { import { IAndroidResourcesMigrationService, IStaticConfig, + IPackageManager, } from "../declarations"; import { IBasePluginData, IPluginsService } from "../definitions/plugins"; import { IDictionary, IFileSystem, IProjectDir } from "../common/declarations"; @@ -34,7 +35,6 @@ import * as _ from "lodash"; import { IInjector } from "../common/definitions/yok"; import { injector } from "../common/yok"; import * as semver from "semver"; -import { resolvePackageJSONPath } from "../helpers/package-path-helper"; interface IProjectFileData { projectData: any; @@ -653,20 +653,17 @@ export class ProjectDataService implements IProjectDataService { // in case we are using a local tgz for the runtime or a range like ~8.0.0, ^8.0.0 etc. or a tag like JSC if (runtimePackage.version.includes("tgz") || isRange || isTag) { try { - const runtimePackageJsonPath = resolvePackageJSONPath( - runtimePackage.name, - { - paths: [projectDir], - }, - ); + const runtimePackagePath = this.$injector + .resolve("packageManager") + .getInstalledPackagePath(runtimePackage.name, projectDir); - if (!runtimePackageJsonPath) { + if (!runtimePackagePath) { // caught below - throw new Error("Runtime package.json not found."); + throw new Error("Runtime package not found."); } runtimePackage.version = this.$fs.readJson( - runtimePackageJsonPath, + path.join(runtimePackagePath, constants.PACKAGE_JSON_FILE_NAME), ).version; } catch (err) { if (isRange) { diff --git a/lib/services/test-execution-service.ts b/lib/services/test-execution-service.ts index e640dcf1fe..2733aea4c5 100644 --- a/lib/services/test-execution-service.ts +++ b/lib/services/test-execution-service.ts @@ -144,7 +144,7 @@ export class TestExecutionService implements ITestExecutionService { } }); - const pathToKarma = await this.$packageManager.getInstalledPackagePath( + const pathToKarma = this.$packageManager.getInstalledPackagePath( "karma", projectData.projectDir, ); diff --git a/lib/services/user-settings-service.ts b/lib/services/user-settings-service.ts index 2ba6be9831..64ed5c3f9f 100644 --- a/lib/services/user-settings-service.ts +++ b/lib/services/user-settings-service.ts @@ -39,6 +39,10 @@ export class UserSettingsService implements IUserSettingsService { ); } + public getSettingValueSync(settingName: string): T { + return this.$jsonFileSettingsService.getSettingValueSync(settingName); + } + public saveSetting( key: string, value: T, diff --git a/lib/services/versions-service.ts b/lib/services/versions-service.ts index dc14694ca1..19c9f2621d 100644 --- a/lib/services/versions-service.ts +++ b/lib/services/versions-service.ts @@ -73,8 +73,8 @@ class VersionsService implements IVersionsService { packageName, this.projectData.projectDir ); - let scopedPackagePath = await resolve(constants.SCOPED_TNS_CORE_MODULES); - let tnsCoreModulesPath = await resolve(constants.TNS_CORE_MODULES_NAME); + let scopedPackagePath = resolve(constants.SCOPED_TNS_CORE_MODULES); + let tnsCoreModulesPath = resolve(constants.TNS_CORE_MODULES_NAME); const dependsOnNonScopedPackage = !!this.projectData.dependencies[ constants.TNS_CORE_MODULES_NAME @@ -91,8 +91,8 @@ class VersionsService implements IVersionsService { await this.$pluginsService.ensureAllDependenciesAreInstalled( this.projectData ); - scopedPackagePath = await resolve(constants.SCOPED_TNS_CORE_MODULES); - tnsCoreModulesPath = await resolve(constants.TNS_CORE_MODULES_NAME); + scopedPackagePath = resolve(constants.SCOPED_TNS_CORE_MODULES); + tnsCoreModulesPath = resolve(constants.TNS_CORE_MODULES_NAME); } if (dependsOnNonScopedPackage && tnsCoreModulesPath) { diff --git a/lib/services/vitest-execution-service.ts b/lib/services/vitest-execution-service.ts index 41b5593791..80c09e8235 100644 --- a/lib/services/vitest-execution-service.ts +++ b/lib/services/vitest-execution-service.ts @@ -25,10 +25,10 @@ export class VitestExecutionService implements IVitestExecutionService { return !!this.getConfigPath(projectData); } - public async canStartTestRun(projectData: IProjectData): Promise { + public canStartTestRun(projectData: IProjectData): boolean { return ( this.isVitestProject(projectData) && - !!(await this.getVitestPackagePath(projectData)) + !!this.getVitestPackagePath(projectData) ); } @@ -36,7 +36,7 @@ export class VitestExecutionService implements IVitestExecutionService { platform: string, projectData: IProjectData, ): Promise { - const vitestPackagePath = await this.getVitestPackagePath(projectData); + const vitestPackagePath = this.getVitestPackagePath(projectData); if (!vitestPackagePath) { this.$errors.fail( "Unable to find 'vitest' in the project. Run '$ ns test init --framework vitest' first.", @@ -89,7 +89,7 @@ export class VitestExecutionService implements IVitestExecutionService { return null; } - private getVitestPackagePath(projectData: IProjectData): Promise { + private getVitestPackagePath(projectData: IProjectData): string { return this.$packageManager.getInstalledPackagePath( "vitest", projectData.projectDir, diff --git a/lib/tools/node-modules/node-modules-dependencies-builder.ts b/lib/tools/node-modules/node-modules-dependencies-builder.ts index 9474b7eb79..abed73c376 100644 --- a/lib/tools/node-modules/node-modules-dependencies-builder.ts +++ b/lib/tools/node-modules/node-modules-dependencies-builder.ts @@ -1,11 +1,10 @@ import * as path from "path"; import { PACKAGE_JSON_FILE_NAME } from "../../constants"; import { INodeModulesDependenciesBuilder } from "../../definitions/platform"; -import { IDependencyData } from "../../declarations"; +import { IDependencyData, IPackageManager } from "../../declarations"; import { IFileSystem } from "../../common/declarations"; import * as _ from "lodash"; import { injector } from "../../common/yok"; -import { resolvePackagePath } from "@rigor789/resolve-package-path"; interface IDependencyDescription { parent: IDependencyDescription; @@ -17,7 +16,10 @@ interface IDependencyDescription { export class NodeModulesDependenciesBuilder implements INodeModulesDependenciesBuilder { - public constructor(private $fs: IFileSystem) {} + public constructor( + private $fs: IFileSystem, + private $packageManager: IPackageManager, + ) {} public getProductionDependencies( projectPath: string, @@ -96,16 +98,18 @@ export class NodeModulesDependenciesBuilder const parentModulesPath = depDescription?.parentDir ?? depDescription?.parent?.parentDir; - let modulePath: string = resolvePackagePath(depDescription.name, { - paths: [parentModulesPath], - }); + let modulePath = this.$packageManager.getInstalledPackagePath( + depDescription.name, + parentModulesPath, + ); // perhaps traverse up the tree here? if (!modulePath) { // fallback to searching in the root path - modulePath = resolvePackagePath(depDescription.name, { - paths: [rootPath], - }); + modulePath = this.$packageManager.getInstalledPackagePath( + depDescription.name, + rootPath, + ); } // if we failed to find the module... diff --git a/test/contracts.ts b/test/contracts.ts index 1be08d6c2a..e07581e01e 100644 --- a/test/contracts.ts +++ b/test/contracts.ts @@ -60,7 +60,7 @@ describe("contracts tranche", () => { async canExecuteLocalBuild(): Promise { return true; } - async checkForDeprecatedShortImportsInAppDir(): Promise {} + checkForDeprecatedShortImportsInAppDir(): void {} } const injector = new Injector([provide(DoctorService, StubDoctorService)]); diff --git a/test/controllers/add-platform-controller.ts b/test/controllers/add-platform-controller.ts index be90db2cbf..934c84d26e 100644 --- a/test/controllers/add-platform-controller.ts +++ b/test/controllers/add-platform-controller.ts @@ -37,6 +37,7 @@ function createInjector(data?: { latestFrameworkVersion: string }) { injector.register("userSettingsService", { getSettingValue: async (settingName: string): Promise => undefined, + getSettingValueSync: (settingName: string): void => undefined, }); injector.register("tempService", TempServiceStub); injector.register("mobileHelper", MobileHelper); diff --git a/test/controllers/prepare-controller.ts b/test/controllers/prepare-controller.ts index 050e8bf5b0..553bcbe7bc 100644 --- a/test/controllers/prepare-controller.ts +++ b/test/controllers/prepare-controller.ts @@ -52,7 +52,7 @@ function createTestInjector(data: { hasNativeChanges: boolean }): IInjector { injector.register("mobileHelper", MobileHelper); injector.register("prepareController", PrepareController); injector.register("packageManager", { - getInstalledPackagePath: async (): Promise => null, + getInstalledPackagePath: (): string => null, }); injector.register("nodeModulesDependenciesBuilder", { diff --git a/test/extension-manifests.ts b/test/extension-manifests.ts index faaba51cfa..33bbe53d80 100644 --- a/test/extension-manifests.ts +++ b/test/extension-manifests.ts @@ -185,10 +185,7 @@ describe("extension manifests", () => { install: async (): Promise => { throw new Error("Extensions are expected to be installed already."); }, - getInstalledPackagePath: async ( - packageName: string, - fromDir: string, - ): Promise => + getInstalledPackagePath: (packageName: string, fromDir: string): string => resolvePackagePath(packageName, { paths: [fromDir] }) || null, uninstall: async (): Promise => undefined, searchNpms: async (): Promise => ({ results: [] }), diff --git a/test/ios-project-service.ts b/test/ios-project-service.ts index 09b124c0fe..9d610a3d3f 100644 --- a/test/ios-project-service.ts +++ b/test/ios-project-service.ts @@ -180,6 +180,7 @@ function createTestInjector( ); testInjector.register("userSettingsService", { getSettingValue: async (settingName: string): Promise => undefined, + getSettingValueSync: (settingName: string): void => undefined, }); testInjector.register("packageManager", PackageManager); testInjector.register("projectConfigService", ProjectConfigServiceStub); diff --git a/test/package-installation-manager.ts b/test/package-installation-manager.ts index 7a1622fb90..df8773c66e 100644 --- a/test/package-installation-manager.ts +++ b/test/package-installation-manager.ts @@ -45,6 +45,7 @@ function createTestInjector(): IInjector { }); testInjector.register("userSettingsService", { getSettingValue: async (settingName: string): Promise => undefined, + getSettingValueSync: (settingName: string): void => undefined, }); testInjector.register("npm", NpmLib.NpmPackageManager); testInjector.register("yarn", YarnLib.YarnPackageManager); diff --git a/test/package-manager-flags.ts b/test/package-manager-flags.ts index 27e9a9ac11..23da5b55c2 100644 --- a/test/package-manager-flags.ts +++ b/test/package-manager-flags.ts @@ -232,10 +232,7 @@ describe("package manager flag mapping", () => { name, ctor, ).resolve(name); - const resolved = await manager.getInstalledPackagePath( - "lodash", - repoRoot, - ); + const resolved = manager.getInstalledPackagePath("lodash", repoRoot); assert.equal(resolved, path.join(repoRoot, "node_modules", "lodash")); }); @@ -245,7 +242,7 @@ describe("package manager flag mapping", () => { ctor, ).resolve(name); assert.isNull( - await manager.getInstalledPackagePath( + manager.getInstalledPackagePath( "definitely-not-installed-package", repoRoot, ), diff --git a/test/plugins-service.ts b/test/plugins-service.ts index 4c7eb6c57e..94b33dce79 100644 --- a/test/plugins-service.ts +++ b/test/plugins-service.ts @@ -69,6 +69,7 @@ function createTestInjector() { testInjector.register("messagesService", MessagesService); testInjector.register("userSettingsService", { getSettingValue: async (settingName: string): Promise => undefined, + getSettingValueSync: (settingName: string): void => undefined, }); testInjector.register("packageManager", PackageManager); testInjector.register( diff --git a/test/services/android-plugin-build-service.ts b/test/services/android-plugin-build-service.ts index 0f8332d5c9..8e5a8c3748 100644 --- a/test/services/android-plugin-build-service.ts +++ b/test/services/android-plugin-build-service.ts @@ -129,10 +129,7 @@ describe("androidPluginBuildService", () => { addProjectRuntime?: boolean; }): any { return { - getInstalledPackagePath: async ( - packageName: string, - fromDir: string, - ): Promise => + getInstalledPackagePath: (packageName: string, fromDir: string): string => resolvePackagePath(packageName, { paths: [fromDir] }) || null, getRegistryPackageData: async (packageName: string): Promise => { const result: any = []; diff --git a/test/services/bundler/bundler-compiler-service.ts b/test/services/bundler/bundler-compiler-service.ts index 36fe0bf39b..94b1c84986 100644 --- a/test/services/bundler/bundler-compiler-service.ts +++ b/test/services/bundler/bundler-compiler-service.ts @@ -58,7 +58,7 @@ function createTestInjector( const testInjector = new Yok(); testInjector.register("packageManager", { getPackageManagerName: async () => packageManager, - getInstalledPackagePath: async (): Promise => null, + getInstalledPackagePath: (): string => null, }); testInjector.register("bundlerCompilerService", BundlerCompilerService); testInjector.register("childProcess", {}); diff --git a/test/services/doctor-service.ts b/test/services/doctor-service.ts index efe7654871..2adfc0eb2f 100644 --- a/test/services/doctor-service.ts +++ b/test/services/doctor-service.ts @@ -65,7 +65,7 @@ class DoctorServiceInheritor extends DoctorService { public getDeprecatedShortImportsInFiles( files: string[], projectDir: string, - ): Promise<{ file: string; line: string }[]> { + ): { file: string; line: string }[] { return super.getDeprecatedShortImportsInFiles(files, projectDir); } } @@ -96,10 +96,7 @@ describe("doctorService", () => { }); testInjector.register("versionsService", {}); testInjector.register("packageManager", { - getInstalledPackagePath: async ( - packageName: string, - fromDir: string, - ): Promise => + getInstalledPackagePath: (packageName: string, fromDir: string): string => packageName === "tns-core-modules" ? path.join(fromDir, "node_modules", packageName) : null, @@ -380,7 +377,7 @@ const Observable = require("tns-core-modules-widgets/data/observable").Observabl fs.readText = (filePath) => filesContents[filePath]; const shortImports = - await doctorService.getDeprecatedShortImportsInFiles( + doctorService.getDeprecatedShortImportsInFiles( _.keys(filesContents), "projectDir", ); @@ -391,14 +388,13 @@ const Observable = require("tns-core-modules-widgets/data/observable").Observabl it("getDeprecatedShortImportsInFiles returns no results when tns-core-modules is not installed", async () => { const testInjector = createTestInjector(); const packageManager = testInjector.resolve("packageManager"); - packageManager.getInstalledPackagePath = async (): Promise => - null; + packageManager.getInstalledPackagePath = (): string => null; const doctorService = testInjector.resolve("doctorService"); const fs = testInjector.resolve("fs"); fs.readText = () => 'const application = require("application");'; - const shortImports = await doctorService.getDeprecatedShortImportsInFiles( + const shortImports = doctorService.getDeprecatedShortImportsInFiles( ["file1"], "projectDir", ); diff --git a/test/services/extensibility-service.ts b/test/services/extensibility-service.ts index eae286db8d..132acff774 100644 --- a/test/services/extensibility-service.ts +++ b/test/services/extensibility-service.ts @@ -74,6 +74,7 @@ describe("extensibilityService", () => { }); testInjector.register("userSettingsService", { getSettingValue: async (settingName: string): Promise => undefined, + getSettingValueSync: (settingName: string): void => undefined, }); testInjector.register("npm", NpmPackageManager); testInjector.register("yarn", YarnPackageManager); @@ -92,10 +93,10 @@ describe("extensibilityService", () => { resolve: (extensionName: string, fromDir: string) => string, ): void => { const packageManager = testInjector.resolve("packageManager"); - packageManager.getInstalledPackagePath = async ( + packageManager.getInstalledPackagePath = ( packageName: string, fromDir: string, - ): Promise => resolve(packageName, fromDir); + ): string => resolve(packageName, fromDir); }; const getExpectedInstallationPathForExtension = ( diff --git a/test/services/test-execution-service.ts b/test/services/test-execution-service.ts index 7daa0d3f7c..bd85b8b3f6 100644 --- a/test/services/test-execution-service.ts +++ b/test/services/test-execution-service.ts @@ -15,10 +15,7 @@ function getTestExecutionService( injector.register("testExecutionService", TestExecutionService); injector.register("runController", {}); injector.register("packageManager", { - getInstalledPackagePath: async ( - packageName: string, - fromDir: string, - ): Promise => + getInstalledPackagePath: (packageName: string, fromDir: string): string => installedPackages.indexOf(packageName) !== -1 ? `${fromDir}/node_modules/${packageName}` : null, diff --git a/test/stubs.ts b/test/stubs.ts index 533714ee1f..6be3af35fc 100644 --- a/test/stubs.ts +++ b/test/stubs.ts @@ -457,10 +457,7 @@ export class PackageInstallationManagerStub implements IPackageInstallationManag export class NodePackageManagerStub implements INodePackageManager { constructor() {} - public async getInstalledPackagePath( - packageName: string, - fromDir: string, - ): Promise { + public getInstalledPackagePath(packageName: string, fromDir: string): string { return null; } diff --git a/test/tools/node-modules/node-modules-dependencies-builder.ts b/test/tools/node-modules/node-modules-dependencies-builder.ts index 8ea64163ef..8f6898ddc7 100644 --- a/test/tools/node-modules/node-modules-dependencies-builder.ts +++ b/test/tools/node-modules/node-modules-dependencies-builder.ts @@ -14,6 +14,7 @@ import { import * as os from "os"; import * as fs from "fs"; import { FileSystem } from "../../../lib/common/file-system"; +import { resolvePackagePath } from "../../../lib/helpers/package-path-helper"; interface IDependencyInfo { name: string; @@ -39,6 +40,10 @@ describe("nodeModulesDependenciesBuilder", () => { const getTestInjector = (): IInjector => { const testInjector = new Yok(); testInjector.register("fs", FileSystem); + testInjector.register("packageManager", { + getInstalledPackagePath: (packageName: string, fromDir: string): string => + resolvePackagePath(packageName, { paths: [fromDir] }) || null, + }); return testInjector; };