From 3e3244012d96967d154f3d0c887d2b83ff91b057 Mon Sep 17 00:00:00 2001 From: Kurt King Date: Tue, 22 Nov 2022 19:37:23 -0600 Subject: [PATCH 01/12] add disclaimer to FeatureFlagged section Signed-off-by: Kurt King Signed-off-by: Kurt King --- docs/plugins/feature-flags.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/plugins/feature-flags.md b/docs/plugins/feature-flags.md index a26eb49a9b..c796058ace 100644 --- a/docs/plugins/feature-flags.md +++ b/docs/plugins/feature-flags.md @@ -52,6 +52,8 @@ import { FeatureFlagged } from '@backstage/core-app-api' ``` +If you use a `` component, you do not need to pass `featureFlags: [{ name: 'show-example-feature' }],` into the `createPlugin()` like done at the top of this page. Doing so will cause duplicate entries to appear under `Settings` > `Feature Flags`. + ## Evaluating Feature Flag State It is also possible to test the feature flag state using the [FeatureFlags Api](https://backstage.io/docs/reference/core-plugin-api.featureflagsapi). From 6ebc7c8719d459234983f4a171c25a83c25e1ef8 Mon Sep 17 00:00:00 2001 From: Kurt King Date: Tue, 29 Nov 2022 07:40:54 -0600 Subject: [PATCH 02/12] check to prevent duplicate feature flag components Signed-off-by: Kurt King Signed-off-by: Kurt King --- packages/core-app-api/src/app/AppManager.tsx | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/core-app-api/src/app/AppManager.tsx b/packages/core-app-api/src/app/AppManager.tsx index 0ca09c0a0c..70d7bc5434 100644 --- a/packages/core-app-api/src/app/AppManager.tsx +++ b/packages/core-app-api/src/app/AppManager.tsx @@ -310,8 +310,14 @@ export class AppManager implements BackstageApp { // Go through the featureFlags returned from the traversal and // register those now the configApi has been loaded + const registeredFlags = featureFlagsApi.getRegisteredFlags(); for (const name of featureFlags) { - featureFlagsApi.registerFlag({ name, pluginId: '' }); + // Prevents adding duplicate feature flags + if (!registeredFlags.some((featureFlag) => { + return featureFlag.name === name + })) { + featureFlagsApi.registerFlag({ name, pluginId: '' }); + } } } }, [hasConfigApi, loadedConfig, featureFlags]); From d1b2ec16f4e7d4ec759d411b6d9af3e3ecf1928a Mon Sep 17 00:00:00 2001 From: Kurt King Date: Tue, 29 Nov 2022 07:41:59 -0600 Subject: [PATCH 03/12] revert documentation back to original state Signed-off-by: Kurt King Signed-off-by: Kurt King --- docs/plugins/feature-flags.md | 3 --- 1 file changed, 3 deletions(-) diff --git a/docs/plugins/feature-flags.md b/docs/plugins/feature-flags.md index c796058ace..738bc5e6aa 100644 --- a/docs/plugins/feature-flags.md +++ b/docs/plugins/feature-flags.md @@ -51,9 +51,6 @@ import { FeatureFlagged } from '@backstage/core-app-api' ``` - -If you use a `` component, you do not need to pass `featureFlags: [{ name: 'show-example-feature' }],` into the `createPlugin()` like done at the top of this page. Doing so will cause duplicate entries to appear under `Settings` > `Feature Flags`. - ## Evaluating Feature Flag State It is also possible to test the feature flag state using the [FeatureFlags Api](https://backstage.io/docs/reference/core-plugin-api.featureflagsapi). From 43244e88895ebae8bfa2d55b87612ed9f9ce0d36 Mon Sep 17 00:00:00 2001 From: Kurt King Date: Tue, 29 Nov 2022 09:01:46 -0600 Subject: [PATCH 04/12] restore docs to original state Signed-off-by: Kurt King Signed-off-by: Kurt King --- docs/plugins/feature-flags.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/plugins/feature-flags.md b/docs/plugins/feature-flags.md index 738bc5e6aa..a26eb49a9b 100644 --- a/docs/plugins/feature-flags.md +++ b/docs/plugins/feature-flags.md @@ -51,6 +51,7 @@ import { FeatureFlagged } from '@backstage/core-app-api' ``` + ## Evaluating Feature Flag State It is also possible to test the feature flag state using the [FeatureFlags Api](https://backstage.io/docs/reference/core-plugin-api.featureflagsapi). From 77b0e42982a59a85f499c200c89833496aa5c4a9 Mon Sep 17 00:00:00 2001 From: Kurt King Date: Tue, 29 Nov 2022 14:00:54 -0600 Subject: [PATCH 05/12] use set to look for existing feature flags Signed-off-by: Kurt King Signed-off-by: Kurt King --- packages/core-app-api/src/app/AppManager.tsx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/core-app-api/src/app/AppManager.tsx b/packages/core-app-api/src/app/AppManager.tsx index 70d7bc5434..8c2e09c903 100644 --- a/packages/core-app-api/src/app/AppManager.tsx +++ b/packages/core-app-api/src/app/AppManager.tsx @@ -311,11 +311,10 @@ export class AppManager implements BackstageApp { // Go through the featureFlags returned from the traversal and // register those now the configApi has been loaded const registeredFlags = featureFlagsApi.getRegisteredFlags(); + const flagNames = new Set(registeredFlags.map(f => f.name)); for (const name of featureFlags) { // Prevents adding duplicate feature flags - if (!registeredFlags.some((featureFlag) => { - return featureFlag.name === name - })) { + if (!flagNames.has(name)) { featureFlagsApi.registerFlag({ name, pluginId: '' }); } } From 9c0582c7b50ad31868b0671320f7534589b56e3b Mon Sep 17 00:00:00 2001 From: Kurt King Date: Thu, 1 Dec 2022 06:16:52 -0600 Subject: [PATCH 06/12] fix m1 issue and add current state of tests Signed-off-by: Kurt King --- packages/core-app-api/package.json | 1 + .../core-app-api/src/app/AppManager.test.tsx | 78 +++++++++++++++++++ 2 files changed, 79 insertions(+) diff --git a/packages/core-app-api/package.json b/packages/core-app-api/package.json index 1c15f7d6f5..9a10514ab7 100644 --- a/packages/core-app-api/package.json +++ b/packages/core-app-api/package.json @@ -37,6 +37,7 @@ "@backstage/types": "workspace:^", "@backstage/version-bridge": "workspace:^", "@types/prop-types": "^15.7.3", + "canvas": "^2.10.2", "prop-types": "^15.7.2", "react-use": "^17.2.4", "zen-observable": "^0.9.0", diff --git a/packages/core-app-api/src/app/AppManager.test.tsx b/packages/core-app-api/src/app/AppManager.test.tsx index c70b43581a..b03fedd89a 100644 --- a/packages/core-app-api/src/app/AppManager.test.tsx +++ b/packages/core-app-api/src/app/AppManager.test.tsx @@ -34,9 +34,11 @@ import { createSubRouteRef, createRoutableExtension, analyticsApiRef, + useApi, } from '@backstage/core-plugin-api'; import { AppManager } from './AppManager'; import { AppComponents, AppIcons } from './types'; +import { FeatureFlagged } from '../routing/FeatureFlagged'; describe('Integration Test', () => { const noOpAnalyticsApi = createApiFactory( @@ -415,6 +417,82 @@ describe('Integration Test', () => { }); }); + it('should prevent duplicate feature flags from being rendered', async () => { + const p1 = createPlugin({ + id: 'p1', + featureFlags: [{ name: 'show-p1-feature' }], + }); + const p2 = createPlugin({ + id: 'p2', + featureFlags: [{ name: 'show-p2-feature' }], + }); + + const storageFlags = new LocalStorageFeatureFlags(); + jest.spyOn(storageFlags, 'registerFlag'); + + const apis = [ + noOpAnalyticsApi, + createApiFactory({ + api: featureFlagsApiRef, + deps: { configApi: configApiRef }, + factory() { + return storageFlags; + }, + }), + ]; + + const app = new AppManager({ + apis, + defaultApis: [], + themes: [ + { + id: 'light', + title: 'Light Theme', + variant: 'light', + Provider: ({ children }) => <>{children}, + }, + ], + icons, + plugins: [p1, p2], + components, + configLoader: async () => [], + bindRoutes: ({ bind }) => { + bind(plugin1.externalRoutes, { + extRouteRef1: plugin1RouteRef, + extRouteRef2: plugin2RouteRef, + }); + }, + }); + + const Provider = app.getProvider(); + const Router = app.getRouter(); + + function FeatureFlags() { + const featureFlags = useApi(featureFlagsApiRef); + return ( +
Flags: {featureFlags.getRegisteredFlags().map(f => f.name)}
+ ); + } + + await renderWithEffects( + + + +
My feature behind a flag
+ + +
My feature behind a flag
+ + + + , + ); + + // we need this test to mimic adding `featureFlags` to a plugin while also passing the value in to a component + + // the added code should prevent duplicates from being added. + }); + it('should track route changes via analytics api', async () => { const mockAnalyticsApi = new MockAnalyticsApi(); const apis = [createApiFactory(analyticsApiRef, mockAnalyticsApi)]; From 770108968b07eb7fa4ed912165b3cf1809dedfdb Mon Sep 17 00:00:00 2001 From: Kurt King Date: Fri, 2 Dec 2022 09:34:38 -0600 Subject: [PATCH 07/12] add yarn.lock Signed-off-by: Kurt King --- yarn.lock | 1 + 1 file changed, 1 insertion(+) diff --git a/yarn.lock b/yarn.lock index f3dcb1a699..5c09cef83a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3339,6 +3339,7 @@ __metadata: "@types/node": ^16.11.26 "@types/prop-types": ^15.7.3 "@types/zen-observable": ^0.8.0 + canvas: ^2.10.2 cross-fetch: ^3.1.5 msw: ^0.49.0 prop-types: ^15.7.2 From b408e891c9d47ab43ff2b9e0b676945107824b42 Mon Sep 17 00:00:00 2001 From: Kurt King Date: Fri, 2 Dec 2022 09:51:58 -0600 Subject: [PATCH 08/12] add test for duplicate feature flags Signed-off-by: Kurt King --- packages/core-app-api/src/app/AppManager.test.tsx | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/core-app-api/src/app/AppManager.test.tsx b/packages/core-app-api/src/app/AppManager.test.tsx index 5a29cc3bfd..e9b18d46e0 100644 --- a/packages/core-app-api/src/app/AppManager.test.tsx +++ b/packages/core-app-api/src/app/AppManager.test.tsx @@ -488,7 +488,10 @@ describe('Integration Test', () => { function FeatureFlags() { const featureFlags = useApi(featureFlagsApiRef); return ( -
Flags: {featureFlags.getRegisteredFlags().map(f => f.name)}
+
{`Flags: ${featureFlags + .getRegisteredFlags() + .map(f => f.name) + .join(',')}`}
); } @@ -506,9 +509,9 @@ describe('Integration Test', () => { , ); - // we need this test to mimic adding `featureFlags` to a plugin while also passing the value in to a component - - // the added code should prevent duplicates from being added. + expect( + screen.getByText('Flags: show-p1-feature,show-p2-feature'), + ).toBeInTheDocument(); }); it('should track route changes via analytics api', async () => { From d2ee1c8ad791ac929f48312a05dc7c399575ffc4 Mon Sep 17 00:00:00 2001 From: Kurt King Date: Fri, 2 Dec 2022 10:07:41 -0600 Subject: [PATCH 09/12] remove dependency that was added by accident Signed-off-by: Kurt King --- packages/core-app-api/package.json | 1 - yarn.lock | 1 - 2 files changed, 2 deletions(-) diff --git a/packages/core-app-api/package.json b/packages/core-app-api/package.json index 5f548374f4..6f4961403d 100644 --- a/packages/core-app-api/package.json +++ b/packages/core-app-api/package.json @@ -37,7 +37,6 @@ "@backstage/types": "workspace:^", "@backstage/version-bridge": "workspace:^", "@types/prop-types": "^15.7.3", - "canvas": "^2.10.2", "prop-types": "^15.7.2", "react-use": "^17.2.4", "zen-observable": "^0.10.0", diff --git a/yarn.lock b/yarn.lock index 6736847714..e3d1b5eb88 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3816,7 +3816,6 @@ __metadata: "@types/node": ^16.11.26 "@types/prop-types": ^15.7.3 "@types/zen-observable": ^0.8.0 - canvas: ^2.10.2 cross-fetch: ^3.1.5 msw: ^0.49.0 prop-types: ^15.7.2 From 203271b74618eb5b68a4227da8fc98368d10d078 Mon Sep 17 00:00:00 2001 From: Kurt King Date: Fri, 2 Dec 2022 10:25:49 -0600 Subject: [PATCH 10/12] add changeset Signed-off-by: Kurt King --- .changeset/nice-apricots-love.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/nice-apricots-love.md diff --git a/.changeset/nice-apricots-love.md b/.changeset/nice-apricots-love.md new file mode 100644 index 0000000000..c5f308dd8f --- /dev/null +++ b/.changeset/nice-apricots-love.md @@ -0,0 +1,5 @@ +--- +'@backstage/core-app-api': minor +--- + +Prevent duplicate feature flag components from rendering in the settings when using components From 5a8dd535a5c2c79c2c2ecd6a7f99cde50511c6fc Mon Sep 17 00:00:00 2001 From: Kurt King Date: Fri, 2 Dec 2022 11:07:32 -0600 Subject: [PATCH 11/12] update changeset to patch Signed-off-by: Kurt King --- .changeset/nice-apricots-love.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/nice-apricots-love.md b/.changeset/nice-apricots-love.md index c5f308dd8f..527b644d5f 100644 --- a/.changeset/nice-apricots-love.md +++ b/.changeset/nice-apricots-love.md @@ -1,5 +1,5 @@ --- -'@backstage/core-app-api': minor +'@backstage/core-app-api': patch --- Prevent duplicate feature flag components from rendering in the settings when using components From 09454420c6881f40122e0eb3df0284b1b0a57fbc Mon Sep 17 00:00:00 2001 From: Kurt King Date: Fri, 2 Dec 2022 11:58:27 -0600 Subject: [PATCH 12/12] cleanup test Signed-off-by: Kurt King --- .../core-app-api/src/app/AppManager.test.tsx | 31 ++----------------- 1 file changed, 2 insertions(+), 29 deletions(-) diff --git a/packages/core-app-api/src/app/AppManager.test.tsx b/packages/core-app-api/src/app/AppManager.test.tsx index e9b18d46e0..f81f382650 100644 --- a/packages/core-app-api/src/app/AppManager.test.tsx +++ b/packages/core-app-api/src/app/AppManager.test.tsx @@ -445,41 +445,14 @@ describe('Integration Test', () => { featureFlags: [{ name: 'show-p2-feature' }], }); - const storageFlags = new LocalStorageFeatureFlags(); - jest.spyOn(storageFlags, 'registerFlag'); - - const apis = [ - noOpAnalyticsApi, - createApiFactory({ - api: featureFlagsApiRef, - deps: { configApi: configApiRef }, - factory() { - return storageFlags; - }, - }), - ]; - const app = new AppManager({ - apis, + apis: [], defaultApis: [], - themes: [ - { - id: 'light', - title: 'Light Theme', - variant: 'light', - Provider: ({ children }) => <>{children}, - }, - ], + themes, icons, plugins: [p1, p2], components, configLoader: async () => [], - bindRoutes: ({ bind }) => { - bind(plugin1.externalRoutes, { - extRouteRef1: plugin1RouteRef, - extRouteRef2: plugin2RouteRef, - }); - }, }); const Provider = app.getProvider();