From 67f98d051d9e02b4d59c672a7d8db880b4224c5f Mon Sep 17 00:00:00 2001 From: David Festal Date: Tue, 8 Oct 2024 14:47:11 +0200 Subject: [PATCH] Fix review comments Signed-off-by: David Festal --- package.json | 1 - .../backend-dynamic-feature-service/README.md | 2 +- .../report.api.md | 94 ++++++++++--------- .../src/features/features.test.ts | 4 +- .../src/manager/plugin-manager.test.ts | 12 +-- .../src/manager/plugin-manager.ts | 16 ++-- .../src/manager/types.ts | 8 +- .../src/scanner/plugin-scanner.ts | 8 +- yarn.lock | 1 - 9 files changed, 78 insertions(+), 68 deletions(-) diff --git a/package.json b/package.json index 4826bcd56b..425b1c1a38 100644 --- a/package.json +++ b/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", diff --git a/packages/backend-dynamic-feature-service/README.md b/packages/backend-dynamic-feature-service/README.md index f4ca11bb1f..6f5af13636 100644 --- a/packages/backend-dynamic-feature-service/README.md +++ b/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 + ``` diff --git a/packages/backend-dynamic-feature-service/report.api.md b/packages/backend-dynamic-feature-service/report.api.md index 615b053869..edbe48b263 100644 --- a/packages/backend-dynamic-feature-service/report.api.md +++ b/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; // (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". diff --git a/packages/backend-dynamic-feature-service/src/features/features.test.ts b/packages/backend-dynamic-feature-service/src/features/features.test.ts index daee127edc..851e1d0578 100644 --- a/packages/backend-dynamic-feature-service/src/features/features.test.ts +++ b/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 }), + ); }, }); }, diff --git a/packages/backend-dynamic-feature-service/src/manager/plugin-manager.test.ts b/packages/backend-dynamic-feature-service/src/manager/plugin-manager.test.ts index 6c129c07e2..b6854e3263 100644 --- a/packages/backend-dynamic-feature-service/src/manager/plugin-manager.test.ts +++ b/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', diff --git a/packages/backend-dynamic-feature-service/src/manager/plugin-manager.ts b/packages/backend-dynamic-feature-service/src/manager/plugin-manager.ts index a3b4bfbf98..a94a01f0f1 100644 --- a/packages/backend-dynamic-feature-service/src/manager/plugin-manager.ts +++ b/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 { diff --git a/packages/backend-dynamic-feature-service/src/manager/types.ts b/packages/backend-dynamic-feature-service/src/manager/types.ts index d3989b5ff9..89ac140bc6 100644 --- a/packages/backend-dynamic-feature-service/src/manager/types.ts +++ b/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[]; } /** diff --git a/packages/backend-dynamic-feature-service/src/scanner/plugin-scanner.ts b/packages/backend-dynamic-feature-service/src/scanner/plugin-scanner.ts index 80ae2ed876..7091999e7b 100644 --- a/packages/backend-dynamic-feature-service/src/scanner/plugin-scanner.ts +++ b/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; diff --git a/yarn.lock b/yarn.lock index c360f2247d..36f19e9093 100644 --- a/yarn.lock +++ b/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:*"