Merge pull request #963 from lntutor/feat/transitive-plugin-discovery-953

Discover transitive NPM plugins

packages/lib/src/load_plugin_list.ts

@@ -1,4 +1,5 @@
 import fs from "node:fs/promises";
+import { createRequire } from "node:module";
 import path from "path";
 import { logger } from "./logging";
 import * as libFileOps from "./file_ops";
@@ -73,6 +74,18 @@ function getPluginName(requireSpec: string) {
 	return context.pluginInfo.name;
 }
 
+function resolvePackagePath(packageName: string, fromPath: string): string | undefined {
+	const packageRequire = createRequire(path.join(fromPath, "package.json"));
+	try {
+		return path.dirname(packageRequire.resolve(`${packageName}/package.json`));
+	} catch (err: any) {
+		if (err.code === "MODULE_NOT_FOUND" || err.code === "ERR_PACKAGE_PATH_NOT_EXPORTED") {
+			return undefined;
+		}
+		throw err;
+	}
+}
+
 /**
  * Find NPM packages in the root package.json that satisfies the requirements for plugins.
  * Adds discovered plugins to the plugin list.
@@ -98,13 +111,36 @@ async function findNpmPlugins(pluginList: Map<string, string>): Promise<boolean>
 	}
 
 	let hasChanged = 0;
-	for (const [packageName, packageVersion] of Object.entries(dependencies)) {
-		if ([...pluginList.values()].includes(packageName)) {
-			continue; // This npm module is already in the plugin list
+	const pendingPackages = Object.entries(dependencies).map(([packageName, packageVersion]) => ({
+		packageName,
+		packageVersion,
+		parentPath: process.cwd(),
+		isRootDependency: true,
+	}));
+	const corePackages = new Set([
+		"@clusterio/controller",
+		"@clusterio/ctl",
+		"@clusterio/host",
+		"@clusterio/lib",
+		"@clusterio/web_ui",
+	]);
+	const visitedPackages = new Set<string>();
+
+	while (pendingPackages.length) {
+		const { packageName, packageVersion, parentPath, isRootDependency } = pendingPackages.shift()!;
+		if (visitedPackages.has(packageName)) {
+			continue;
 		}
-		// Find the package in node_modules and read the package.json to check if it's a clusterio-plugin
-		if (await checkPackageJson(path.resolve("node_modules", packageName))) {
-			const pluginName = getPluginName(path.resolve("node_modules", packageName));
+
+		const npmPackagePath = resolvePackagePath(packageName, parentPath);
+		if (!npmPackagePath) {
+			continue;
+		}
+
+		visitedPackages.add(packageName);
+
+		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, ` +
@@ -112,16 +148,22 @@ async function findNpmPlugins(pluginList: Map<string, string>): Promise<boolean>
 				);
 			} else {
 				pluginList.set(pluginName, packageName);
-				logger.info(`Added ${pluginName} from NPM`);
+				logger.info(`Added ${pluginName} from node modules`);
 				hasChanged += 1;
 			}
-		} else if (![
-			"@clusterio/controller",
-			"@clusterio/ctl",
-			"@clusterio/host",
-			"@clusterio/lib",
-			"@clusterio/web_ui",
-		].includes(packageName)) {
+
+			const packageJson = JSON.parse(
+				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: npmPackagePath,
+					isRootDependency: false,
+				});
+			}
+		} else if (isRootDependency && !corePackages.has(packageName)) {
 			logger.warn(`${packageName}@${packageVersion} is not a clusterio-plugin`);
 		}
 	}

test/lib/plugin_loader.js

@@ -121,15 +121,27 @@ 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 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 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() {
 			// Setup test plugins
-			async function writePlugin(pluginPath, name) {
+			async function writePlugin(
+				pluginPath,
+				name,
+				dependencies = {},
+				packageJsonFields = {},
+			) {
 				await fs.mkdir(pluginPath, { recursive: true });
 				await fs.writeFile(
 					path.join(pluginPath, "index.js"),
@@ -141,42 +153,72 @@ describe("lib/plugin_loader", function() {
 						name: path.basename(pluginPath),
 						version: "0.0.1",
 						keywords: ["clusterio-plugin"],
+						dependencies,
+						...packageJsonFields,
 					})
 				);
 			}
 
+			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");
+			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-aliased": "npm:real-plugin@^0.0.1",
+						"plugin-deep": "^0.0.1",
 					},
 				})
 			);
-			// Create an npm module that is not a plugin
-			await fs.mkdir(notAPluginPath, { recursive: true });
-			await fs.writeFile(
-				path.join(notAPluginPath, "package.json"),
-				JSON.stringify({ name: "not-a-plugin", version: "1.0.0" })
-			);
 
 			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"));
@@ -186,11 +228,18 @@ describe("lib/plugin_loader", function() {
 
 		it("should discover npm plugins", async function() {
 			assert.ok(pluginList.has("npm"));
-			assert.strictEqual(pluginList.get("npm"), "npm-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"), "plugin-transitive");
+			assert.strictEqual(pluginList.get("nested"), undefined);
+			assert.strictEqual(pluginList.get("hidden"), undefined);
+		});
+
 		it("should load existing plugin list", async function() {
 			const existingList = new Map([["test", "/test/path"]]);
 			await fs.writeFile(pluginListPath, JSON.stringify([...existingList]));
@@ -200,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() {
@@ -219,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 });
-		});
 	});
 });