Skip to content

Commit 14a41fa

Browse files
rigor789rigor789
committed
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.
1 parent c8d2a28 commit 14a41fa

10 files changed

Lines changed: 111 additions & 121 deletions

‎lib/commands/test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ async function canExecuteTestCommand(
106106

107107
if ($vitestExecutionService.isVitestProject($projectData)) {
108108
const canStartTestRun =
109-
$vitestExecutionService.canStartTestRun($projectData);
109+
await $vitestExecutionService.canStartTestRun($projectData);
110110
if (!canStartTestRun) {
111111
$errors.fail({
112112
formatStr:

‎lib/definitions/project.d.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -522,7 +522,7 @@ interface ITestExecutionService {
522522

523523
interface IVitestExecutionService {
524524
isVitestProject(projectData: IProjectData): boolean;
525-
canStartTestRun(projectData: IProjectData): boolean;
525+
canStartTestRun(projectData: IProjectData): Promise<boolean>;
526526
startTestRun(platform: string, projectData: IProjectData): Promise<void>;
527527
}
528528

‎lib/services/bundler/bundler-compiler-service.ts‎

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,6 @@ import {
3535
import { ICleanupService } from "../../definitions/cleanup-service";
3636
import { ViteHmrPortService } from "../../contracts/vite-hmr-port-service";
3737
import { injector } from "../../common/yok";
38-
import {
39-
resolvePackagePath,
40-
resolvePackageJSONPath,
41-
} from "../../helpers/package-path-helper";
4238

4339
// todo: move out of here
4440
interface IBundlerMessage<T = any> {
@@ -573,10 +569,13 @@ export class BundlerCompilerService
573569
additionalNodeArgs.unshift("--max_old_space_size=4096");
574570
}
575571

572+
const bundlerExecutablePath =
573+
await this.getBundlerExecutablePath(projectData);
574+
const isModernBundler = await this.isModernBundler(projectData);
576575
const args = [
577576
...additionalNodeArgs,
578-
this.getBundlerExecutablePath(projectData),
579-
isVite || this.isModernBundler(projectData) ? "build" : null,
577+
bundlerExecutablePath,
578+
isVite || isModernBundler ? "build" : null,
580579
`--config=${projectData.bundlerConfigPath}`,
581580
...envParams,
582581
].filter(Boolean);
@@ -726,7 +725,7 @@ export class BundlerCompilerService
726725
// go after `--` so vite's CLI doesn't choke on unknown options.
727726
const args = [
728727
...additionalNodeArgs,
729-
this.getBundlerExecutablePath(projectData),
728+
await this.getBundlerExecutablePath(projectData),
730729
"serve",
731730
`--config=${projectData.bundlerConfigPath}`,
732731
`--mode=development`,
@@ -1166,21 +1165,24 @@ export class BundlerCompilerService
11661165
});
11671166
}
11681167

1169-
private getBundlerExecutablePath(projectData: IProjectData): string {
1168+
private async getBundlerExecutablePath(
1169+
projectData: IProjectData,
1170+
): Promise<string> {
11701171
const bundler = this.getBundler();
1172+
const resolve = (packageName: string) =>
1173+
this.$packageManager.getInstalledPackagePath(
1174+
packageName,
1175+
projectData.projectDir,
1176+
);
11711177

11721178
if (bundler === "vite") {
1173-
const packagePath = resolvePackagePath(`vite`, {
1174-
paths: [projectData.projectDir],
1175-
});
1179+
const packagePath = await resolve("vite");
11761180

11771181
if (packagePath) {
11781182
return path.resolve(packagePath, "bin", "vite.js");
11791183
}
1180-
} else if (this.isModernBundler(projectData)) {
1181-
const packagePath = resolvePackagePath(this.getBundlerPackageName(), {
1182-
paths: [projectData.projectDir],
1183-
});
1184+
} else if (await this.isModernBundler(projectData)) {
1185+
const packagePath = await resolve(this.getBundlerPackageName());
11841186

11851187
if (packagePath) {
11861188
return path.resolve(packagePath, "dist", "bin", "index.js");
@@ -1200,9 +1202,7 @@ export class BundlerCompilerService
12001202
);
12011203
}
12021204

1203-
const packagePath = resolvePackagePath("webpack", {
1204-
paths: [projectData.projectDir],
1205-
});
1205+
const packagePath = await resolve("webpack");
12061206

12071207
if (!packagePath) {
12081208
return "";
@@ -1224,21 +1224,21 @@ export class BundlerCompilerService
12241224
);
12251225
}
12261226

1227-
private isModernBundler(projectData: IProjectData): boolean {
1227+
private async isModernBundler(projectData: IProjectData): Promise<boolean> {
12281228
const bundler = this.getBundler();
12291229
switch (bundler) {
12301230
case "rspack":
12311231
return true;
12321232
default:
1233-
const packageJSONPath = resolvePackageJSONPath(
1233+
const packagePath = await this.$packageManager.getInstalledPackagePath(
12341234
this.getBundlerPackageName(),
1235-
{
1236-
paths: [projectData.projectDir],
1237-
},
1235+
projectData.projectDir,
12381236
);
12391237

1240-
if (packageJSONPath) {
1241-
const packageData = this.$fs.readJson(packageJSONPath);
1238+
if (packagePath) {
1239+
const packageData = this.$fs.readJson(
1240+
path.join(packagePath, "package.json"),
1241+
);
12421242
const ver = semver.coerce(packageData.version);
12431243

12441244
if (semver.satisfies(ver, ">= 5.0.0")) {

‎lib/services/extensibility-service.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -512,11 +512,12 @@ export class ExtensibilityService implements IExtensibilityService {
512512
extensionName: string,
513513
): Promise<void> {
514514
this.$logger.trace(`Asserting extension ${extensionName} is installed.`);
515-
const installedExtensions = this.$fs.readDirectory(
516-
path.join(this.pathToExtensions, constants.NODE_MODULES_FOLDER_NAME),
515+
const installedPath = await this.$packageManager.getInstalledPackagePath(
516+
extensionName,
517+
this.pathToExtensions,
517518
);
518519

519-
if (installedExtensions.indexOf(extensionName) === -1) {
520+
if (!installedPath) {
520521
this.$logger.trace(
521522
`Extension ${extensionName} is not installed, starting installation.`,
522523
);

‎lib/services/test-execution-service.ts‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,13 @@ import {
77
IProjectDataService,
88
IProjectData,
99
} from "../definitions/project";
10-
import { IConfiguration, IOptions } from "../declarations";
10+
import { IConfiguration, IOptions, IPackageManager } from "../declarations";
1111
import { IPluginsService } from "../definitions/plugins";
1212
import { Server, IFileSystem, IChildProcess } from "../common/declarations";
1313
import { ErrorCodes } from "../common/enums";
1414
import * as _ from "lodash";
1515
import { injector } from "../common/yok";
1616
import { ICommandParameter } from "../common/definitions/commands";
17-
import { resolvePackagePath } from "../helpers/package-path-helper";
1817

1918
interface IKarmaConfigOptions {
2019
debugBrk: boolean;
@@ -36,6 +35,7 @@ export class TestExecutionService implements ITestExecutionService {
3635
private $pluginsService: IPluginsService,
3736
private $projectDataService: IProjectDataService,
3837
private $childProcess: IChildProcess,
38+
private $packageManager: IPackageManager,
3939
) {}
4040

4141
public platform: string;
@@ -144,9 +144,10 @@ export class TestExecutionService implements ITestExecutionService {
144144
}
145145
});
146146

147-
const pathToKarma = resolvePackagePath("karma", {
148-
paths: [projectData.projectDir],
149-
});
147+
const pathToKarma = await this.$packageManager.getInstalledPackagePath(
148+
"karma",
149+
projectData.projectDir,
150+
);
150151

151152
canStartKarmaServer = canStartKarmaServer && !!pathToKarma;
152153

‎lib/services/vitest-execution-service.ts‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
11
import * as path from "path";
22
import { IProjectData, IVitestExecutionService } from "../definitions/project";
3-
import { IOptions } from "../declarations";
3+
import { IOptions, IPackageManager } from "../declarations";
44
import { IChildProcess, IErrors, IFileSystem } from "../common/declarations";
55
import { injector } from "../common/yok";
6-
import { resolvePackagePath } from "../helpers/package-path-helper";
76

87
const VITEST_CONFIG_FILES = [
98
"vitest.config.mts",
@@ -19,26 +18,25 @@ export class VitestExecutionService implements IVitestExecutionService {
1918
private $fs: IFileSystem,
2019
private $logger: ILogger,
2120
private $options: IOptions,
21+
private $packageManager: IPackageManager,
2222
) {}
2323

2424
public isVitestProject(projectData: IProjectData): boolean {
2525
return !!this.getConfigPath(projectData);
2626
}
2727

28-
public canStartTestRun(projectData: IProjectData): boolean {
28+
public async canStartTestRun(projectData: IProjectData): Promise<boolean> {
2929
return (
3030
this.isVitestProject(projectData) &&
31-
!!resolvePackagePath("vitest", { paths: [projectData.projectDir] })
31+
!!(await this.getVitestPackagePath(projectData))
3232
);
3333
}
3434

3535
public async startTestRun(
3636
platform: string,
3737
projectData: IProjectData,
3838
): Promise<void> {
39-
const vitestPackagePath = resolvePackagePath("vitest", {
40-
paths: [projectData.projectDir],
41-
});
39+
const vitestPackagePath = await this.getVitestPackagePath(projectData);
4240
if (!vitestPackagePath) {
4341
this.$errors.fail(
4442
"Unable to find 'vitest' in the project. Run '$ ns test init --framework vitest' first.",
@@ -90,6 +88,14 @@ export class VitestExecutionService implements IVitestExecutionService {
9088
}
9189
return null;
9290
}
91+
92+
private getVitestPackagePath(projectData: IProjectData): Promise<string> {
93+
return this.$packageManager.getInstalledPackagePath(
94+
"vitest",
95+
projectData.projectDir,
96+
);
97+
}
98+
9399
}
94100

95101
injector.register("vitestExecutionService", VitestExecutionService);

‎test/extension-manifests.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { assert } from "chai";
2+
import { resolvePackagePath } from "../lib/helpers/package-path-helper";
23
import * as fs from "fs";
34
import * as os from "os";
45
import * as path from "path";
@@ -184,6 +185,11 @@ describe("extension manifests", () => {
184185
install: async (): Promise<any> => {
185186
throw new Error("Extensions are expected to be installed already.");
186187
},
188+
getInstalledPackagePath: async (
189+
packageName: string,
190+
fromDir: string,
191+
): Promise<string> =>
192+
resolvePackagePath(packageName, { paths: [fromDir] }) || null,
187193
uninstall: async (): Promise<any> => undefined,
188194
searchNpms: async (): Promise<any> => ({ results: [] }),
189195
getRegistryPackageData: async (): Promise<any> => ({}),

‎test/services/bundler/bundler-compiler-service.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ function createTestInjector(
5858
const testInjector = new Yok();
5959
testInjector.register("packageManager", {
6060
getPackageManagerName: async () => packageManager,
61+
getInstalledPackagePath: async (): Promise<string> => null,
6162
});
6263
testInjector.register("bundlerCompilerService", BundlerCompilerService);
6364
testInjector.register("childProcess", {});

‎test/services/extensibility-service.ts‎

Lines changed: 31 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,17 @@ describe("extensibilityService", () => {
8787
return testInjector;
8888
};
8989

90+
const stubInstalledExtensions = (
91+
testInjector: IInjector,
92+
resolve: (extensionName: string, fromDir: string) => string,
93+
): void => {
94+
const packageManager = testInjector.resolve("packageManager");
95+
packageManager.getInstalledPackagePath = async (
96+
packageName: string,
97+
fromDir: string,
98+
): Promise<string> => resolve(packageName, fromDir);
99+
};
100+
90101
const getExpectedInstallationPathForExtension = (
91102
testInjector: IInjector,
92103
extensionName: string,
@@ -320,14 +331,9 @@ describe("extensibilityService", () => {
320331
const fs: IFileSystem = testInjector.resolve("fs");
321332
const extensionNames = ["extension1", "extension2", "extension3"];
322333
fs.exists = (pathToCheck: string): boolean => true;
323-
fs.readDirectory = (dir: string): string[] => {
324-
assert.deepStrictEqual(
325-
path.basename(dir),
326-
constants.NODE_MODULES_FOLDER_NAME,
327-
);
328-
// Simulates extensions are installed in node_modules
329-
return extensionNames;
330-
};
334+
stubInstalledExtensions(testInjector, (name, fromDir) =>
335+
path.join(fromDir, constants.NODE_MODULES_FOLDER_NAME, name),
336+
);
331337

332338
mockFsReadJson(testInjector, extensionNames);
333339

@@ -359,20 +365,15 @@ describe("extensibilityService", () => {
359365
fs.exists = (pathToCheck: string): boolean =>
360366
path.basename(pathToCheck) !== extensionNames[0];
361367

362-
let isFirstReadDirExecution = true;
363-
fs.readDirectory = (dir: string): string[] => {
364-
assert.deepStrictEqual(
365-
path.basename(dir),
366-
constants.NODE_MODULES_FOLDER_NAME,
367-
);
368-
// Simulates extensions are installed in node_modules
369-
if (isFirstReadDirExecution) {
370-
isFirstReadDirExecution = false;
371-
return extensionNames.filter((ext) => ext !== "extension1");
372-
} else {
373-
return extensionNames;
368+
// extension1 is missing until the service installs it
369+
let isExtension1Installed = false;
370+
stubInstalledExtensions(testInjector, (name, fromDir) => {
371+
if (name === "extension1" && !isExtension1Installed) {
372+
isExtension1Installed = true;
373+
return null;
374374
}
375-
};
375+
return path.join(fromDir, constants.NODE_MODULES_FOLDER_NAME, name);
376+
});
376377

377378
mockFsReadJson(testInjector, extensionNames);
378379

@@ -415,14 +416,9 @@ describe("extensibilityService", () => {
415416
const fs: IFileSystem = testInjector.resolve("fs");
416417
const extensionNames = ["extension1", "extension2", "extension3"];
417418
fs.exists = (pathToCheck: string): boolean => true;
418-
fs.readDirectory = (dir: string): string[] => {
419-
assert.deepStrictEqual(
420-
path.basename(dir),
421-
constants.NODE_MODULES_FOLDER_NAME,
422-
);
423-
// Simulates extensions are installed in node_modules
424-
return extensionNames;
425-
};
419+
stubInstalledExtensions(testInjector, (name, fromDir) =>
420+
path.join(fromDir, constants.NODE_MODULES_FOLDER_NAME, name),
421+
);
426422

427423
mockFsReadJson(testInjector, extensionNames);
428424

@@ -469,7 +465,7 @@ describe("extensibilityService", () => {
469465
}
470466
});
471467

472-
it("rejects all promises when unable to read node_modules dir (simulate EPERM error)", async () => {
468+
it("rejects all promises when the package manager cannot locate extensions (simulate EPERM error)", async () => {
473469
const testInjector = getTestInjector();
474470
const extensionNames = ["extension1", "extension2", "extension3"];
475471
const fs: IFileSystem = testInjector.resolve("fs");
@@ -480,14 +476,10 @@ describe("extensibilityService", () => {
480476
mockFsReadJson(testInjector, extensionNames);
481477

482478
let isReadDirCalled = false;
483-
fs.readDirectory = (dir: string): string[] => {
479+
stubInstalledExtensions(testInjector, () => {
484480
isReadDirCalled = true;
485-
assert.deepStrictEqual(
486-
path.basename(dir),
487-
constants.NODE_MODULES_FOLDER_NAME,
488-
);
489481
throw new Error(expectedErrorMessage);
490-
};
482+
});
491483

492484
const extensibilityService: IExtensibilityService =
493485
testInjector.resolve(ExtensibilityService);
@@ -532,14 +524,10 @@ describe("extensibilityService", () => {
532524
mockFsReadJson(testInjector, extensionNames);
533525

534526
let isReadDirCalled = false;
535-
fs.readDirectory = (dir: string): string[] => {
527+
stubInstalledExtensions(testInjector, () => {
536528
isReadDirCalled = true;
537-
assert.deepStrictEqual(
538-
path.basename(dir),
539-
constants.NODE_MODULES_FOLDER_NAME,
540-
);
541-
return [];
542-
};
529+
return null;
530+
});
543531

544532
let isNpmInstallCalled = false;
545533
const npm: INodePackageManager = testInjector.resolve("npm");

0 commit comments

Comments
 (0)