Fix review comments

Signed-off-by: David Festal <dfestal@redhat.com>

package.json

@@ -93,7 +93,6 @@
     "jest-haste-map@^29.7.0": "patch:jest-haste-map@npm%3A29.7.0#./.yarn/patches/jest-haste-map-npm-29.7.0-e3be419eff.patch"
   },
   "dependencies": {
-    "@backstage/cli-node": "workspace:^",
     "@backstage/errors": "workspace:^",
     "@manypkg/get-packages": "^1.1.3",
     "@types/global-agent": "^2.1.3",

packages/backend-dynamic-feature-service/README.md

@@ -15,7 +15,7 @@ In the `backend` application, it can be enabled by adding the `backend-dynamic-f
 ```ts
 const backend = createBackend();
 +
-+ backend.add(dynamicPluginsFeatureLoader) which provides features loaded by dynamic plugins
++ backend.add(dynamicPluginsFeatureLoader) // provides features loaded by dynamic plugins
 +
 ```
 

packages/backend-dynamic-feature-service/report.api.md

@@ -51,7 +51,7 @@ export type BackendDynamicPluginInstaller =
 // @public (undocumented)
 export interface BackendPluginProvider {
   // (undocumented)
-  backendPlugins(includeFailed?: boolean): BackendDynamicPlugin[];
+  backendPlugins(options?: { includeFailed?: boolean }): BackendDynamicPlugin[];
 }
 
 // @public (undocumented)
@@ -78,17 +78,19 @@ export class DynamicPluginManager implements DynamicPluginProvider {
   // (undocumented)
   get availablePackages(): ScannedPluginPackage[];
   // (undocumented)
-  backendPlugins(includeFailed?: boolean): BackendDynamicPlugin[];
+  backendPlugins(options?: { includeFailed?: boolean }): BackendDynamicPlugin[];
   // (undocumented)
   static create(
     options: DynamicPluginManagerOptions,
   ): Promise<DynamicPluginManager>;
   // (undocumented)
-  frontendPlugins(includeFailed?: boolean): FrontendDynamicPlugin[];
+  frontendPlugins(options?: {
+    includeFailed?: boolean;
+  }): FrontendDynamicPlugin[];
   // (undocumented)
   getScannedPackage(plugin: DynamicPlugin): ScannedPluginPackage;
   // (undocumented)
-  plugins(includeFailed?: boolean): DynamicPlugin[];
+  plugins(options?: { includeFailed?: boolean }): DynamicPlugin[];
 }
 
 // @public (undocumented)
@@ -110,7 +112,7 @@ export interface DynamicPluginProvider
   // (undocumented)
   getScannedPackage(plugin: DynamicPlugin): ScannedPluginPackage;
   // (undocumented)
-  plugins(includeFailed?: boolean): DynamicPlugin[];
+  plugins(options?: { includeFailed?: boolean }): DynamicPlugin[];
 }
 
 // @public (undocumented)
@@ -201,7 +203,9 @@ export interface FrontendDynamicPlugin extends BaseDynamicPlugin {
 // @public (undocumented)
 export interface FrontendPluginProvider {
   // (undocumented)
-  frontendPlugins(includeFailed?: boolean): FrontendDynamicPlugin[];
+  frontendPlugins(options?: {
+    includeFailed?: boolean;
+  }): FrontendDynamicPlugin[];
 }
 
 // @public (undocumented)
@@ -305,49 +309,49 @@ export interface ScannedPluginPackage {
 // src/manager/plugin-manager.d.ts:27:5 - (ae-undocumented) Missing documentation for "availablePackages".
 // src/manager/plugin-manager.d.ts:28:5 - (ae-undocumented) Missing documentation for "addBackendPlugin".
 // src/manager/plugin-manager.d.ts:31:5 - (ae-undocumented) Missing documentation for "backendPlugins".
-// src/manager/plugin-manager.d.ts:32:5 - (ae-undocumented) Missing documentation for "frontendPlugins".
-// src/manager/plugin-manager.d.ts:33:5 - (ae-undocumented) Missing documentation for "plugins".
-// src/manager/plugin-manager.d.ts:34:5 - (ae-undocumented) Missing documentation for "getScannedPackage".
-// src/manager/plugin-manager.d.ts:39:22 - (ae-undocumented) Missing documentation for "dynamicPluginsServiceRef".
-// src/manager/plugin-manager.d.ts:43:1 - (ae-undocumented) Missing documentation for "DynamicPluginsFactoryOptions".
-// src/manager/plugin-manager.d.ts:44:5 - (ae-undocumented) Missing documentation for "moduleLoader".
-// src/manager/plugin-manager.d.ts:50:22 - (ae-undocumented) Missing documentation for "dynamicPluginsServiceFactoryWithOptions".
-// src/manager/plugin-manager.d.ts:55:22 - (ae-undocumented) Missing documentation for "dynamicPluginsServiceFactory".
-// src/manager/plugin-manager.d.ts:60:22 - (ae-undocumented) Missing documentation for "dynamicPluginsFeatureDiscoveryServiceFactory".
-// src/manager/plugin-manager.d.ts:65:22 - (ae-undocumented) Missing documentation for "dynamicPluginsFeatureDiscoveryLoader".
+// src/manager/plugin-manager.d.ts:34:5 - (ae-undocumented) Missing documentation for "frontendPlugins".
+// src/manager/plugin-manager.d.ts:37:5 - (ae-undocumented) Missing documentation for "plugins".
+// src/manager/plugin-manager.d.ts:40:5 - (ae-undocumented) Missing documentation for "getScannedPackage".
+// src/manager/plugin-manager.d.ts:45:22 - (ae-undocumented) Missing documentation for "dynamicPluginsServiceRef".
+// src/manager/plugin-manager.d.ts:49:1 - (ae-undocumented) Missing documentation for "DynamicPluginsFactoryOptions".
+// src/manager/plugin-manager.d.ts:50:5 - (ae-undocumented) Missing documentation for "moduleLoader".
+// src/manager/plugin-manager.d.ts:56:22 - (ae-undocumented) Missing documentation for "dynamicPluginsServiceFactoryWithOptions".
+// src/manager/plugin-manager.d.ts:61:22 - (ae-undocumented) Missing documentation for "dynamicPluginsServiceFactory".
+// src/manager/plugin-manager.d.ts:66:22 - (ae-undocumented) Missing documentation for "dynamicPluginsFeatureDiscoveryServiceFactory".
+// src/manager/plugin-manager.d.ts:71:22 - (ae-undocumented) Missing documentation for "dynamicPluginsFeatureDiscoveryLoader".
 // src/manager/types.d.ts:28:1 - (ae-undocumented) Missing documentation for "LegacyPluginEnvironment".
 // src/manager/types.d.ts:46:1 - (ae-undocumented) Missing documentation for "DynamicPluginProvider".
 // src/manager/types.d.ts:47:5 - (ae-undocumented) Missing documentation for "plugins".
-// src/manager/types.d.ts:48:5 - (ae-undocumented) Missing documentation for "getScannedPackage".
-// src/manager/types.d.ts:53:1 - (ae-undocumented) Missing documentation for "BackendPluginProvider".
-// src/manager/types.d.ts:54:5 - (ae-undocumented) Missing documentation for "backendPlugins".
-// src/manager/types.d.ts:59:1 - (ae-undocumented) Missing documentation for "FrontendPluginProvider".
-// src/manager/types.d.ts:60:5 - (ae-undocumented) Missing documentation for "frontendPlugins".
-// src/manager/types.d.ts:65:1 - (ae-undocumented) Missing documentation for "BaseDynamicPlugin".
-// src/manager/types.d.ts:66:5 - (ae-undocumented) Missing documentation for "name".
-// src/manager/types.d.ts:67:5 - (ae-undocumented) Missing documentation for "version".
-// src/manager/types.d.ts:68:5 - (ae-undocumented) Missing documentation for "role".
-// src/manager/types.d.ts:69:5 - (ae-undocumented) Missing documentation for "platform".
-// src/manager/types.d.ts:70:5 - (ae-undocumented) Missing documentation for "failure".
-// src/manager/types.d.ts:75:1 - (ae-undocumented) Missing documentation for "DynamicPlugin".
-// src/manager/types.d.ts:79:1 - (ae-undocumented) Missing documentation for "FrontendDynamicPlugin".
-// src/manager/types.d.ts:80:5 - (ae-undocumented) Missing documentation for "platform".
-// src/manager/types.d.ts:85:1 - (ae-undocumented) Missing documentation for "BackendDynamicPlugin".
+// src/manager/types.d.ts:50:5 - (ae-undocumented) Missing documentation for "getScannedPackage".
+// src/manager/types.d.ts:55:1 - (ae-undocumented) Missing documentation for "BackendPluginProvider".
+// src/manager/types.d.ts:56:5 - (ae-undocumented) Missing documentation for "backendPlugins".
+// src/manager/types.d.ts:63:1 - (ae-undocumented) Missing documentation for "FrontendPluginProvider".
+// src/manager/types.d.ts:64:5 - (ae-undocumented) Missing documentation for "frontendPlugins".
+// src/manager/types.d.ts:71:1 - (ae-undocumented) Missing documentation for "BaseDynamicPlugin".
+// src/manager/types.d.ts:72:5 - (ae-undocumented) Missing documentation for "name".
+// src/manager/types.d.ts:73:5 - (ae-undocumented) Missing documentation for "version".
+// src/manager/types.d.ts:74:5 - (ae-undocumented) Missing documentation for "role".
+// src/manager/types.d.ts:75:5 - (ae-undocumented) Missing documentation for "platform".
+// src/manager/types.d.ts:76:5 - (ae-undocumented) Missing documentation for "failure".
+// src/manager/types.d.ts:81:1 - (ae-undocumented) Missing documentation for "DynamicPlugin".
+// src/manager/types.d.ts:85:1 - (ae-undocumented) Missing documentation for "FrontendDynamicPlugin".
 // src/manager/types.d.ts:86:5 - (ae-undocumented) Missing documentation for "platform".
-// src/manager/types.d.ts:87:5 - (ae-undocumented) Missing documentation for "installer".
-// src/manager/types.d.ts:92:1 - (ae-undocumented) Missing documentation for "BackendDynamicPluginInstaller".
-// src/manager/types.d.ts:96:1 - (ae-undocumented) Missing documentation for "NewBackendPluginInstaller".
-// src/manager/types.d.ts:97:5 - (ae-undocumented) Missing documentation for "kind".
-// src/manager/types.d.ts:98:5 - (ae-undocumented) Missing documentation for "install".
-// src/manager/types.d.ts:111:1 - (ae-undocumented) Missing documentation for "LegacyBackendPluginInstaller".
-// src/manager/types.d.ts:112:5 - (ae-undocumented) Missing documentation for "kind".
-// src/manager/types.d.ts:113:5 - (ae-undocumented) Missing documentation for "router".
-// src/manager/types.d.ts:117:5 - (ae-undocumented) Missing documentation for "catalog".
-// src/manager/types.d.ts:118:5 - (ae-undocumented) Missing documentation for "scaffolder".
-// src/manager/types.d.ts:119:5 - (ae-undocumented) Missing documentation for "search".
-// src/manager/types.d.ts:120:5 - (ae-undocumented) Missing documentation for "events".
-// src/manager/types.d.ts:121:5 - (ae-undocumented) Missing documentation for "permissions".
-// src/manager/types.d.ts:128:1 - (ae-undocumented) Missing documentation for "isBackendDynamicPluginInstaller".
+// src/manager/types.d.ts:91:1 - (ae-undocumented) Missing documentation for "BackendDynamicPlugin".
+// src/manager/types.d.ts:92:5 - (ae-undocumented) Missing documentation for "platform".
+// src/manager/types.d.ts:93:5 - (ae-undocumented) Missing documentation for "installer".
+// src/manager/types.d.ts:98:1 - (ae-undocumented) Missing documentation for "BackendDynamicPluginInstaller".
+// src/manager/types.d.ts:102:1 - (ae-undocumented) Missing documentation for "NewBackendPluginInstaller".
+// src/manager/types.d.ts:103:5 - (ae-undocumented) Missing documentation for "kind".
+// src/manager/types.d.ts:104:5 - (ae-undocumented) Missing documentation for "install".
+// src/manager/types.d.ts:117:1 - (ae-undocumented) Missing documentation for "LegacyBackendPluginInstaller".
+// src/manager/types.d.ts:118:5 - (ae-undocumented) Missing documentation for "kind".
+// src/manager/types.d.ts:119:5 - (ae-undocumented) Missing documentation for "router".
+// src/manager/types.d.ts:123:5 - (ae-undocumented) Missing documentation for "catalog".
+// src/manager/types.d.ts:124:5 - (ae-undocumented) Missing documentation for "scaffolder".
+// src/manager/types.d.ts:125:5 - (ae-undocumented) Missing documentation for "search".
+// src/manager/types.d.ts:126:5 - (ae-undocumented) Missing documentation for "events".
+// src/manager/types.d.ts:127:5 - (ae-undocumented) Missing documentation for "permissions".
+// src/manager/types.d.ts:134:1 - (ae-undocumented) Missing documentation for "isBackendDynamicPluginInstaller".
 // src/scanner/types.d.ts:5:1 - (ae-undocumented) Missing documentation for "ScannedPluginPackage".
 // src/scanner/types.d.ts:6:5 - (ae-undocumented) Missing documentation for "location".
 // src/scanner/types.d.ts:7:5 - (ae-undocumented) Missing documentation for "manifest".

packages/backend-dynamic-feature-service/src/features/features.test.ts

@@ -75,7 +75,9 @@ class DynamicPluginLister {
             dynamicPlugins: dynamicPluginsServiceRef,
           },
           async init({ dynamicPlugins }) {
-            that.loadedPlugins.push(...dynamicPlugins.plugins(true));
+            that.loadedPlugins.push(
+              ...dynamicPlugins.plugins({ includeFailed: true }),
+            );
           },
         });
       },

packages/backend-dynamic-feature-service/src/manager/plugin-manager.test.ts

@@ -682,7 +682,7 @@ describe('backend-dynamic-feature-service', () => {
           version: '0.0.0',
         },
       ]);
-      expect(pluginManager.backendPlugins(false)).toEqual([
+      expect(pluginManager.backendPlugins({ includeFailed: false })).toEqual([
         {
           name: 'a-backend-plugin',
           platform: 'node',
@@ -696,7 +696,7 @@ describe('backend-dynamic-feature-service', () => {
           version: '0.0.0',
         },
       ]);
-      expect(pluginManager.backendPlugins(true)).toEqual([
+      expect(pluginManager.backendPlugins({ includeFailed: true })).toEqual([
         {
           name: 'a-backend-plugin',
           platform: 'node',
@@ -740,7 +740,7 @@ describe('backend-dynamic-feature-service', () => {
           version: '0.0.0',
         },
       ]);
-      expect(pluginManager.frontendPlugins(false)).toEqual([
+      expect(pluginManager.frontendPlugins({ includeFailed: false })).toEqual([
         {
           name: 'a-frontend-plugin',
           platform: 'web',
@@ -754,7 +754,7 @@ describe('backend-dynamic-feature-service', () => {
           version: '0.0.0',
         },
       ]);
-      expect(pluginManager.frontendPlugins(true)).toEqual([
+      expect(pluginManager.frontendPlugins({ includeFailed: true })).toEqual([
         {
           name: 'a-frontend-plugin',
           platform: 'web',
@@ -810,7 +810,7 @@ describe('backend-dynamic-feature-service', () => {
           version: '0.0.0',
         },
       ]);
-      expect(pluginManager.plugins(false)).toEqual([
+      expect(pluginManager.plugins({ includeFailed: false })).toEqual([
         {
           name: 'a-frontend-plugin',
           platform: 'web',
@@ -836,7 +836,7 @@ describe('backend-dynamic-feature-service', () => {
           version: '0.0.0',
         },
       ]);
-      expect(pluginManager.plugins(true)).toEqual([
+      expect(pluginManager.plugins({ includeFailed: true })).toEqual([
         {
           name: 'a-frontend-plugin',
           platform: 'web',

packages/backend-dynamic-feature-service/src/manager/plugin-manager.ts

@@ -217,20 +217,24 @@ export class DynamicPluginManager implements DynamicPluginProvider {
     }
   }
 
-  backendPlugins(includeFailed?: boolean): BackendDynamicPlugin[] {
-    return this.plugins(includeFailed).filter(
+  backendPlugins(options?: {
+    includeFailed?: boolean;
+  }): BackendDynamicPlugin[] {
+    return this.plugins(options).filter(
       (p): p is BackendDynamicPlugin => p.platform === 'node',
     );
   }
 
-  frontendPlugins(includeFailed?: boolean): FrontendDynamicPlugin[] {
-    return this.plugins(includeFailed).filter(
+  frontendPlugins(options?: {
+    includeFailed?: boolean;
+  }): FrontendDynamicPlugin[] {
+    return this.plugins(options).filter(
       (p): p is FrontendDynamicPlugin => p.platform === 'web',
     );
   }
 
-  plugins(includeFailed?: boolean): DynamicPlugin[] {
-    return this._plugins.filter(p => includeFailed || !p.failure);
+  plugins(options?: { includeFailed?: boolean }): DynamicPlugin[] {
+    return this._plugins.filter(p => options?.includeFailed || !p.failure);
   }
 
   getScannedPackage(plugin: DynamicPlugin): ScannedPluginPackage {

packages/backend-dynamic-feature-service/src/manager/types.ts

@@ -79,7 +79,7 @@ export type LegacyPluginEnvironment = {
 export interface DynamicPluginProvider
   extends FrontendPluginProvider,
     BackendPluginProvider {
-  plugins(includeFailed?: boolean): DynamicPlugin[];
+  plugins(options?: { includeFailed?: boolean }): DynamicPlugin[];
   getScannedPackage(plugin: DynamicPlugin): ScannedPluginPackage;
 }
 
@@ -87,14 +87,16 @@ export interface DynamicPluginProvider
  * @public
  */
 export interface BackendPluginProvider {
-  backendPlugins(includeFailed?: boolean): BackendDynamicPlugin[];
+  backendPlugins(options?: { includeFailed?: boolean }): BackendDynamicPlugin[];
 }
 
 /**
  * @public
  */
 export interface FrontendPluginProvider {
-  frontendPlugins(includeFailed?: boolean): FrontendDynamicPlugin[];
+  frontendPlugins(options?: {
+    includeFailed?: boolean;
+  }): FrontendDynamicPlugin[];
 }
 
 /**

packages/backend-dynamic-feature-service/src/scanner/plugin-scanner.ts

@@ -72,25 +72,25 @@ export class PluginScanner {
   private applyConfig(): void | never {
     const dynamicPlugins = this.config.getOptional(configKey);
     if (!dynamicPlugins) {
-      this.logger.info("'dynamicPlugins' config entry not found.");
+      this.logger.info(`'${configKey}' config entry not found.`);
       this._rootDirectory = undefined;
       return;
     }
     if (typeof dynamicPlugins !== 'object') {
-      this.logger.warn("'dynamicPlugins' config entry should be an object.");
+      this.logger.warn(`'${configKey}' config entry should be an object.`);
       this._rootDirectory = undefined;
       return;
     }
     if (!('rootDirectory' in dynamicPlugins)) {
       this.logger.warn(
-        "'dynamicPlugins' config entry does not contain the 'rootDirectory' field.",
+        `'${configKey}' config entry does not contain the 'rootDirectory' field.`,
       );
       this._rootDirectory = undefined;
       return;
     }
     if (typeof dynamicPlugins.rootDirectory !== 'string') {
       this.logger.warn(
-        "'dynamicPlugins.rootDirectory' config entry should be a string.",
+        `'${configKey}.rootDirectory' config entry should be a string.`,
       );
       this._rootDirectory = undefined;
       return;

yarn.lock

@@ -40003,7 +40003,6 @@ __metadata:
   resolution: "root@workspace:."
   dependencies:
     "@backstage/cli": "workspace:*"
-    "@backstage/cli-node": "workspace:^"
     "@backstage/codemods": "workspace:*"
     "@backstage/create-app": "workspace:*"
     "@backstage/e2e-test-utils": "workspace:*"