From bfbd41d64bf434ebdd75f3c4e032eb2dc1435d16 Mon Sep 17 00:00:00 2001 From: Cooldude2606 <25043174+Cooldude2606@users.noreply.github.com> Date: Fri, 31 Jul 2026 18:33:19 +0100 Subject: [PATCH] Cleaner path resolution and tests --- packages/lib/src/load_plugin_list.ts | 66 +++++--------- test/lib/plugin_loader.js | 125 +++++++++++++-------------- 2 files changed, 79 insertions(+), 112 deletions(-) diff --git a/packages/lib/src/load_plugin_list.ts b/packages/lib/src/load_plugin_list.ts index 7eedcaf8..8d4d1689 100644 --- a/packages/lib/src/load_plugin_list.ts +++ b/packages/lib/src/load_plugin_list.ts @@ -74,42 +74,16 @@ function getPluginName(requireSpec: string) { return context.pluginInfo.name; } -async function resolveNpmPackage( - packageName: string, - parentPath: string, -): Promise<{ packagePath: string; requirePath: string } | undefined> { - const packageRequire = createRequire(path.join(parentPath, "package.json")); - let entryPath; +function resolvePackagePath(packageName: string, fromPath: string): string | undefined { + const packageRequire = createRequire(path.join(fromPath, "package.json")); try { - entryPath = packageRequire.resolve(packageName); + return path.dirname(packageRequire.resolve(`${packageName}/package.json`)); } catch (err: any) { - if (err.code === "MODULE_NOT_FOUND") { + if (err.code === "MODULE_NOT_FOUND" || err.code === "ERR_PACKAGE_PATH_NOT_EXPORTED") { return undefined; } throw err; } - - const resolvedEntryPath = await fs.realpath(entryPath); - for (const searchPath of packageRequire.resolve.paths(packageName) ?? []) { - const packagePath = path.join(searchPath, packageName); - try { - await fs.access(path.join(packagePath, "package.json")); - const resolvedPackagePath = await fs.realpath(packagePath); - const relativeEntryPath = path.relative(resolvedPackagePath, resolvedEntryPath); - if ( - relativeEntryPath !== ".." - && !relativeEntryPath.startsWith(`..${path.sep}`) - && !path.isAbsolute(relativeEntryPath) - ) { - return { packagePath, requirePath: entryPath }; - } - } catch (err: any) { - if (err.code !== "ENOENT") { - throw err; - } - } - } - return undefined; } /** @@ -143,6 +117,13 @@ async function findNpmPlugins(pluginList: Map): Promise parentPath: process.cwd(), isRootDependency: true, })); + const corePackages = new Set([ + "@clusterio/controller", + "@clusterio/ctl", + "@clusterio/host", + "@clusterio/lib", + "@clusterio/web_ui", + ]); const visitedPackages = new Set(); while (pendingPackages.length) { @@ -150,15 +131,16 @@ async function findNpmPlugins(pluginList: Map): Promise if (visitedPackages.has(packageName)) { continue; } - const npmPackage = await resolveNpmPackage(packageName, parentPath); - if (!npmPackage) { + + const npmPackagePath = resolvePackagePath(packageName, parentPath); + if (!npmPackagePath) { continue; } - const { packagePath, requirePath } = npmPackage; + visitedPackages.add(packageName); - if (await checkPackageJson(packagePath)) { - const pluginName = getPluginName(requirePath); + if (await checkPackageJson(npmPackagePath)) { + const pluginName = getPluginName(npmPackagePath); if (pluginList.has(pluginName)) { logger.warn( `${pluginName} provided by ${packageName}@${packageVersion} is already in the plugin list, ` + @@ -166,28 +148,22 @@ async function findNpmPlugins(pluginList: Map): Promise ); } else { pluginList.set(pluginName, packageName); - logger.info(`Added ${pluginName} from NPM`); + logger.info(`Added ${pluginName} from node modules`); hasChanged += 1; } const packageJson = JSON.parse( - await fs.readFile(path.join(packagePath, "package.json"), { encoding: "utf8" }) + await fs.readFile(path.join(npmPackagePath, "package.json"), { encoding: "utf8" }) ); for (const [dependencyName, dependencyVersion] of Object.entries(packageJson.dependencies ?? {})) { pendingPackages.push({ packageName: dependencyName, packageVersion: dependencyVersion, - parentPath: packagePath, + parentPath: npmPackagePath, isRootDependency: false, }); } - } else if (isRootDependency && ![ - "@clusterio/controller", - "@clusterio/ctl", - "@clusterio/host", - "@clusterio/lib", - "@clusterio/web_ui", - ].includes(packageName)) { + } else if (isRootDependency && !corePackages.has(packageName)) { logger.warn(`${packageName}@${packageVersion} is not a clusterio-plugin`); } } diff --git a/test/lib/plugin_loader.js b/test/lib/plugin_loader.js index 2a800369..946e7f1e 100644 --- a/test/lib/plugin_loader.js +++ b/test/lib/plugin_loader.js @@ -121,17 +121,17 @@ describe("lib/plugin_loader", function() { const localPluginPathAbs = path.resolve(localPluginPath); const externalPluginPath = path.join(baseDir, "external_plugins", "external_plugin"); const externalPluginPathAbs = path.resolve(externalPluginPath); - const npmPluginPath = path.join(baseDir, "node_modules", "npm-plugin"); - const aliasedPluginPath = path.join(baseDir, "node_modules", "plugin-alias"); - const nestedManifestPluginPath = path.join(baseDir, "node_modules", "nested-manifest-plugin"); - const monorepoPluginPath = path.join(baseDir, "external_plugins", "monorepo", "monorepo-plugin"); + const monorepoPluginPath = path.join(baseDir, "external_plugins", "monorepo", "monorepo_plugin"); const monorepoPluginPathAbs = path.resolve(monorepoPluginPath); - const transitivePluginPath = path.join(baseDir, "node_modules", "transitive-plugin"); - const duplicateNpmPluginPath = path.join( - transitivePluginPath, "node_modules", "npm-plugin" - ); - const hiddenPluginPath = path.join(baseDir, "node_modules", "hidden-plugin"); - const notAPluginPath = path.join(baseDir, "node_modules", "not-a-plugin"); + + const nodeModules = path.join(baseDir, "node_modules"); + const npmPluginPath = path.join(nodeModules, "plugin-npm"); + const aliasedPluginPath = path.join(nodeModules, "plugin-aliased"); + const deepPluginPath = path.join(nodeModules, "plugin-deep"); + const transitivePluginPath = path.join(nodeModules, "plugin-transitive"); + const nestedPluginPath = path.join(transitivePluginPath, "node_modules", "plugin-nested"); + const hiddenPluginPath = path.join(nodeModules, "plugin-hidden"); + const notAPluginPath = path.join(nodeModules, "not-a-plugin"); let pluginList; before(async function() { @@ -140,7 +140,7 @@ describe("lib/plugin_loader", function() { pluginPath, name, dependencies = {}, - packageJsonFields = { exports: "./index.js" }, + packageJsonFields = {}, ) { await fs.mkdir(pluginPath, { recursive: true }); await fs.writeFile( @@ -159,69 +159,66 @@ describe("lib/plugin_loader", function() { ); } + try { + await fs.rm(baseDir, { force: true, recursive: true, maxRetries: 10 }); + } catch {} + // Create local plugin - await writePlugin(localPluginPath, "local_plugin"); + await writePlugin(localPluginPath, "local"); + // Create external plugin - await writePlugin(externalPluginPath, "external_plugin"); - // Create npm plugin - await writePlugin( - npmPluginPath, - "npm", - { "transitive-plugin": "^1.0.0" }, - ); - await writePlugin(transitivePluginPath, "transitive", { "npm-plugin": "^1.0.0" }); - await writePlugin(duplicateNpmPluginPath, "nested-npm"); - await writePlugin( - aliasedPluginPath, - "aliased", - {}, - { name: "real-plugin", exports: "./index.js" }, - ); - await writePlugin( - nestedManifestPluginPath, - "unused-root-entrypoint", - {}, - { exports: "./dist/index.js" }, - ); - await fs.mkdir(path.join(nestedManifestPluginPath, "dist")); - await fs.writeFile( - path.join(nestedManifestPluginPath, "dist", "index.js"), - 'module.exports.plugin = { name: "nested-manifest" };' - ); - await fs.writeFile( - path.join(nestedManifestPluginPath, "dist", "package.json"), - JSON.stringify({ type: "commonjs" }) - ); + await writePlugin(externalPluginPath, "external"); + // Write a monorepo plugin - await writePlugin(monorepoPluginPath, "monorepo-plugin"); + await writePlugin(monorepoPluginPath, "monorepo"); + + // Create a plugin that is not in root or transitive + await writePlugin(hiddenPluginPath, "hidden"); + + // Create npm plugins + await writePlugin(npmPluginPath, "npm", { "plugin-transitive": "^1.0.0" }); + await writePlugin(transitivePluginPath, "transitive", { "plugin-npm": "^1.0.0" }); + await writePlugin(nestedPluginPath, "nested"); + await writePlugin(aliasedPluginPath, "aliased", {}, { name: "real-plugin" }); + + // Create npm plugin with deep entry point + await writePlugin(deepPluginPath, "unused", {}, { + main: "./dist/index.js", + }); + await fs.mkdir(path.join(deepPluginPath, "dist")); + await fs.writeFile( + path.join(deepPluginPath, "dist", "index.js"), + 'module.exports.plugin = { name: "deep" };' + ); + + // Create an npm module that is not a plugin + await writePlugin(notAPluginPath, "not-a-plugin", { "plugin-hidden": "^0.0.1" }, { keywords: [] }); + // Create root package.json await fs.writeFile( path.join(baseDir, "package.json"), JSON.stringify({ dependencies: { - "npm-plugin": "^1.0.0", + "plugin-npm": "^1.0.0", "not-a-plugin": "^1.0.0", - "plugin-alias": "npm:real-plugin@^0.0.1", - "nested-manifest-plugin": "^0.0.1", + "plugin-aliased": "npm:real-plugin@^0.0.1", + "plugin-deep": "^0.0.1", }, }) ); - // Create an npm module that is not a plugin - await writePlugin( - notAPluginPath, - "not-a-plugin", - { "hidden-plugin": "^0.0.1" }, - { keywords: [] }, - ); - await writePlugin(hiddenPluginPath, "hidden"); process.chdir(baseDir); pluginList = await lib.loadPluginList(pluginListPath, true); }); + beforeEach(async function() { await fs.rm(pluginListPath, { force: true }); }); + after(async function() { + process.chdir(old_cwd); + }); + it("should discover local plugins", async function() { assert.ok(pluginList.has("local_plugin")); assert.ok(pluginList.has("external_plugin")); @@ -231,16 +228,15 @@ describe("lib/plugin_loader", function() { it("should discover npm plugins", async function() { assert.ok(pluginList.has("npm")); - assert.strictEqual(pluginList.get("npm"), "npm-plugin"); - assert.strictEqual(pluginList.get("aliased"), "plugin-alias"); - assert.strictEqual(pluginList.get("nested-manifest"), "nested-manifest-plugin"); - // Check that it does not contain not-a-plugin + assert.strictEqual(pluginList.get("npm"), "plugin-npm"); + assert.strictEqual(pluginList.get("aliased"), "plugin-aliased"); + assert.strictEqual(pluginList.get("deep"), "plugin-deep"); assert.strictEqual(pluginList.get("not-a-plugin"), undefined); }); it("should discover transitive npm plugins without traversing non-plugin packages", async function() { - assert.strictEqual(pluginList.get("transitive"), "transitive-plugin"); - assert.strictEqual(pluginList.get("nested-npm"), undefined); + assert.strictEqual(pluginList.get("transitive"), "plugin-transitive"); + assert.strictEqual(pluginList.get("nested"), undefined); assert.strictEqual(pluginList.get("hidden"), undefined); }); @@ -253,8 +249,8 @@ describe("lib/plugin_loader", function() { }); it("should support monorepo plugins", async function() { - assert.ok(pluginList.has("monorepo-plugin")); - assert.strictEqual(pluginList.get("monorepo-plugin"), monorepoPluginPathAbs); + assert.ok(pluginList.has("monorepo_plugin")); + assert.strictEqual(pluginList.get("monorepo_plugin"), monorepoPluginPathAbs); }); it("should not throw when package.json has no dependencies field", async function() { @@ -272,10 +268,5 @@ describe("lib/plugin_loader", function() { await lib.loadPluginList(pluginListPath, true); }); }); - - after(async function() { - process.chdir(old_cwd); - await fs.rm(baseDir, { force: true, recursive: true, maxRetries: 10 }); - }); }); });