Skip to content

Commit aa7e55c

Browse files
rigor789edusperoni
rigor789
authored and
edusperoni
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 51a7403 commit aa7e55c

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
@@ -108,7 +108,7 @@ async function canExecuteTestCommand(
108108

109109
if ($vitestExecutionService.isVitestProject($projectData)) {
110110
const canStartTestRun =
111-
$vitestExecutionService.canStartTestRun($projectData);
111+
await $vitestExecutionService.canStartTestRun($projectData);
112112
if (!canStartTestRun) {
113113
$errors.fail({
114114
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`,
@@ -1170,21 +1169,24 @@ export class BundlerCompilerService
11701169
});
11711170
}
11721171

1173-
private getBundlerExecutablePath(projectData: IProjectData): string {
1172+
private async getBundlerExecutablePath(
1173+
projectData: IProjectData,
1174+
): Promise<string> {
11741175
const bundler = this.getBundler();
1176+
const resolve = (packageName: string) =>
1177+
this.$packageManager.getInstalledPackagePath(
1178+
packageName,
1179+
projectData.projectDir,
1180+
);
11751181

11761182
if (bundler === "vite") {
1177-
const packagePath = resolvePackagePath(`vite`, {
1178-
paths: [projectData.projectDir],
1179-
});
1183+
const packagePath = await resolve("vite");
11801184

11811185
if (packagePath) {
11821186
return path.resolve(packagePath, "bin", "vite.js");
11831187
}
1184-
} else if (this.isModernBundler(projectData)) {
1185-
const packagePath = resolvePackagePath(this.getBundlerPackageName(), {
1186-
paths: [projectData.projectDir],
1187-
});
1188+
} else if (await this.isModernBundler(projectData)) {
1189+
const packagePath = await resolve(this.getBundlerPackageName());
11881190

11891191
if (packagePath) {
11901192
return path.resolve(packagePath, "dist", "bin", "index.js");
@@ -1204,9 +1206,7 @@ export class BundlerCompilerService
12041206
);
12051207
}
12061208

1207-
const packagePath = resolvePackagePath("webpack", {
1208-
paths: [projectData.projectDir],
1209-
});
1209+
const packagePath = await resolve("webpack");
12101210

12111211
if (!packagePath) {
12121212
return "";
@@ -1228,21 +1228,21 @@ export class BundlerCompilerService
12281228
);
12291229
}
12301230

1231-
private isModernBundler(projectData: IProjectData): boolean {
1231+
private async isModernBundler(projectData: IProjectData): Promise<boolean> {
12321232
const bundler = this.getBundler();
12331233
switch (bundler) {
12341234
case "rspack":
12351235
return true;
12361236
default:
1237-
const packageJSONPath = resolvePackageJSONPath(
1237+
const packagePath = await this.$packageManager.getInstalledPackagePath(
12381238
this.getBundlerPackageName(),
1239-
{
1240-
paths: [projectData.projectDir],
1241-
},
1239+
projectData.projectDir,
12421240
);
12431241

1244-
if (packageJSONPath) {
1245-
const packageData = this.$fs.readJson(packageJSONPath);
1242+
if (packagePath) {
1243+
const packageData = this.$fs.readJson(
1244+
path.join(packagePath, "package.json"),
1245+
);
12461246
const ver = semver.coerce(packageData.version);
12471247

12481248
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
@@ -62,6 +62,7 @@ function createTestInjector(
6262
const testInjector = new Yok();
6363
testInjector.register("packageManager", {
6464
getPackageManagerName: async () => packageManager,
65+
getInstalledPackagePath: async (): Promise<string> => null,
6566
});
6667
testInjector.register("bundlerCompilerService", BundlerCompilerService);
6768
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)