From 636777d9352d15feb419bf3573123e976424bcd4 Mon Sep 17 00:00:00 2001 From: Danielv123 Date: Mon, 25 Nov 2024 12:22:16 +0000 Subject: [PATCH 01/16] Add locally provided plugins by default --- packages/controller/controller.ts | 9 +--- packages/ctl/ctl.ts | 9 +--- packages/host/host.ts | 9 +--- packages/lib/index.ts | 1 + packages/lib/src/load_plugin_list.ts | 74 ++++++++++++++++++++++++++++ packages/lib/src/plugin_loader.ts | 9 ++++ 6 files changed, 87 insertions(+), 24 deletions(-) create mode 100644 packages/lib/src/load_plugin_list.ts diff --git a/packages/controller/controller.ts b/packages/controller/controller.ts index d9bdaf9c..6587e01d 100644 --- a/packages/controller/controller.ts +++ b/packages/controller/controller.ts @@ -317,14 +317,7 @@ async function initialize(): Promise { } logger.info(`Loading available plugins from ${args.pluginList}`); - let pluginList = new Map(); - try { - pluginList = new Map(JSON.parse(await fs.readFile(args.pluginList, { encoding: "utf8" }))); - } catch (err: any) { - if (err.code !== "ENOENT") { - throw err; - } - } + let pluginList = await lib.loadPluginList(args.pluginList); // If the command is plugin management we don't try to load plugins if (command === "plugin") { diff --git a/packages/ctl/ctl.ts b/packages/ctl/ctl.ts index b5edb31c..70d49964 100755 --- a/packages/ctl/ctl.ts +++ b/packages/ctl/ctl.ts @@ -188,14 +188,7 @@ async function startControl() { lib.handleUnhandledErrors(); logger.verbose(`Loading available plugins from ${args.pluginList}`); - let pluginList = new Map(); - try { - pluginList = new Map(JSON.parse(await fs.readFile(args.pluginList, "utf8"))); - } catch (err: any) { - if (err.code !== "ENOENT") { - throw err; - } - } + let pluginList = await lib.loadPluginList(args.pluginList); // If the command is plugin management we don't try to load plugins if (args._[0] === "plugin") { diff --git a/packages/host/host.ts b/packages/host/host.ts index b0123964..0247e9aa 100755 --- a/packages/host/host.ts +++ b/packages/host/host.ts @@ -147,14 +147,7 @@ async function startHost() { } logger.info(`Loading available plugins from ${args.pluginList}`); - let pluginList = new Map(); - try { - pluginList = new Map(JSON.parse(await fs.readFile(args.pluginList, "utf8"))); - } catch (err: any) { - if (err.code !== "ENOENT") { - throw err; - } - } + let pluginList = await lib.loadPluginList(args.pluginList); // If the command is plugin management we don't try to load plugins if (command === "plugin") { diff --git a/packages/lib/index.ts b/packages/lib/index.ts index bec088ad..9cd87213 100644 --- a/packages/lib/index.ts +++ b/packages/lib/index.ts @@ -29,6 +29,7 @@ export * from "./src/system_collectors"; export * from "./src/zip_ops"; export * from "./src/subscriptions"; export * from "./src/datastore"; +export * from "./src/load_plugin_list"; export { default as ExponentialBackoff } from "./src/ExponentialBackoff"; export { default as ModStore } from "./src/ModStore"; diff --git a/packages/lib/src/load_plugin_list.ts b/packages/lib/src/load_plugin_list.ts new file mode 100644 index 00000000..3545c747 --- /dev/null +++ b/packages/lib/src/load_plugin_list.ts @@ -0,0 +1,74 @@ +import fs from "fs-extra"; +import path from "path"; +import { logger } from "./logging"; +import * as libFileOps from "./file_ops"; + +/** + * Find local plugins to add to the plugin list. This is primarily used during development. + */ +async function findLocalPlugins(pluginList: Map, pluginListPath: string) { + // Check folders for plugins + const pluginFolders = [ + path.join(process.cwd(), "plugins"), + path.join(process.cwd(), "external_plugins"), + ]; + + let has_changed = 0; + for (const pluginFolder of pluginFolders) { + const pluginNames = await fs.readdir(pluginFolder); + for (const pluginName of pluginNames) { + if (pluginList.has(pluginName)) { + continue; + } + const pluginPath = path.resolve(pluginFolder, pluginName); + const stats = await fs.stat(pluginPath); + if (!stats.isDirectory()) { + continue; + } + + // Check that the plugin has a package.json file + const packageJsonPath = path.resolve(pluginPath, "package.json"); + if (!(await fs.exists(packageJsonPath))) { + continue; + } + console.log(`Adding ${pluginName} from ${pluginFolder}`); + + pluginList.set(pluginName, pluginPath); + logger.info(`Added ${pluginName} from ${pluginFolder}`); + has_changed++; + } + } + + if (has_changed > 0) { + await libFileOps.safeOutputFile(pluginListPath, JSON.stringify([...pluginList], null, "\t")); + } +} + +/** + * Loads the plugin list from the specified file and optionally discovers local plugins + * + * @param pluginListPath - Path to the plugin list JSON file + * @param findLocal - Whether to look for local plugins in plugins/ and external_plugins/ directories + * @returns Map of plugin name to plugin path + */ +export async function loadPluginList(pluginListPath: string, findLocal = true): Promise> { + let pluginList = new Map(); + + // Try to load existing plugin list + try { + pluginList = new Map(JSON.parse(await fs.readFile(pluginListPath, { encoding: "utf8" }))); + logger.info(`Loaded ${pluginList.size} plugins from ${pluginListPath}`); + } catch (err: any) { + if (err.code !== "ENOENT") { + throw err; + } + logger.info(`No existing plugin list found at ${pluginListPath}`); + } + + // Optionally discover local plugins + if (findLocal) { + await findLocalPlugins(pluginList, pluginListPath); + } + + return pluginList; +} diff --git a/packages/lib/src/plugin_loader.ts b/packages/lib/src/plugin_loader.ts index 3c1af67f..824ca94f 100644 --- a/packages/lib/src/plugin_loader.ts +++ b/packages/lib/src/plugin_loader.ts @@ -26,6 +26,15 @@ export async function loadPluginInfos(pluginList: Map) { let pluginInfo: libPlugin.PluginNodeEnvInfo; let pluginPackage: { name?: string, version: string, main?: string, private?: boolean }; + // Check if plugin path exists, otherwise remove it + try { + require.resolve(pluginPath); + } catch (err) { + logger.warn(`Plugin path ${pluginPath} does not exist, removing ${pluginName}`); + pluginList.delete(pluginName); + continue; + } + try { pluginInfo = require(pluginPath).plugin; pluginPackage = require(path.posix.join(pluginPath, "package.json")); From 397dc6a5a621a1341a751edd80d4133d1ab3edf2 Mon Sep 17 00:00:00 2001 From: Danielv123 Date: Mon, 25 Nov 2024 17:23:03 +0000 Subject: [PATCH 02/16] Autodiscover plugins in dependencies with keyword "clusterio-plugin" --- packages/create/templates/common/package.json | 3 ++ packages/lib/src/load_plugin_list.ts | 34 +++++++++++++++++++ plugins/global_chat/package.json | 3 ++ plugins/inventory_sync/package.json | 3 ++ plugins/player_auth/package.json | 3 ++ plugins/research_sync/package.json | 3 ++ plugins/statistics_exporter/package.json | 3 ++ plugins/subspace_storage/package.json | 3 ++ 8 files changed, 55 insertions(+) diff --git a/packages/create/templates/common/package.json b/packages/create/templates/common/package.json index b05122b3..36e5ad01 100644 --- a/packages/create/templates/common/package.json +++ b/packages/create/templates/common/package.json @@ -8,6 +8,9 @@ //%if !typescript "main": "index.js", //%endif + "keywords": [ + "clusterio-plugin" + ], "scripts": { "prepare": "__prepare__" }, diff --git a/packages/lib/src/load_plugin_list.ts b/packages/lib/src/load_plugin_list.ts index 3545c747..e0eb2b4f 100644 --- a/packages/lib/src/load_plugin_list.ts +++ b/packages/lib/src/load_plugin_list.ts @@ -44,6 +44,37 @@ async function findLocalPlugins(pluginList: Map, pluginListPath: } } +/** + * Find NPM packages in the root package.json that satisfies the requirements for plugins. + */ +async function findNpmPlugins(pluginList: Map, pluginListPath: string) { + const rootPackageJson = JSON.parse(await fs.readFile(path.resolve(process.cwd(), "package.json"), { encoding: "utf8" })); + const dependencies = rootPackageJson.dependencies; + + let changed = 0; + for (const [pluginName, pluginVersion] of Object.entries(dependencies)) { + if (pluginList.has(pluginName)) { + console.log(`${pluginName} already exists`); + continue; + } + // Find the package in node_modules and read the package.json + const packageJsonPath = path.resolve(process.cwd(), "node_modules", pluginName, "package.json"); + const packageJson = JSON.parse(await fs.readFile(packageJsonPath, { encoding: "utf8" })); + // Check if the package has the "clusterio-plugin" keyword + if (packageJson.keywords?.includes("clusterio-plugin")) { + const pluginInfo = require(path.resolve(process.cwd(), "node_modules", pluginName)).plugin; + pluginList.set(pluginInfo.name, pluginName); + logger.info(`Added ${pluginInfo.name} from NPM`); + changed++; + } else { + logger.info(`${pluginName}@${pluginVersion} is not a clusterio-plugin`); + } + } + if (changed > 0) { + await libFileOps.safeOutputFile(pluginListPath, JSON.stringify([...pluginList], null, "\t")); + } +} + /** * Loads the plugin list from the specified file and optionally discovers local plugins * @@ -70,5 +101,8 @@ export async function loadPluginList(pluginListPath: string, findLocal = true): await findLocalPlugins(pluginList, pluginListPath); } + // Optionally discover NPM plugins + await findNpmPlugins(pluginList, pluginListPath); + return pluginList; } diff --git a/plugins/global_chat/package.json b/plugins/global_chat/package.json index b8d97aa2..c20e1846 100644 --- a/plugins/global_chat/package.json +++ b/plugins/global_chat/package.json @@ -7,6 +7,9 @@ "license": "MIT", "repository": "clusterio/clusterio", "main": "dist/node/index.js", + "keywords": [ + "clusterio-plugin" + ], "scripts": { "prepare": "tsc --build && webpack-cli --env production", "test": "echo \"Error: run tests from root\" && exit 1" diff --git a/plugins/inventory_sync/package.json b/plugins/inventory_sync/package.json index e184e734..03f31ade 100644 --- a/plugins/inventory_sync/package.json +++ b/plugins/inventory_sync/package.json @@ -7,6 +7,9 @@ "license": "MIT", "repository": "clusterio/factorioClusterio", "main": "dist/node/index.js", + "keywords": [ + "clusterio-plugin" + ], "scripts": { "prepare": "tsc --build && webpack-cli --env production", "test": "echo \"Error: run tests from root\" && exit 1" diff --git a/plugins/player_auth/package.json b/plugins/player_auth/package.json index a7c40a5b..a05ba974 100644 --- a/plugins/player_auth/package.json +++ b/plugins/player_auth/package.json @@ -7,6 +7,9 @@ "license": "MIT", "repository": "clusterio/clusterio", "main": "dist/node/index.js", + "keywords": [ + "clusterio-plugin" + ], "scripts": { "prepare": "tsc --build && webpack-cli --env production", "test": "echo \"Error: run tests from root\" && exit 1" diff --git a/plugins/research_sync/package.json b/plugins/research_sync/package.json index 955874ae..9b553b1d 100644 --- a/plugins/research_sync/package.json +++ b/plugins/research_sync/package.json @@ -7,6 +7,9 @@ "license": "MIT", "repository": "clusterio/clusterio", "main": "dist/node/index.js", + "keywords": [ + "clusterio-plugin" + ], "scripts": { "prepare": "tsc --build && webpack-cli --env production", "test": "echo \"Error: run tests from root\" && exit 1" diff --git a/plugins/statistics_exporter/package.json b/plugins/statistics_exporter/package.json index cb4e9d0c..9737a2a6 100644 --- a/plugins/statistics_exporter/package.json +++ b/plugins/statistics_exporter/package.json @@ -7,6 +7,9 @@ "license": "MIT", "repository": "clusterio/clusterio", "main": "dist/node/index.js", + "keywords": [ + "clusterio-plugin" + ], "scripts": { "prepare": "tsc --build && webpack-cli --env production", "test": "echo \"Error: run tests from root\" && exit 1" diff --git a/plugins/subspace_storage/package.json b/plugins/subspace_storage/package.json index e6de5b57..2825680f 100644 --- a/plugins/subspace_storage/package.json +++ b/plugins/subspace_storage/package.json @@ -7,6 +7,9 @@ "license": "MIT", "repository": "clusterio/clusterio", "main": "dist/node/index.js", + "keywords": [ + "clusterio-plugin" + ], "scripts": { "prepare": "tsc --build && webpack-cli --env production", "test": "echo \"Error: run tests from root\" && exit 1" From 472177bf3909400e9f7f1d2f11eb47f5e963382a Mon Sep 17 00:00:00 2001 From: Danielv123 Date: Mon, 25 Nov 2024 17:55:08 +0000 Subject: [PATCH 03/16] Add jsdoc, reduce logging --- packages/lib/src/load_plugin_list.ts | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/packages/lib/src/load_plugin_list.ts b/packages/lib/src/load_plugin_list.ts index e0eb2b4f..55fe99cd 100644 --- a/packages/lib/src/load_plugin_list.ts +++ b/packages/lib/src/load_plugin_list.ts @@ -4,7 +4,12 @@ import { logger } from "./logging"; import * as libFileOps from "./file_ops"; /** - * Find local plugins to add to the plugin list. This is primarily used during development. + * Searches for local plugins in the plugins/ and external_plugins/ directories + * and adds them to the plugin list if they contain a package.json file. + * + * @param pluginList - Map of plugin names to their file paths + * @param pluginListPath - Path to the JSON file storing the plugin list + * @returns Promise that resolves when plugin discovery is complete */ async function findLocalPlugins(pluginList: Map, pluginListPath: string) { // Check folders for plugins @@ -31,11 +36,10 @@ async function findLocalPlugins(pluginList: Map, pluginListPath: if (!(await fs.exists(packageJsonPath))) { continue; } - console.log(`Adding ${pluginName} from ${pluginFolder}`); pluginList.set(pluginName, pluginPath); logger.info(`Added ${pluginName} from ${pluginFolder}`); - has_changed++; + has_changed += 1; } } @@ -46,15 +50,21 @@ async function findLocalPlugins(pluginList: Map, pluginListPath: /** * Find NPM packages in the root package.json that satisfies the requirements for plugins. + * + * @param pluginList - Map of plugin names to their file paths + * @param pluginListPath - Path to the JSON file storing the plugin list + * @returns Promise that resolves when plugin discovery is complete */ async function findNpmPlugins(pluginList: Map, pluginListPath: string) { - const rootPackageJson = JSON.parse(await fs.readFile(path.resolve(process.cwd(), "package.json"), { encoding: "utf8" })); + const rootPackageJson = JSON.parse(await fs.readFile(path.resolve( + process.cwd(), + "package.json", + ), { encoding: "utf8" })); const dependencies = rootPackageJson.dependencies; let changed = 0; for (const [pluginName, pluginVersion] of Object.entries(dependencies)) { if (pluginList.has(pluginName)) { - console.log(`${pluginName} already exists`); continue; } // Find the package in node_modules and read the package.json @@ -62,10 +72,11 @@ async function findNpmPlugins(pluginList: Map, pluginListPath: s const packageJson = JSON.parse(await fs.readFile(packageJsonPath, { encoding: "utf8" })); // Check if the package has the "clusterio-plugin" keyword if (packageJson.keywords?.includes("clusterio-plugin")) { + // eslint-disable-next-line @typescript-eslint/no-var-requires const pluginInfo = require(path.resolve(process.cwd(), "node_modules", pluginName)).plugin; pluginList.set(pluginInfo.name, pluginName); logger.info(`Added ${pluginInfo.name} from NPM`); - changed++; + changed += 1; } else { logger.info(`${pluginName}@${pluginVersion} is not a clusterio-plugin`); } From 91cbd7a464eb7e8fa70557475840e4216f922e02 Mon Sep 17 00:00:00 2001 From: Danielv123 Date: Mon, 25 Nov 2024 18:26:35 +0000 Subject: [PATCH 04/16] Handle files not existing --- packages/lib/src/load_plugin_list.ts | 35 ++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/packages/lib/src/load_plugin_list.ts b/packages/lib/src/load_plugin_list.ts index 55fe99cd..d21f2718 100644 --- a/packages/lib/src/load_plugin_list.ts +++ b/packages/lib/src/load_plugin_list.ts @@ -14,13 +14,22 @@ import * as libFileOps from "./file_ops"; async function findLocalPlugins(pluginList: Map, pluginListPath: string) { // Check folders for plugins const pluginFolders = [ - path.join(process.cwd(), "plugins"), - path.join(process.cwd(), "external_plugins"), + "plugins", + "external_plugins", ]; let has_changed = 0; for (const pluginFolder of pluginFolders) { - const pluginNames = await fs.readdir(pluginFolder); + let pluginNames; + try { + pluginNames = await fs.readdir(pluginFolder); + } catch (err: any) { + if (err.code === "ENOENT") { + // Skip if folder doesn't exist + continue; + } + throw err; + } for (const pluginName of pluginNames) { if (pluginList.has(pluginName)) { continue; @@ -56,11 +65,17 @@ async function findLocalPlugins(pluginList: Map, pluginListPath: * @returns Promise that resolves when plugin discovery is complete */ async function findNpmPlugins(pluginList: Map, pluginListPath: string) { - const rootPackageJson = JSON.parse(await fs.readFile(path.resolve( - process.cwd(), - "package.json", - ), { encoding: "utf8" })); - const dependencies = rootPackageJson.dependencies; + let dependencies; + try { + const rootPackageJson = JSON.parse(await fs.readFile(path.resolve("package.json"), { encoding: "utf8" })); + dependencies = rootPackageJson.dependencies; + } catch (err: any) { + if (err.code === "ENOENT") { + // Skip if package.json doesn't exist + return; + } + throw err; + } let changed = 0; for (const [pluginName, pluginVersion] of Object.entries(dependencies)) { @@ -68,12 +83,12 @@ async function findNpmPlugins(pluginList: Map, pluginListPath: s continue; } // Find the package in node_modules and read the package.json - const packageJsonPath = path.resolve(process.cwd(), "node_modules", pluginName, "package.json"); + const packageJsonPath = path.resolve("node_modules", pluginName, "package.json"); const packageJson = JSON.parse(await fs.readFile(packageJsonPath, { encoding: "utf8" })); // Check if the package has the "clusterio-plugin" keyword if (packageJson.keywords?.includes("clusterio-plugin")) { // eslint-disable-next-line @typescript-eslint/no-var-requires - const pluginInfo = require(path.resolve(process.cwd(), "node_modules", pluginName)).plugin; + const pluginInfo = require(path.resolve("node_modules", pluginName)).plugin; pluginList.set(pluginInfo.name, pluginName); logger.info(`Added ${pluginInfo.name} from NPM`); changed += 1; From 5586b1640b0dd305655f65660658f4837f4347a1 Mon Sep 17 00:00:00 2001 From: Danielv123 Date: Tue, 26 Nov 2024 08:17:45 +0000 Subject: [PATCH 05/16] Include keywords for clusterio and factorio in all first party packages --- packages/controller/package.json | 1 + packages/create/package.json | 4 ++-- packages/create/templates/common/package.json | 4 +--- packages/ctl/package.json | 1 + packages/host/package.json | 1 + packages/lib/package.json | 1 + packages/lib/src/load_plugin_list.ts | 2 +- packages/web_ui/package.json | 3 ++- plugins/global_chat/package.json | 4 +++- plugins/inventory_sync/package.json | 4 +++- plugins/player_auth/package.json | 4 +++- plugins/research_sync/package.json | 4 +++- plugins/statistics_exporter/package.json | 4 +++- plugins/subspace_storage/package.json | 4 +++- 14 files changed, 28 insertions(+), 13 deletions(-) diff --git a/packages/controller/package.json b/packages/controller/package.json index ccda8907..9aeb29bf 100644 --- a/packages/controller/package.json +++ b/packages/controller/package.json @@ -13,6 +13,7 @@ "clusteriocontroller": "./dist/node/controller.js" }, "keywords": [ + "clusterio", "factorio" ], "engines": { diff --git a/packages/create/package.json b/packages/create/package.json index 814e26f0..8fc99cb5 100644 --- a/packages/create/package.json +++ b/packages/create/package.json @@ -9,8 +9,8 @@ "clusteriocreate": "./create.js" }, "keywords": [ - "factorio", - "clusterio" + "clusterio", + "factorio" ], "engines": { "node": ">=18" diff --git a/packages/create/templates/common/package.json b/packages/create/templates/common/package.json index 36e5ad01..ada0a72b 100644 --- a/packages/create/templates/common/package.json +++ b/packages/create/templates/common/package.json @@ -8,9 +8,6 @@ //%if !typescript "main": "index.js", //%endif - "keywords": [ - "clusterio-plugin" - ], "scripts": { "prepare": "__prepare__" }, @@ -51,6 +48,7 @@ }, "keywords": [ "clusterio", + "clusterio-plugin", "factorio" ] } diff --git a/packages/ctl/package.json b/packages/ctl/package.json index 28c79023..e26219d3 100644 --- a/packages/ctl/package.json +++ b/packages/ctl/package.json @@ -13,6 +13,7 @@ "clusterioctl": "./dist/node/ctl.js" }, "keywords": [ + "clusterio", "factorio" ], "engines": { diff --git a/packages/host/package.json b/packages/host/package.json index ea44fdc9..f5bc92d8 100644 --- a/packages/host/package.json +++ b/packages/host/package.json @@ -13,6 +13,7 @@ "clusteriohost": "./dist/node/host.js" }, "keywords": [ + "clusterio", "factorio" ], "engines": { diff --git a/packages/lib/package.json b/packages/lib/package.json index d5c58364..0f0ed1b5 100644 --- a/packages/lib/package.json +++ b/packages/lib/package.json @@ -11,6 +11,7 @@ "main": "dist/node/index.js", "browser": "browser.ts", "keywords": [ + "clusterio", "factorio" ], "engines": { diff --git a/packages/lib/src/load_plugin_list.ts b/packages/lib/src/load_plugin_list.ts index d21f2718..8af16781 100644 --- a/packages/lib/src/load_plugin_list.ts +++ b/packages/lib/src/load_plugin_list.ts @@ -127,7 +127,7 @@ export async function loadPluginList(pluginListPath: string, findLocal = true): await findLocalPlugins(pluginList, pluginListPath); } - // Optionally discover NPM plugins + // Discover NPM plugins await findNpmPlugins(pluginList, pluginListPath); return pluginList; diff --git a/packages/web_ui/package.json b/packages/web_ui/package.json index f61a632f..1921c733 100644 --- a/packages/web_ui/package.json +++ b/packages/web_ui/package.json @@ -10,7 +10,8 @@ "test": "echo \"Error: no test specified\" && exit 1" }, "keywords": [ - "clusterio" + "clusterio", + "factorio" ], "engines": { "node": ">=12" diff --git a/plugins/global_chat/package.json b/plugins/global_chat/package.json index c20e1846..e4620e37 100644 --- a/plugins/global_chat/package.json +++ b/plugins/global_chat/package.json @@ -8,7 +8,9 @@ "repository": "clusterio/clusterio", "main": "dist/node/index.js", "keywords": [ - "clusterio-plugin" + "clusterio", + "clusterio-plugin", + "factorio" ], "scripts": { "prepare": "tsc --build && webpack-cli --env production", diff --git a/plugins/inventory_sync/package.json b/plugins/inventory_sync/package.json index 03f31ade..013b6c6e 100644 --- a/plugins/inventory_sync/package.json +++ b/plugins/inventory_sync/package.json @@ -8,7 +8,9 @@ "repository": "clusterio/factorioClusterio", "main": "dist/node/index.js", "keywords": [ - "clusterio-plugin" + "clusterio", + "clusterio-plugin", + "factorio" ], "scripts": { "prepare": "tsc --build && webpack-cli --env production", diff --git a/plugins/player_auth/package.json b/plugins/player_auth/package.json index a05ba974..8b40b929 100644 --- a/plugins/player_auth/package.json +++ b/plugins/player_auth/package.json @@ -8,7 +8,9 @@ "repository": "clusterio/clusterio", "main": "dist/node/index.js", "keywords": [ - "clusterio-plugin" + "clusterio", + "clusterio-plugin", + "factorio" ], "scripts": { "prepare": "tsc --build && webpack-cli --env production", diff --git a/plugins/research_sync/package.json b/plugins/research_sync/package.json index 9b553b1d..5cf8a269 100644 --- a/plugins/research_sync/package.json +++ b/plugins/research_sync/package.json @@ -8,7 +8,9 @@ "repository": "clusterio/clusterio", "main": "dist/node/index.js", "keywords": [ - "clusterio-plugin" + "clusterio", + "clusterio-plugin", + "factorio" ], "scripts": { "prepare": "tsc --build && webpack-cli --env production", diff --git a/plugins/statistics_exporter/package.json b/plugins/statistics_exporter/package.json index 9737a2a6..691c8c38 100644 --- a/plugins/statistics_exporter/package.json +++ b/plugins/statistics_exporter/package.json @@ -8,7 +8,9 @@ "repository": "clusterio/clusterio", "main": "dist/node/index.js", "keywords": [ - "clusterio-plugin" + "clusterio", + "clusterio-plugin", + "factorio" ], "scripts": { "prepare": "tsc --build && webpack-cli --env production", diff --git a/plugins/subspace_storage/package.json b/plugins/subspace_storage/package.json index 2825680f..ee517d3d 100644 --- a/plugins/subspace_storage/package.json +++ b/plugins/subspace_storage/package.json @@ -8,7 +8,9 @@ "repository": "clusterio/clusterio", "main": "dist/node/index.js", "keywords": [ - "clusterio-plugin" + "clusterio", + "clusterio-plugin", + "factorio" ], "scripts": { "prepare": "tsc --build && webpack-cli --env production", From f95e1d29983b0db6dc593b283a6b9ee902c50751 Mon Sep 17 00:00:00 2001 From: Danielv123 Date: Tue, 26 Nov 2024 08:27:29 +0000 Subject: [PATCH 06/16] Only log not-a-plugin for non-core packages --- packages/lib/src/load_plugin_list.ts | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/packages/lib/src/load_plugin_list.ts b/packages/lib/src/load_plugin_list.ts index 8af16781..3f2bd879 100644 --- a/packages/lib/src/load_plugin_list.ts +++ b/packages/lib/src/load_plugin_list.ts @@ -78,22 +78,28 @@ async function findNpmPlugins(pluginList: Map, pluginListPath: s } let changed = 0; - for (const [pluginName, pluginVersion] of Object.entries(dependencies)) { - if (pluginList.has(pluginName)) { + for (const [packageName, packageVersion] of Object.entries(dependencies)) { + if (pluginList.has(packageName)) { continue; } // Find the package in node_modules and read the package.json - const packageJsonPath = path.resolve("node_modules", pluginName, "package.json"); + const packageJsonPath = path.resolve("node_modules", packageName, "package.json"); const packageJson = JSON.parse(await fs.readFile(packageJsonPath, { encoding: "utf8" })); // Check if the package has the "clusterio-plugin" keyword if (packageJson.keywords?.includes("clusterio-plugin")) { // eslint-disable-next-line @typescript-eslint/no-var-requires - const pluginInfo = require(path.resolve("node_modules", pluginName)).plugin; - pluginList.set(pluginInfo.name, pluginName); + const pluginInfo = require(path.resolve("node_modules", packageName)).plugin; + pluginList.set(pluginInfo.name, packageName); logger.info(`Added ${pluginInfo.name} from NPM`); changed += 1; - } else { - logger.info(`${pluginName}@${pluginVersion} is not a clusterio-plugin`); + } else if (![ + "@clusterio/controller", + "@clusterio/ctl", + "@clusterio/host", + "@clusterio/lib", + "@clusterio/web_ui", + ].includes(packageName)) { + logger.warn(`${packageName}@${packageVersion} is not a clusterio-plugin`); } } if (changed > 0) { From 37fed2064e9e4713611a61ef4145563d41bd0087 Mon Sep 17 00:00:00 2001 From: Danielv123 Date: Tue, 26 Nov 2024 08:49:30 +0000 Subject: [PATCH 07/16] Warn when multiple plugins of the same name are detected --- packages/lib/src/load_plugin_list.ts | 33 ++++++++++++++++++++++------ packages/lib/src/plugin_loader.ts | 2 +- 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/packages/lib/src/load_plugin_list.ts b/packages/lib/src/load_plugin_list.ts index 3f2bd879..d0323ae4 100644 --- a/packages/lib/src/load_plugin_list.ts +++ b/packages/lib/src/load_plugin_list.ts @@ -2,6 +2,7 @@ import fs from "fs-extra"; import path from "path"; import { logger } from "./logging"; import * as libFileOps from "./file_ops"; +import vm from "vm"; /** * Searches for local plugins in the plugins/ and external_plugins/ directories @@ -31,10 +32,14 @@ async function findLocalPlugins(pluginList: Map, pluginListPath: throw err; } for (const pluginName of pluginNames) { + const pluginPath = path.resolve(pluginFolder, pluginName); if (pluginList.has(pluginName)) { + // If the one in the list is not the same as this one, warn + if (pluginList.get(pluginName) !== pluginPath) { + logger.warn(`${pluginName} is already in the plugin list, but with a different path - using ${pluginList.get(pluginName)}`); + } continue; } - const pluginPath = path.resolve(pluginFolder, pluginName); const stats = await fs.stat(pluginPath); if (!stats.isDirectory()) { continue; @@ -57,6 +62,13 @@ async function findLocalPlugins(pluginList: Map, pluginListPath: } } +function getPluginName(requireSpec: string) { + const context = vm.createContext({ require: require }); + const code = `const pluginInfo = require(${JSON.stringify(requireSpec)}).plugin;`; + vm.runInContext(code, context); + return context.pluginInfo.name; +} + /** * Find NPM packages in the root package.json that satisfies the requirements for plugins. * @@ -79,8 +91,8 @@ async function findNpmPlugins(pluginList: Map, pluginListPath: s let changed = 0; for (const [packageName, packageVersion] of Object.entries(dependencies)) { - if (pluginList.has(packageName)) { - continue; + if ([...pluginList.values()].includes(packageName)) { + continue; // This npm module is already in the plugin list } // Find the package in node_modules and read the package.json const packageJsonPath = path.resolve("node_modules", packageName, "package.json"); @@ -88,10 +100,17 @@ async function findNpmPlugins(pluginList: Map, pluginListPath: s // Check if the package has the "clusterio-plugin" keyword if (packageJson.keywords?.includes("clusterio-plugin")) { // eslint-disable-next-line @typescript-eslint/no-var-requires - const pluginInfo = require(path.resolve("node_modules", packageName)).plugin; - pluginList.set(pluginInfo.name, packageName); - logger.info(`Added ${pluginInfo.name} from NPM`); - changed += 1; + const pluginName = getPluginName(path.resolve("node_modules", packageName)); + if (pluginList.has(pluginName)) { + logger.warn( + `${pluginName} provided by ${packageName}@${packageVersion} is already in the plugin list, ` + + `but with a different path - using ${pluginList.get(pluginName)}` + ); + } else { + pluginList.set(pluginName, packageName); + logger.info(`Added ${pluginName} from NPM`); + changed += 1; + } } else if (![ "@clusterio/controller", "@clusterio/ctl", diff --git a/packages/lib/src/plugin_loader.ts b/packages/lib/src/plugin_loader.ts index 824ca94f..edd456b7 100644 --- a/packages/lib/src/plugin_loader.ts +++ b/packages/lib/src/plugin_loader.ts @@ -30,7 +30,7 @@ export async function loadPluginInfos(pluginList: Map) { try { require.resolve(pluginPath); } catch (err) { - logger.warn(`Plugin path ${pluginPath} does not exist, removing ${pluginName}`); + logger.warn(`Plugin path ${pluginPath} does not exist, not loading ${pluginName}`); pluginList.delete(pluginName); continue; } From 5dbfff9bfd99bb09302637906a9216a9fbae553a Mon Sep 17 00:00:00 2001 From: Danielv123 Date: Tue, 26 Nov 2024 09:23:09 +0000 Subject: [PATCH 08/16] Support for monorepo autodiscovery --- packages/lib/src/load_plugin_list.ts | 37 ++++++++++++++++++---------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/packages/lib/src/load_plugin_list.ts b/packages/lib/src/load_plugin_list.ts index d0323ae4..c3b97f97 100644 --- a/packages/lib/src/load_plugin_list.ts +++ b/packages/lib/src/load_plugin_list.ts @@ -20,6 +20,9 @@ async function findLocalPlugins(pluginList: Map, pluginListPath: ]; let has_changed = 0; + const ignoredFolders = new Set(["node_modules", ".git", "dist", "build", "disabled"]); + const maxDepth = 3; + for (const pluginFolder of pluginFolders) { let pluginNames; try { @@ -32,28 +35,36 @@ async function findLocalPlugins(pluginList: Map, pluginListPath: throw err; } for (const pluginName of pluginNames) { - const pluginPath = path.resolve(pluginFolder, pluginName); + const pluginPath = path.join(pluginFolder, pluginName); if (pluginList.has(pluginName)) { // If the one in the list is not the same as this one, warn - if (pluginList.get(pluginName) !== pluginPath) { + if (pluginList.get(pluginName) !== path.resolve(pluginPath)) { logger.warn(`${pluginName} is already in the plugin list, but with a different path - using ${pluginList.get(pluginName)}`); } continue; } const stats = await fs.stat(pluginPath); - if (!stats.isDirectory()) { - continue; - } + if (stats.isDirectory()) { + // Check for package.json in current directory + const packageJsonPath = path.join(pluginPath, "package.json"); + if (await fs.exists(packageJsonPath)) { + // Read and parse package.json + const packageJson = JSON.parse(await fs.readFile(packageJsonPath, { encoding: "utf8" })); - // Check that the plugin has a package.json file - const packageJsonPath = path.resolve(pluginPath, "package.json"); - if (!(await fs.exists(packageJsonPath))) { - continue; - } + // Check if package has the clusterio-plugin keyword + if (packageJson.keywords?.includes("clusterio-plugin")) { + pluginList.set(pluginName, path.resolve(pluginPath)); + logger.info(`Added ${pluginName} from ${pluginFolder}`); + has_changed += 1; + continue; + } + } - pluginList.set(pluginName, pluginPath); - logger.info(`Added ${pluginName} from ${pluginFolder}`); - has_changed += 1; + if (pluginPath.split(path.sep).length < maxDepth && !ignoredFolders.has(pluginName)) { + logger.warn(`Possible monorepo detected, scanning ${pluginPath} for clusterio-plugins`); + pluginFolders.push(pluginPath); + } + } } } From fea07e4f5502bce0a692571a284715ae72c800eb Mon Sep 17 00:00:00 2001 From: Danielv123 Date: Tue, 26 Nov 2024 10:03:35 +0000 Subject: [PATCH 09/16] Add tests --- packages/lib/src/load_plugin_list.ts | 4 +- test/lib/plugin_loader.js | 77 ++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 2 deletions(-) diff --git a/packages/lib/src/load_plugin_list.ts b/packages/lib/src/load_plugin_list.ts index c3b97f97..29279a64 100644 --- a/packages/lib/src/load_plugin_list.ts +++ b/packages/lib/src/load_plugin_list.ts @@ -74,8 +74,8 @@ async function findLocalPlugins(pluginList: Map, pluginListPath: } function getPluginName(requireSpec: string) { - const context = vm.createContext({ require: require }); - const code = `const pluginInfo = require(${JSON.stringify(requireSpec)}).plugin;`; + const context = vm.createContext({ require: require, pluginInfo: null }); + const code = `pluginInfo = require(${JSON.stringify(requireSpec)}).plugin;`; vm.runInContext(code, context); return context.pluginInfo.name; } diff --git a/test/lib/plugin_loader.js b/test/lib/plugin_loader.js index b8f86ea4..551a770a 100644 --- a/test/lib/plugin_loader.js +++ b/test/lib/plugin_loader.js @@ -112,4 +112,81 @@ describe("lib/plugin_loader", function() { ); }); }); + describe("loadPluginList()", async function() { + const old_cwd = process.cwd(); + const baseDir = path.join("temp", "test", "plugin_list"); + const pluginListPath = path.join(baseDir, "plugin_list.json"); + const localPluginPath = path.join(baseDir, "plugins", "local_plugin"); + 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"); + let pluginList; + + before(async function() { + // Setup test plugins + async function writePlugin(pluginPath, name) { + await fs.outputFile( + path.join(pluginPath, "index.js"), + `module.exports.plugin = { name: "${name}" };` + ); + await fs.outputFile( + path.join(pluginPath, "package.json"), + JSON.stringify({ + name: path.basename(pluginPath), + version: "0.0.1", + keywords: ["clusterio-plugin"] + }) + ); + } + + // Create local plugin + await writePlugin(localPluginPath, "local_plugin"); + // Create external plugin + await writePlugin(externalPluginPath, "external_plugin"); + // Create npm plugin + await writePlugin(npmPluginPath, "npm"); + // Create root package.json + await fs.outputFile( + path.join(baseDir, "package.json"), + JSON.stringify({ + dependencies: { + "npm-plugin": "^1.0.0" + } + }) + ); + + process.chdir(baseDir); + pluginList = await lib.loadPluginList(pluginListPath, true); + }); + beforeEach(async function() { + await fs.remove(pluginListPath); + }); + + it("should discover local plugins", async function() { + assert.ok(pluginList.has("local_plugin")); + assert.ok(pluginList.has("external_plugin")); + assert.strictEqual(pluginList.get("local_plugin"), localPluginPathAbs); + assert.strictEqual(pluginList.get("external_plugin"), externalPluginPathAbs); + }); + + it("should discover npm plugins", async function() { + assert.ok(pluginList.has("npm")); + assert.strictEqual(pluginList.get("npm"), "npm-plugin"); + }); + + it("should load existing plugin list", async function() { + const existingList = new Map([["test", "/test/path"]]); + await fs.outputFile(pluginListPath, JSON.stringify([...existingList])); + const pluginList = await lib.loadPluginList(pluginListPath, false); + assert.ok(pluginList.has("test")); + assert.strictEqual(pluginList.get("test"), "/test/path"); + }); + + after(async function() { + process.chdir(old_cwd); + console.log(baseDir, process.cwd()); + await fs.remove(baseDir); + }); + }); }); From 79c4bf144135fac39755c5a6b677a99296ee0593 Mon Sep 17 00:00:00 2001 From: Danielv123 Date: Tue, 26 Nov 2024 10:07:14 +0000 Subject: [PATCH 10/16] Ignore missing plugins instead of throwing --- test/lib/plugin_loader.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/test/lib/plugin_loader.js b/test/lib/plugin_loader.js index 551a770a..22131f5d 100644 --- a/test/lib/plugin_loader.js +++ b/test/lib/plugin_loader.js @@ -33,11 +33,9 @@ describe("lib/plugin_loader", function() { await writePlugin(invalidPlugin, "wrong"); }); - it("should throw on missing plugin", async function() { - await assert.rejects( - lib.loadPluginInfos(new Map([["missing", missingPlugin]]), []), - new RegExp(`^Error: PluginError: Cannot find module '${escapeRegExp(missingPlugin)}'`) - ); + it("should ignore missing plugins", async function() { + const result = await lib.loadPluginInfos(new Map([["missing", missingPlugin]]), []); + assert.deepEqual(result, []); }); it("should load test plugin", async function() { assert.deepEqual( From 427acfe82ebfb66c21e5c2f61f1d352993ac7736 Mon Sep 17 00:00:00 2001 From: Danielv123 Date: Tue, 26 Nov 2024 10:13:36 +0000 Subject: [PATCH 11/16] Refactor, fix linting issues --- packages/lib/src/load_plugin_list.ts | 42 ++++++++++++++++++---------- test/lib/plugin_loader.js | 13 ++++----- 2 files changed, 34 insertions(+), 21 deletions(-) diff --git a/packages/lib/src/load_plugin_list.ts b/packages/lib/src/load_plugin_list.ts index 29279a64..826a667d 100644 --- a/packages/lib/src/load_plugin_list.ts +++ b/packages/lib/src/load_plugin_list.ts @@ -39,25 +39,21 @@ async function findLocalPlugins(pluginList: Map, pluginListPath: if (pluginList.has(pluginName)) { // If the one in the list is not the same as this one, warn if (pluginList.get(pluginName) !== path.resolve(pluginPath)) { - logger.warn(`${pluginName} is already in the plugin list, but with a different path - using ${pluginList.get(pluginName)}`); + logger.warn( + `${pluginName} is already in the plugin list, but with a different path - ` + + `using ${pluginList.get(pluginName)}` + ); } continue; } const stats = await fs.stat(pluginPath); if (stats.isDirectory()) { - // Check for package.json in current directory - const packageJsonPath = path.join(pluginPath, "package.json"); - if (await fs.exists(packageJsonPath)) { - // Read and parse package.json - const packageJson = JSON.parse(await fs.readFile(packageJsonPath, { encoding: "utf8" })); - - // Check if package has the clusterio-plugin keyword - if (packageJson.keywords?.includes("clusterio-plugin")) { - pluginList.set(pluginName, path.resolve(pluginPath)); - logger.info(`Added ${pluginName} from ${pluginFolder}`); - has_changed += 1; - continue; - } + // Check for package.json and plugin keyword + if (await checkPackageJson(pluginPath)) { + pluginList.set(pluginName, path.resolve(pluginPath)); + logger.info(`Added ${pluginName} from ${pluginFolder}`); + has_changed += 1; + continue; } if (pluginPath.split(path.sep).length < maxDepth && !ignoredFolders.has(pluginName)) { @@ -168,3 +164,21 @@ export async function loadPluginList(pluginListPath: string, findLocal = true): return pluginList; } + +/** + * Helper function to check if a package.json file exists and contains the clusterio-plugin keyword + * @param pluginPath - Path to the plugin directory + * @returns Promise that resolves to true if package.json exists and has clusterio-plugin keyword + */ +async function checkPackageJson(pluginPath: string): Promise { + const packageJsonPath = path.join(pluginPath, "package.json"); + if (!await fs.exists(packageJsonPath)) { + return false; + } + + // Read and parse package.json + const packageJson = JSON.parse(await fs.readFile(packageJsonPath, { encoding: "utf8" })); + + // Check if package has the clusterio-plugin keyword + return packageJson.keywords?.includes("clusterio-plugin"); +} diff --git a/test/lib/plugin_loader.js b/test/lib/plugin_loader.js index 22131f5d..8cca497a 100644 --- a/test/lib/plugin_loader.js +++ b/test/lib/plugin_loader.js @@ -133,7 +133,7 @@ describe("lib/plugin_loader", function() { JSON.stringify({ name: path.basename(pluginPath), version: "0.0.1", - keywords: ["clusterio-plugin"] + keywords: ["clusterio-plugin"], }) ); } @@ -149,8 +149,8 @@ describe("lib/plugin_loader", function() { path.join(baseDir, "package.json"), JSON.stringify({ dependencies: { - "npm-plugin": "^1.0.0" - } + "npm-plugin": "^1.0.0", + }, }) ); @@ -176,14 +176,13 @@ describe("lib/plugin_loader", function() { it("should load existing plugin list", async function() { const existingList = new Map([["test", "/test/path"]]); await fs.outputFile(pluginListPath, JSON.stringify([...existingList])); - const pluginList = await lib.loadPluginList(pluginListPath, false); - assert.ok(pluginList.has("test")); - assert.strictEqual(pluginList.get("test"), "/test/path"); + const loadedPlugins = await lib.loadPluginList(pluginListPath, false); + assert.ok(loadedPlugins.has("test")); + assert.strictEqual(loadedPlugins.get("test"), "/test/path"); }); after(async function() { process.chdir(old_cwd); - console.log(baseDir, process.cwd()); await fs.remove(baseDir); }); }); From 2e729b9e4765670b487b95afdbedc5ac00f2c3ed Mon Sep 17 00:00:00 2001 From: Danielv123 Date: Tue, 26 Nov 2024 10:33:26 +0000 Subject: [PATCH 12/16] Add tests for monorepo plugins and non-plugin packages --- test/lib/plugin_loader.js | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/test/lib/plugin_loader.js b/test/lib/plugin_loader.js index 8cca497a..0173e85a 100644 --- a/test/lib/plugin_loader.js +++ b/test/lib/plugin_loader.js @@ -119,6 +119,8 @@ describe("lib/plugin_loader", function() { 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 monorepoPluginPathAbs = path.resolve(monorepoPluginPath); let pluginList; before(async function() { @@ -144,15 +146,23 @@ describe("lib/plugin_loader", function() { await writePlugin(externalPluginPath, "external_plugin"); // Create npm plugin await writePlugin(npmPluginPath, "npm"); + // Write a monorepo plugin + await writePlugin(monorepoPluginPath, "monorepo-plugin"); // Create root package.json await fs.outputFile( path.join(baseDir, "package.json"), JSON.stringify({ dependencies: { "npm-plugin": "^1.0.0", + "not-a-plugin": "^1.0.0", }, }) ); + // Create an npm module that is not a plugin + await fs.outputFile( + path.join(baseDir, "node_modules", "not-a-plugin", "package.json"), + JSON.stringify({ name: "not-a-plugin", version: "1.0.0" }) + ); process.chdir(baseDir); pluginList = await lib.loadPluginList(pluginListPath, true); @@ -171,6 +181,8 @@ 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("not-a-plugin"), undefined); }); it("should load existing plugin list", async function() { @@ -181,6 +193,11 @@ describe("lib/plugin_loader", function() { assert.strictEqual(loadedPlugins.get("test"), "/test/path"); }); + it("should support monorepo plugins", async function() { + assert.ok(pluginList.has("monorepo-plugin")); + assert.strictEqual(pluginList.get("monorepo-plugin"), monorepoPluginPathAbs); + }); + after(async function() { process.chdir(old_cwd); await fs.remove(baseDir); From 9b0a06815bbd8d92b0d38176d7660641680af822 Mon Sep 17 00:00:00 2001 From: Danielv123 Date: Thu, 28 Nov 2024 20:27:31 +0000 Subject: [PATCH 13/16] Correctly handle installs with no plugins or dependencies --- packages/lib/src/load_plugin_list.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/lib/src/load_plugin_list.ts b/packages/lib/src/load_plugin_list.ts index 826a667d..2e62dda3 100644 --- a/packages/lib/src/load_plugin_list.ts +++ b/packages/lib/src/load_plugin_list.ts @@ -96,17 +96,17 @@ async function findNpmPlugins(pluginList: Map, pluginListPath: s throw err; } + if (!dependencies) { + return; + } + let changed = 0; for (const [packageName, packageVersion] of Object.entries(dependencies)) { if ([...pluginList.values()].includes(packageName)) { continue; // This npm module is already in the plugin list } - // Find the package in node_modules and read the package.json - const packageJsonPath = path.resolve("node_modules", packageName, "package.json"); - const packageJson = JSON.parse(await fs.readFile(packageJsonPath, { encoding: "utf8" })); - // Check if the package has the "clusterio-plugin" keyword - if (packageJson.keywords?.includes("clusterio-plugin")) { - // eslint-disable-next-line @typescript-eslint/no-var-requires + // 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)); if (pluginList.has(pluginName)) { logger.warn( From 747c13a7153f6d6edf98d181087924f8993ffc07 Mon Sep 17 00:00:00 2001 From: Danielv123 Date: Thu, 28 Nov 2024 20:43:58 +0000 Subject: [PATCH 14/16] Only write changed plugin list once --- packages/lib/src/load_plugin_list.ts | 42 +++++++++++++++------------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/packages/lib/src/load_plugin_list.ts b/packages/lib/src/load_plugin_list.ts index 2e62dda3..22d3fbaa 100644 --- a/packages/lib/src/load_plugin_list.ts +++ b/packages/lib/src/load_plugin_list.ts @@ -9,17 +9,16 @@ import vm from "vm"; * and adds them to the plugin list if they contain a package.json file. * * @param pluginList - Map of plugin names to their file paths - * @param pluginListPath - Path to the JSON file storing the plugin list - * @returns Promise that resolves when plugin discovery is complete + * @returns Promise that resolves to true if any plugins were added to the list, false otherwise */ -async function findLocalPlugins(pluginList: Map, pluginListPath: string) { +async function findLocalPlugins(pluginList: Map): Promise { // Check folders for plugins const pluginFolders = [ "plugins", "external_plugins", ]; - let has_changed = 0; + let hasChanged = 0; const ignoredFolders = new Set(["node_modules", ".git", "dist", "build", "disabled"]); const maxDepth = 3; @@ -52,7 +51,7 @@ async function findLocalPlugins(pluginList: Map, pluginListPath: if (await checkPackageJson(pluginPath)) { pluginList.set(pluginName, path.resolve(pluginPath)); logger.info(`Added ${pluginName} from ${pluginFolder}`); - has_changed += 1; + hasChanged += 1; continue; } @@ -64,9 +63,7 @@ async function findLocalPlugins(pluginList: Map, pluginListPath: } } - if (has_changed > 0) { - await libFileOps.safeOutputFile(pluginListPath, JSON.stringify([...pluginList], null, "\t")); - } + return hasChanged > 0; } function getPluginName(requireSpec: string) { @@ -78,12 +75,12 @@ function getPluginName(requireSpec: string) { /** * Find NPM packages in the root package.json that satisfies the requirements for plugins. + * Adds discovered plugins to the plugin list. * * @param pluginList - Map of plugin names to their file paths - * @param pluginListPath - Path to the JSON file storing the plugin list - * @returns Promise that resolves when plugin discovery is complete + * @returns Promise that resolves to true if any plugins were added to the list, false otherwise */ -async function findNpmPlugins(pluginList: Map, pluginListPath: string) { +async function findNpmPlugins(pluginList: Map): Promise { let dependencies; try { const rootPackageJson = JSON.parse(await fs.readFile(path.resolve("package.json"), { encoding: "utf8" })); @@ -91,16 +88,16 @@ async function findNpmPlugins(pluginList: Map, pluginListPath: s } catch (err: any) { if (err.code === "ENOENT") { // Skip if package.json doesn't exist - return; + return false; } throw err; } if (!dependencies) { - return; + return false; } - let changed = 0; + 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 @@ -116,7 +113,7 @@ async function findNpmPlugins(pluginList: Map, pluginListPath: s } else { pluginList.set(pluginName, packageName); logger.info(`Added ${pluginName} from NPM`); - changed += 1; + hasChanged += 1; } } else if (![ "@clusterio/controller", @@ -128,9 +125,7 @@ async function findNpmPlugins(pluginList: Map, pluginListPath: s logger.warn(`${packageName}@${packageVersion} is not a clusterio-plugin`); } } - if (changed > 0) { - await libFileOps.safeOutputFile(pluginListPath, JSON.stringify([...pluginList], null, "\t")); - } + return hasChanged > 0; } /** @@ -154,13 +149,20 @@ export async function loadPluginList(pluginListPath: string, findLocal = true): logger.info(`No existing plugin list found at ${pluginListPath}`); } + // Track if any changes were made + let hasChanges = false; + // Optionally discover local plugins if (findLocal) { - await findLocalPlugins(pluginList, pluginListPath); + hasChanges = await findLocalPlugins(pluginList) || hasChanges; } // Discover NPM plugins - await findNpmPlugins(pluginList, pluginListPath); + hasChanges = await findNpmPlugins(pluginList) || hasChanges; + + if (hasChanges) { + await libFileOps.safeOutputFile(pluginListPath, JSON.stringify([...pluginList], null, "\t")); + } return pluginList; } From d7792ca369a5e655874871cf4c8e912a89c01cf2 Mon Sep 17 00:00:00 2001 From: Danielv123 Date: Fri, 29 Nov 2024 20:01:45 +0000 Subject: [PATCH 15/16] Log error when a specified plugin does not exist, but allow loading without it Preventing loading just because a plugin is missing would cause the host to become inaccessible if something went wrong when managing plugins through the remote interface --- packages/lib/src/plugin_loader.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/lib/src/plugin_loader.ts b/packages/lib/src/plugin_loader.ts index edd456b7..fa5307cc 100644 --- a/packages/lib/src/plugin_loader.ts +++ b/packages/lib/src/plugin_loader.ts @@ -30,7 +30,7 @@ export async function loadPluginInfos(pluginList: Map) { try { require.resolve(pluginPath); } catch (err) { - logger.warn(`Plugin path ${pluginPath} does not exist, not loading ${pluginName}`); + logger.error(`Plugin path ${pluginPath} does not exist, not loading ${pluginName}`); pluginList.delete(pluginName); continue; } From 17133dc1f52bf27e5026eb0cac56eeeffef79fc7 Mon Sep 17 00:00:00 2001 From: Danielv123 Date: Fri, 29 Nov 2024 20:08:13 +0000 Subject: [PATCH 16/16] Test for compatability with missing dependencies field in package.json --- test/lib/plugin_loader.js | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/test/lib/plugin_loader.js b/test/lib/plugin_loader.js index 0173e85a..fd5126bb 100644 --- a/test/lib/plugin_loader.js +++ b/test/lib/plugin_loader.js @@ -198,6 +198,22 @@ describe("lib/plugin_loader", function() { assert.strictEqual(pluginList.get("monorepo-plugin"), monorepoPluginPathAbs); }); + it("should not throw when package.json has no dependencies field", async function() { + // Create a package.json without dependencies + await fs.outputFile( + path.join("package.json"), + JSON.stringify({ + name: "test-package", + version: "1.0.0", + }) + ); + + // Should not throw + await assert.doesNotReject(async () => { + await lib.loadPluginList(pluginListPath, true); + }); + }); + after(async function() { process.chdir(old_cwd); await fs.remove(baseDir);