From 8145ba8b945f358b5dec62e6dad3cc49741a99aa Mon Sep 17 00:00:00 2001 From: Bilawal Hameed Date: Thu, 26 Mar 2020 11:21:46 +0100 Subject: [PATCH 01/35] [docs] added skeleton --- docs/reference/README.md | 1 + docs/reference/createPlugin-feature-flags.md | 15 +++++++++++++++ docs/reference/createPlugin.md | 9 ++++++--- 3 files changed, 22 insertions(+), 3 deletions(-) create mode 100644 docs/reference/createPlugin-feature-flags.md diff --git a/docs/reference/README.md b/docs/reference/README.md index 8438917945..dd926d57dc 100644 --- a/docs/reference/README.md +++ b/docs/reference/README.md @@ -4,5 +4,6 @@ APIs and Components - [createPlugin](createPlugin.md) - [createPlugin - router](createPlugin-router.md) +- [createPlugin - feature flags](createPlugin-feature-flags.md) [Back to Docs](../README.md) diff --git a/docs/reference/createPlugin-feature-flags.md b/docs/reference/createPlugin-feature-flags.md new file mode 100644 index 0000000000..d8ad043ddc --- /dev/null +++ b/docs/reference/createPlugin-feature-flags.md @@ -0,0 +1,15 @@ +# createPlugin - feature flags + +The `featureFlags` object passed to the `register` function makes it possible for plugins to register Feature Flags in Backstage for users to opt into. You can use this to split out logic in your code for manual A/B testing, etc. + +```typescript +// This needs to be updated. +``` + +Then, later, if you want to check if the user has enabled them: + +```typescript +// This needs to be updated. +``` + +[Back to References](README.md) diff --git a/docs/reference/createPlugin.md b/docs/reference/createPlugin.md index 77048199ef..c0b4dece71 100644 --- a/docs/reference/createPlugin.md +++ b/docs/reference/createPlugin.md @@ -17,13 +17,14 @@ type PluginHooks = { }; ``` -[Read more about the router here](createPlugin-router.md) +- [Read more about the router here](createPlugin-router.md) +- [Read more about feature flags here](createPlugin-feature-flags.md) ## Example Uses ### Creating a basic plugin -Showcasing adding multiple routes and a redirect. +Showcasing adding multiple routes, a feature flag and a redirect. ```jsx import { createPlugin } from '@backstage/core'; @@ -31,7 +32,9 @@ import ExampleComponent from './components/ExampleComponent'; export default createPlugin({ id: 'new-plugin', - register({ router }) { + register({ router, featureFlags: { registerFeatureFlag } }) { + registerFeatureFlag('enable-example-component'); + router.registerRoute('/new-plugin', ExampleComponent); }, }); From 45d7e2736f25fae55a1f32e06040ebf2d5914f5f Mon Sep 17 00:00:00 2001 From: Bilawal Hameed Date: Thu, 26 Mar 2020 11:34:30 +0100 Subject: [PATCH 02/35] [core/api/plugins] added featureFlags.registerFeatureFlag --- packages/core/src/api/plugin/Plugin.tsx | 17 ++++++++++++++++- packages/core/src/api/plugin/types.ts | 13 ++++++++++++- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/packages/core/src/api/plugin/Plugin.tsx b/packages/core/src/api/plugin/Plugin.tsx index efaabd0ff1..aa1ff417b7 100644 --- a/packages/core/src/api/plugin/Plugin.tsx +++ b/packages/core/src/api/plugin/Plugin.tsx @@ -15,7 +15,12 @@ */ import { ComponentType } from 'react'; -import { PluginOutput, RoutePath, RouteOptions } from './types'; +import { + PluginOutput, + RoutePath, + RouteOptions, + FeatureFlagName, +} from './types'; import { Widget } from '../widgetView/types'; export type PluginConfig = { @@ -26,6 +31,7 @@ export type PluginConfig = { export type PluginHooks = { router: RouterHooks; widgets: WidgetHooks; + featureFlags: FeatureFlagsHooks; }; export type RouterHooks = { @@ -46,6 +52,10 @@ export type WidgetHooks = { add(widget: Widget): void; }; +export type FeatureFlagsHooks = { + registerFeatureFlag(name: FeatureFlagName): void; +}; + export const registerSymbol = Symbol('plugin-register'); export const outputSymbol = Symbol('plugin-output'); @@ -78,6 +88,11 @@ export default class Plugin { outputs.push({ type: 'widget', widget }); }, }, + featureFlags: { + registerFeatureFlag(name) { + outputs.push({ type: 'feature-flag', name }); + }, + }, }); this.storedOutput = outputs; diff --git a/packages/core/src/api/plugin/types.ts b/packages/core/src/api/plugin/types.ts index 47c121fae8..b006e06d9a 100644 --- a/packages/core/src/api/plugin/types.ts +++ b/packages/core/src/api/plugin/types.ts @@ -43,4 +43,15 @@ export type WidgetOutput = { widget: Widget; }; -export type PluginOutput = RouteOutput | RedirectRouteOutput | WidgetOutput; +export type FeatureFlagName = string; + +export type FeatureFlagOutput = { + type: 'feature-flag'; + name: FeatureFlagName; +}; + +export type PluginOutput = + | RouteOutput + | RedirectRouteOutput + | WidgetOutput + | FeatureFlagOutput; From e6a88a5dc0feadc99b2f37bdcb46ae140ff7353f Mon Sep 17 00:00:00 2001 From: Bilawal Hameed Date: Thu, 26 Mar 2020 11:56:37 +0100 Subject: [PATCH 03/35] [core/AppBuilder] collect list of registered feature flags --- packages/core/src/api/app/AppBuilder.tsx | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/core/src/api/app/AppBuilder.tsx b/packages/core/src/api/app/AppBuilder.tsx index ab6f60a293..b29cef79c3 100644 --- a/packages/core/src/api/app/AppBuilder.tsx +++ b/packages/core/src/api/app/AppBuilder.tsx @@ -19,6 +19,7 @@ import { Route, Switch, Redirect } from 'react-router-dom'; import { AppContextProvider } from './AppContext'; import { App } from './types'; import BackstagePlugin from '../plugin/Plugin'; +import { FeatureFlagName } from '../plugin/Plugin/types'; import { IconComponent, SystemIcons, @@ -62,6 +63,7 @@ export default class AppBuilder { const app = new AppImpl(this.systemIcons); const routes = new Array(); + const registeredFeatureFlags = new Array<{ name: FeatureFlagName }>(); for (const plugin of this.plugins.values()) { for (const output of plugin.output()) { @@ -87,6 +89,10 @@ export default class AppBuilder { ); break; } + case 'feature-flag': { + registeredFeatureFlags.push({ name: output.name }); + break; + } default: break; } From 79654e477e47dfae70fdefee0e0d9e6c759917a6 Mon Sep 17 00:00:00 2001 From: Bilawal Hameed Date: Thu, 26 Mar 2020 12:18:05 +0100 Subject: [PATCH 04/35] [core/apis/definitions] added featureFlags definition --- .../src/api/apis/definitions/featureFlags.ts | 53 +++++++++++++++++++ .../core/src/api/apis/definitions/index.ts | 1 + 2 files changed, 54 insertions(+) create mode 100644 packages/core/src/api/apis/definitions/featureFlags.ts diff --git a/packages/core/src/api/apis/definitions/featureFlags.ts b/packages/core/src/api/apis/definitions/featureFlags.ts new file mode 100644 index 0000000000..c3656ac4cd --- /dev/null +++ b/packages/core/src/api/apis/definitions/featureFlags.ts @@ -0,0 +1,53 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import ApiRef from '../ApiRef'; +import { FeatureFlagName } from '../../plugin/types'; + +/** + * The feature flags API is used to toggle functionality to users across plugins and Backstage. + * + * Plugins can use this API to register feature flags that they have available + * for users to enable/disable, and this API will centralize the current user's + * state of which feature flags they would like to enable. + * + * This is ideal for Backstage plugins, as well as your own App, to trial incomplete + * or unstable upcoming features. Although there will be a common interface for users + * to enable and disable feature flags, this API acts as another way to enable/disable. + */ +export type FeatureFlagsApi = { + /** + * Get the current user's status of a Feature Flag + * + * @returns bool True if the current user has enabled the feature flag + */ + getItem(name: FeatureFlagName): boolean; + + /** + * Enable an registered feature flag. + */ + enable(name: FeatureFlagName): void; + + /** + * Disable an registered feature flag. + */ + disable(name: FeatureFlagName): void; +}; + +export const featureFlagsApiRef = new ApiRef({ + id: 'core.featureflags', + description: 'Used to toggle functionality in features across Backstage', +}); diff --git a/packages/core/src/api/apis/definitions/index.ts b/packages/core/src/api/apis/definitions/index.ts index bdf2a0b6a2..55923b50b7 100644 --- a/packages/core/src/api/apis/definitions/index.ts +++ b/packages/core/src/api/apis/definitions/index.ts @@ -21,3 +21,4 @@ // If you think some API definition is missing, please open an Issue or send a PR! export * from './error'; +export * from './featureFlags'; From 817c4760504a7568685ea7436b565cfde0cd1989 Mon Sep 17 00:00:00 2001 From: Bilawal Hameed Date: Thu, 26 Mar 2020 12:22:01 +0100 Subject: [PATCH 05/35] [docs] updated with typescript definitions + code sample --- docs/reference/createPlugin-feature-flags.md | 24 +++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/docs/reference/createPlugin-feature-flags.md b/docs/reference/createPlugin-feature-flags.md index d8ad043ddc..6c3a5f0bc5 100644 --- a/docs/reference/createPlugin-feature-flags.md +++ b/docs/reference/createPlugin-feature-flags.md @@ -3,13 +3,31 @@ The `featureFlags` object passed to the `register` function makes it possible for plugins to register Feature Flags in Backstage for users to opt into. You can use this to split out logic in your code for manual A/B testing, etc. ```typescript -// This needs to be updated. +export type FeatureFlagsHooks = { + registerFeatureFlag(name: FeatureFlagName): void; +}; ``` Then, later, if you want to check if the user has enabled them: -```typescript -// This needs to be updated. +```tsx +import React, { FC } from 'react'; +import { Button } from '@material-ui/core'; +import { featureFlagsApiRef, useApi } from '@backstage/core'; + +const ExampleButton: FC<{}> = () => { + const featureFlagsApi = useApi(featureFlagsApiRef); + + const handleClick = () => { + featureFlagsApi.enable('enable-example-feature'); + }; + + return ( + + ); +}; ``` [Back to References](README.md) From 3e6d2ce730d2ef02e57adc0e11b57cf9b1f86f53 Mon Sep 17 00:00:00 2001 From: Bilawal Hameed Date: Thu, 26 Mar 2020 12:56:56 +0100 Subject: [PATCH 06/35] [core/api/app] add FeatureFlags API implementation --- packages/core/src/api/app/FeatureFlags.tsx | 73 ++++++++++++++++++++++ packages/core/src/api/index.ts | 1 + 2 files changed, 74 insertions(+) create mode 100644 packages/core/src/api/app/FeatureFlags.tsx diff --git a/packages/core/src/api/app/FeatureFlags.tsx b/packages/core/src/api/app/FeatureFlags.tsx new file mode 100644 index 0000000000..31a3c37341 --- /dev/null +++ b/packages/core/src/api/app/FeatureFlags.tsx @@ -0,0 +1,73 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// import React, { createContext, useContext, useState, FC } from 'react'; +import { FeatureFlagName } from '../plugin/Plugin/types'; +import { FeatureFlagsApi } from '../apis/definitions/featureFlags'; + +// TODO: figure out where to put implementations of APIs, both inside apps +// but also in core/separate package. +export class FeatureFlags implements FeatureFlagsApi { + private readonly localStorageKey = 'featureFlags'; + + private getEnabledFeatureFlags(): Set { + if (!('localStorage' in window)) { + throw new Error( + 'Feature Flags are not supported on browsers without the Local Storage API', + ); + } + + try { + const featureFlagsJson = window.localStorage.getItem( + this.localStorageKey, + ); + return new Set<>(Object.keys(JSON.parse(featureFlagsJson!))); + } catch (err) { + return new Set<>(); + } + } + + private saveFeatureFlags(flags: Set): void { + if (!('localStorage' in window)) { + throw new Error( + 'Feature Flags are not supported on browsers without the Local Storage API', + ); + } + + window.localStorage.setItem( + this.localStorageKey, + JSON.stringify( + [...flags].reduce((list, flag) => ({ ...list, [flag]: true }), {}), + ), + ); + } + + getItem(name: FeatureFlagName): boolean { + return this.getFeatureFlags().has(name); + } + + enable(name: FeatureFlagName): void { + const flags = this.getFeatureFlags(); + flags.add(name); + this.saveFeatureFlags(flags); + } + + disable(name: FeatureFlagName): void { + const flags = this.getFeatureFlags(); + flags.delete(name); + this.saveFeatureFlags(flags); + } +} diff --git a/packages/core/src/api/index.ts b/packages/core/src/api/index.ts index dff75514ae..e60a7efa18 100644 --- a/packages/core/src/api/index.ts +++ b/packages/core/src/api/index.ts @@ -16,4 +16,5 @@ export * from './api'; export * from './apis'; +export { FeatureFlags } from './app/FeatureFlags'; export { useApp } from './app/AppContext'; From 10a7102f4c48604597db02f4b0f1f6c9ace3f7e8 Mon Sep 17 00:00:00 2001 From: Bilawal Hameed Date: Thu, 26 Mar 2020 13:12:17 +0100 Subject: [PATCH 07/35] [core/api/app] Set FeatureFlags class to static --- packages/core/src/api/app/FeatureFlags.tsx | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/core/src/api/app/FeatureFlags.tsx b/packages/core/src/api/app/FeatureFlags.tsx index 31a3c37341..c75e072149 100644 --- a/packages/core/src/api/app/FeatureFlags.tsx +++ b/packages/core/src/api/app/FeatureFlags.tsx @@ -21,9 +21,9 @@ import { FeatureFlagsApi } from '../apis/definitions/featureFlags'; // TODO: figure out where to put implementations of APIs, both inside apps // but also in core/separate package. export class FeatureFlags implements FeatureFlagsApi { - private readonly localStorageKey = 'featureFlags'; + private static readonly localStorageKey = 'featureFlags'; - private getEnabledFeatureFlags(): Set { + private static getEnabledFeatureFlags(): Set { if (!('localStorage' in window)) { throw new Error( 'Feature Flags are not supported on browsers without the Local Storage API', @@ -40,7 +40,7 @@ export class FeatureFlags implements FeatureFlagsApi { } } - private saveFeatureFlags(flags: Set): void { + private static saveFeatureFlags(flags: Set): void { if (!('localStorage' in window)) { throw new Error( 'Feature Flags are not supported on browsers without the Local Storage API', @@ -55,17 +55,17 @@ export class FeatureFlags implements FeatureFlagsApi { ); } - getItem(name: FeatureFlagName): boolean { + static getItem(name: FeatureFlagName): boolean { return this.getFeatureFlags().has(name); } - enable(name: FeatureFlagName): void { + static enable(name: FeatureFlagName): void { const flags = this.getFeatureFlags(); flags.add(name); this.saveFeatureFlags(flags); } - disable(name: FeatureFlagName): void { + static disable(name: FeatureFlagName): void { const flags = this.getFeatureFlags(); flags.delete(name); this.saveFeatureFlags(flags); From b865bdd4729d8b433905ebc21aa6198067dd16ae Mon Sep 17 00:00:00 2001 From: Bilawal Hameed Date: Thu, 26 Mar 2020 13:35:45 +0100 Subject: [PATCH 08/35] [core] fix bad imports --- packages/core/src/api/app/AppBuilder.tsx | 2 +- packages/core/src/api/app/FeatureFlags.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core/src/api/app/AppBuilder.tsx b/packages/core/src/api/app/AppBuilder.tsx index b29cef79c3..6ea2d0f366 100644 --- a/packages/core/src/api/app/AppBuilder.tsx +++ b/packages/core/src/api/app/AppBuilder.tsx @@ -19,7 +19,7 @@ import { Route, Switch, Redirect } from 'react-router-dom'; import { AppContextProvider } from './AppContext'; import { App } from './types'; import BackstagePlugin from '../plugin/Plugin'; -import { FeatureFlagName } from '../plugin/Plugin/types'; +import { FeatureFlagName } from '../plugin/types'; import { IconComponent, SystemIcons, diff --git a/packages/core/src/api/app/FeatureFlags.tsx b/packages/core/src/api/app/FeatureFlags.tsx index c75e072149..fc0d25dee5 100644 --- a/packages/core/src/api/app/FeatureFlags.tsx +++ b/packages/core/src/api/app/FeatureFlags.tsx @@ -15,7 +15,7 @@ */ // import React, { createContext, useContext, useState, FC } from 'react'; -import { FeatureFlagName } from '../plugin/Plugin/types'; +import { FeatureFlagName } from '../plugin/types'; import { FeatureFlagsApi } from '../apis/definitions/featureFlags'; // TODO: figure out where to put implementations of APIs, both inside apps From c86387ea09c72902380b413a3a5bd3fa80e416f8 Mon Sep 17 00:00:00 2001 From: Bilawal Hameed Date: Thu, 26 Mar 2020 13:36:41 +0100 Subject: [PATCH 09/35] [core] added typescript decorators (experimental) --- packages/core/src/api/app/FeatureFlags.tsx | 20 ++++++---- packages/core/src/testUtils/decorators.ts | 45 ++++++++++++++++++++++ packages/core/src/testUtils/index.js | 1 + packages/core/tsconfig.json | 3 +- 4 files changed, 60 insertions(+), 9 deletions(-) create mode 100644 packages/core/src/testUtils/decorators.ts diff --git a/packages/core/src/api/app/FeatureFlags.tsx b/packages/core/src/api/app/FeatureFlags.tsx index fc0d25dee5..e55660f240 100644 --- a/packages/core/src/api/app/FeatureFlags.tsx +++ b/packages/core/src/api/app/FeatureFlags.tsx @@ -17,13 +17,15 @@ // import React, { createContext, useContext, useState, FC } from 'react'; import { FeatureFlagName } from '../plugin/types'; import { FeatureFlagsApi } from '../apis/definitions/featureFlags'; +import { staticImplements } from '../../testUtils'; // TODO: figure out where to put implementations of APIs, both inside apps // but also in core/separate package. -export class FeatureFlags implements FeatureFlagsApi { +@staticImplements() +export class FeatureFlags { private static readonly localStorageKey = 'featureFlags'; - private static getEnabledFeatureFlags(): Set { + private static getUserEnabledFeatureFlags(): Set { if (!('localStorage' in window)) { throw new Error( 'Feature Flags are not supported on browsers without the Local Storage API', @@ -40,7 +42,9 @@ export class FeatureFlags implements FeatureFlagsApi { } } - private static saveFeatureFlags(flags: Set): void { + private static saveUserEnabledFeatureFlags( + flags: Set, + ): void { if (!('localStorage' in window)) { throw new Error( 'Feature Flags are not supported on browsers without the Local Storage API', @@ -56,18 +60,18 @@ export class FeatureFlags implements FeatureFlagsApi { } static getItem(name: FeatureFlagName): boolean { - return this.getFeatureFlags().has(name); + return this.getUserEnabledFeatureFlags().has(name); } static enable(name: FeatureFlagName): void { - const flags = this.getFeatureFlags(); + const flags = this.getUserEnabledFeatureFlags(); flags.add(name); - this.saveFeatureFlags(flags); + this.saveUserEnabledFeatureFlags(flags); } static disable(name: FeatureFlagName): void { - const flags = this.getFeatureFlags(); + const flags = this.getUserEnabledFeatureFlags(); flags.delete(name); - this.saveFeatureFlags(flags); + this.saveUserEnabledFeatureFlags(flags); } } diff --git a/packages/core/src/testUtils/decorators.ts b/packages/core/src/testUtils/decorators.ts new file mode 100644 index 0000000000..2cea8901b4 --- /dev/null +++ b/packages/core/src/testUtils/decorators.ts @@ -0,0 +1,45 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * This utilizes an experimental TypeScript feature called decorators. + * This was originally added to allow us to expose a FeatureFlags API + * with static methods (for backwards compatibility primarily). It takes + * an existing interface and applies the types to static methods in classes. + * + * @see https://www.typescriptlang.org/docs/handbook/decorators.html + * @example + * interface StaticProps { + * append(name: string, extra: string): string; + * reverse(name: string): string; + * } + * + * @staticImplements() + * class StaticPropsClass { + * static append(name, extra) { + * return `${name}${extra}`; + * } + * + * static reverse(name) { + * return name.reverse(); + * } + * } + */ +export function staticImplements() { + return (constructor: U) => { + constructor; + }; +} diff --git a/packages/core/src/testUtils/index.js b/packages/core/src/testUtils/index.js index a0eab3e72b..902ba5d92d 100644 --- a/packages/core/src/testUtils/index.js +++ b/packages/core/src/testUtils/index.js @@ -28,6 +28,7 @@ import { render } from '@testing-library/react'; export { default as Keyboard } from './Keyboard'; export { default as mockBreakpoint } from './mockBreakpoint'; export * from './logCollector'; +export * from './decorators'; export function wrapInTestApp(Component, initialRouterEntries) { const Wrapper = Component instanceof Function ? Component : () => Component; diff --git a/packages/core/tsconfig.json b/packages/core/tsconfig.json index ad43a0b986..4e024e548f 100644 --- a/packages/core/tsconfig.json +++ b/packages/core/tsconfig.json @@ -2,6 +2,7 @@ "extends": "../../tsconfig.json", "include": ["src"], "compilerOptions": { - "noImplicitAny": false + "noImplicitAny": false, + "experimentalDecorators": true } } From 7ba4f4ca2db2038e20e1bd1b094552cf78a88d2e Mon Sep 17 00:00:00 2001 From: Bilawal Hameed Date: Thu, 26 Mar 2020 15:39:19 +0100 Subject: [PATCH 10/35] [plugins/welcome/WelcomePage] add featureFlags via useApi --- packages/app/src/apis.ts | 9 ++++++++- .../welcome/src/components/WelcomePage/WelcomePage.tsx | 3 +++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/packages/app/src/apis.ts b/packages/app/src/apis.ts index c3315257d8..4d339c3734 100644 --- a/packages/app/src/apis.ts +++ b/packages/app/src/apis.ts @@ -14,7 +14,13 @@ * limitations under the License. */ -import { ApiHolder, ApiRegistry, errorApiRef } from '@backstage/core'; +import { + ApiHolder, + ApiRegistry, + errorApiRef, + featureFlagsApiRef, + FeatureFlags, +} from '@backstage/core'; import { ErrorDisplayForwarder } from './components/ErrorDisplay/ErrorDisplay'; const builder = ApiRegistry.builder(); @@ -22,5 +28,6 @@ const builder = ApiRegistry.builder(); export const errorDialogForwarder = new ErrorDisplayForwarder(); builder.add(errorApiRef, errorDialogForwarder); +builder.add(featureFlagsApiRef, FeatureFlags); export default builder.build() as ApiHolder; diff --git a/plugins/welcome/src/components/WelcomePage/WelcomePage.tsx b/plugins/welcome/src/components/WelcomePage/WelcomePage.tsx index 69a892a95e..0b6eb71982 100644 --- a/plugins/welcome/src/components/WelcomePage/WelcomePage.tsx +++ b/plugins/welcome/src/components/WelcomePage/WelcomePage.tsx @@ -33,10 +33,13 @@ import { pageTheme, ContentHeader, SupportButton, + featureFlagsApiRef, + useApi, } from '@backstage/core'; import ErrorButton from './ErrorButton'; const WelcomePage: FC<{}> = () => { + const featureFlagsApi = useApi(featureFlagsApiRef); const profile = { givenName: '' }; return ( From 85c3ce7d9a9344c62b8c6370a74850152757cab8 Mon Sep 17 00:00:00 2001 From: Bilawal Hameed Date: Thu, 26 Mar 2020 15:50:45 +0100 Subject: [PATCH 11/35] [core/api/app] added context for registered flags --- packages/core/src/api/app/AppBuilder.tsx | 11 ++++++-- packages/core/src/api/app/FeatureFlags.tsx | 33 +++++++++++++++++++++- packages/core/src/api/index.ts | 2 +- 3 files changed, 42 insertions(+), 4 deletions(-) diff --git a/packages/core/src/api/app/AppBuilder.tsx b/packages/core/src/api/app/AppBuilder.tsx index 6ea2d0f366..95537d5101 100644 --- a/packages/core/src/api/app/AppBuilder.tsx +++ b/packages/core/src/api/app/AppBuilder.tsx @@ -19,7 +19,7 @@ import { Route, Switch, Redirect } from 'react-router-dom'; import { AppContextProvider } from './AppContext'; import { App } from './types'; import BackstagePlugin from '../plugin/Plugin'; -import { FeatureFlagName } from '../plugin/types'; +import { FeatureFlagsEntry, FeatureFlagsContextProvider } from './FeatureFlags'; import { IconComponent, SystemIcons, @@ -63,7 +63,7 @@ export default class AppBuilder { const app = new AppImpl(this.systemIcons); const routes = new Array(); - const registeredFeatureFlags = new Array<{ name: FeatureFlagName }>(); + const registeredFeatureFlags = new Array(); for (const plugin of this.plugins.values()) { for (const output of plugin.output()) { @@ -114,6 +114,13 @@ export default class AppBuilder { rendered = ; } + rendered = ( + + ); + return () => ; } } diff --git a/packages/core/src/api/app/FeatureFlags.tsx b/packages/core/src/api/app/FeatureFlags.tsx index e55660f240..a5b0c7eaed 100644 --- a/packages/core/src/api/app/FeatureFlags.tsx +++ b/packages/core/src/api/app/FeatureFlags.tsx @@ -14,7 +14,7 @@ * limitations under the License. */ -// import React, { createContext, useContext, useState, FC } from 'react'; +import React, { ComponentType, createContext, FC } from 'react'; import { FeatureFlagName } from '../plugin/types'; import { FeatureFlagsApi } from '../apis/definitions/featureFlags'; import { staticImplements } from '../../testUtils'; @@ -75,3 +75,34 @@ export class FeatureFlags { this.saveUserEnabledFeatureFlags(flags); } } + +/** + * Create a shared React context for Feature Flags. + * + * This will be used to propagate all available feature flags to + * Backstage components. This enables viewing all of the components. + */ +export interface FeatureFlagsEntry { + name: FeatureFlagName; +} + +export const FeatureFlagsContext = createContext<{ + registeredFeatureFlags: FeatureFlagsEntry[]; +}>({ + registeredFeatureFlags: [], +}); + +interface Props { + registeredFeatureFlags: FeatureFlagsEntry[]; + children: ComponentType; +} + +export const FeatureFlagsContextProvider: FC = ({ + registeredFeatureFlags, + children, +}) => ( + +); diff --git a/packages/core/src/api/index.ts b/packages/core/src/api/index.ts index e60a7efa18..e1bca9814e 100644 --- a/packages/core/src/api/index.ts +++ b/packages/core/src/api/index.ts @@ -16,5 +16,5 @@ export * from './api'; export * from './apis'; -export { FeatureFlags } from './app/FeatureFlags'; +export { FeatureFlags, FeatureFlagsContext } from './app/FeatureFlags'; export { useApp } from './app/AppContext'; From 758c12101eaebe4dd71c3e600ccd298154954156 Mon Sep 17 00:00:00 2001 From: Bilawal Hameed Date: Thu, 26 Mar 2020 16:00:32 +0100 Subject: [PATCH 12/35] [core/api] added toggle method to FeatureFlags --- packages/core/src/api/apis/definitions/featureFlags.ts | 5 +++++ packages/core/src/api/app/FeatureFlags.tsx | 4 ++++ 2 files changed, 9 insertions(+) diff --git a/packages/core/src/api/apis/definitions/featureFlags.ts b/packages/core/src/api/apis/definitions/featureFlags.ts index c3656ac4cd..dbebec643b 100644 --- a/packages/core/src/api/apis/definitions/featureFlags.ts +++ b/packages/core/src/api/apis/definitions/featureFlags.ts @@ -45,6 +45,11 @@ export type FeatureFlagsApi = { * Disable an registered feature flag. */ disable(name: FeatureFlagName): void; + + /** + * Toggle an registered feature flag on or off. + */ + toggle(name: FeatureFlagName): void; }; export const featureFlagsApiRef = new ApiRef({ diff --git a/packages/core/src/api/app/FeatureFlags.tsx b/packages/core/src/api/app/FeatureFlags.tsx index a5b0c7eaed..55558bfc1a 100644 --- a/packages/core/src/api/app/FeatureFlags.tsx +++ b/packages/core/src/api/app/FeatureFlags.tsx @@ -74,6 +74,10 @@ export class FeatureFlags { flags.delete(name); this.saveUserEnabledFeatureFlags(flags); } + + static toggle(name: FeatureFlagName): void { + (this.getItem(name) ? this.disable : this.enable).bind(this)(name); + } } /** From 71072da838e4c47c32086ebfbc990f786c953c7a Mon Sep 17 00:00:00 2001 From: Bilawal Hameed Date: Thu, 26 Mar 2020 16:07:48 +0100 Subject: [PATCH 13/35] [docs] updated to reflect both useApi and FeatureFlags object --- docs/reference/createPlugin-feature-flags.md | 43 ++++++++++++++++++-- docs/reference/createPlugin.md | 4 +- 2 files changed, 41 insertions(+), 6 deletions(-) diff --git a/docs/reference/createPlugin-feature-flags.md b/docs/reference/createPlugin-feature-flags.md index 6c3a5f0bc5..6da8d6b084 100644 --- a/docs/reference/createPlugin-feature-flags.md +++ b/docs/reference/createPlugin-feature-flags.md @@ -8,7 +8,25 @@ export type FeatureFlagsHooks = { }; ``` -Then, later, if you want to check if the user has enabled them: +## Using with useApi + +This is the **recommended** way of using the API. It's officially supported by Backstage. To use it, you'll first need to register the `FeatureFlags` API via `ApiRegistry` in your `apis.ts` in your App: + +```tsx +import { + ApiHolder, + ApiRegistry, + featureFlagsApiRef, + FeatureFlags, +} from '@backstage/core'; + +const builder = ApiRegistry.builder(); +builder.add(featureFlagsApiRef, FeatureFlags); + +export default builder.build() as ApiHolder; +``` + +Then, later, you can directly use it via `useApi`: ```tsx import React, { FC } from 'react'; @@ -17,10 +35,27 @@ import { featureFlagsApiRef, useApi } from '@backstage/core'; const ExampleButton: FC<{}> = () => { const featureFlagsApi = useApi(featureFlagsApiRef); + const handleClick = () => featureFlagsApi.enable('enable-example-feature'); - const handleClick = () => { - featureFlagsApi.enable('enable-example-feature'); - }; + return ( + + ); +}; +``` + +## Using directly with the `FeatureFlags` object + +This is **only** for backwards-compatibility support for Spotify's internal Backstage plugins. This may be deprecated in the distant future so generally we advise against using it. + +```tsx +import React, { FC } from 'react'; +import { Button } from '@material-ui/core'; +import { FeatureFlags } from '@backstage/core'; + +const ExampleButton: FC<{}> = () => { + const handleClick = () => FeatureFlags.enable('enable-example-feature'); return ( diff --git a/plugins/welcome/src/plugin.ts b/plugins/welcome/src/plugin.ts index 20ec41d4c2..a54a19d4af 100644 --- a/plugins/welcome/src/plugin.ts +++ b/plugins/welcome/src/plugin.ts @@ -19,7 +19,9 @@ import WelcomePage from './components/WelcomePage'; export default createPlugin({ id: 'welcome', - register({ router }) { + register({ router, featureFlags }) { router.registerRoute('/', WelcomePage); + + featureFlags.registerFeatureFlag('enable-welcome-box'); }, }); From 4969c6046b21682d7e52184c60f65e1c6faf5804 Mon Sep 17 00:00:00 2001 From: Bilawal Hameed Date: Thu, 26 Mar 2020 16:34:12 +0100 Subject: [PATCH 15/35] [core/apis] moved away from ts decorators --- packages/core/src/api/app/FeatureFlags.tsx | 24 ++++++------ packages/core/src/testUtils/decorators.ts | 45 ---------------------- packages/core/tsconfig.json | 3 +- 3 files changed, 12 insertions(+), 60 deletions(-) delete mode 100644 packages/core/src/testUtils/decorators.ts diff --git a/packages/core/src/api/app/FeatureFlags.tsx b/packages/core/src/api/app/FeatureFlags.tsx index 55558bfc1a..6459b38f0d 100644 --- a/packages/core/src/api/app/FeatureFlags.tsx +++ b/packages/core/src/api/app/FeatureFlags.tsx @@ -17,15 +17,13 @@ import React, { ComponentType, createContext, FC } from 'react'; import { FeatureFlagName } from '../plugin/types'; import { FeatureFlagsApi } from '../apis/definitions/featureFlags'; -import { staticImplements } from '../../testUtils'; // TODO: figure out where to put implementations of APIs, both inside apps // but also in core/separate package. -@staticImplements() -export class FeatureFlags { - private static readonly localStorageKey = 'featureFlags'; +class FeatureFlagsImpl implements FeatureFlagsApi { + private readonly localStorageKey = 'featureFlags'; - private static getUserEnabledFeatureFlags(): Set { + private getUserEnabledFeatureFlags(): Set { if (!('localStorage' in window)) { throw new Error( 'Feature Flags are not supported on browsers without the Local Storage API', @@ -42,9 +40,7 @@ export class FeatureFlags { } } - private static saveUserEnabledFeatureFlags( - flags: Set, - ): void { + private saveUserEnabledFeatureFlags(flags: Set): void { if (!('localStorage' in window)) { throw new Error( 'Feature Flags are not supported on browsers without the Local Storage API', @@ -59,32 +55,34 @@ export class FeatureFlags { ); } - static getItem(name: FeatureFlagName): boolean { + getItem(name: FeatureFlagName): boolean { return this.getUserEnabledFeatureFlags().has(name); } - static enable(name: FeatureFlagName): void { + enable(name: FeatureFlagName): void { const flags = this.getUserEnabledFeatureFlags(); flags.add(name); this.saveUserEnabledFeatureFlags(flags); } - static disable(name: FeatureFlagName): void { + disable(name: FeatureFlagName): void { const flags = this.getUserEnabledFeatureFlags(); flags.delete(name); this.saveUserEnabledFeatureFlags(flags); } - static toggle(name: FeatureFlagName): void { + toggle(name: FeatureFlagName): void { (this.getItem(name) ? this.disable : this.enable).bind(this)(name); } } +export const FeatureFlags = new FeatureFlagsImpl(); + /** * Create a shared React context for Feature Flags. * * This will be used to propagate all available feature flags to - * Backstage components. This enables viewing all of the components. + * Backstage components. This enables viewing all of the available flags. */ export interface FeatureFlagsEntry { name: FeatureFlagName; diff --git a/packages/core/src/testUtils/decorators.ts b/packages/core/src/testUtils/decorators.ts deleted file mode 100644 index 2cea8901b4..0000000000 --- a/packages/core/src/testUtils/decorators.ts +++ /dev/null @@ -1,45 +0,0 @@ -/* - * Copyright 2020 Spotify AB - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -/** - * This utilizes an experimental TypeScript feature called decorators. - * This was originally added to allow us to expose a FeatureFlags API - * with static methods (for backwards compatibility primarily). It takes - * an existing interface and applies the types to static methods in classes. - * - * @see https://www.typescriptlang.org/docs/handbook/decorators.html - * @example - * interface StaticProps { - * append(name: string, extra: string): string; - * reverse(name: string): string; - * } - * - * @staticImplements() - * class StaticPropsClass { - * static append(name, extra) { - * return `${name}${extra}`; - * } - * - * static reverse(name) { - * return name.reverse(); - * } - * } - */ -export function staticImplements() { - return (constructor: U) => { - constructor; - }; -} diff --git a/packages/core/tsconfig.json b/packages/core/tsconfig.json index 4e024e548f..ad43a0b986 100644 --- a/packages/core/tsconfig.json +++ b/packages/core/tsconfig.json @@ -2,7 +2,6 @@ "extends": "../../tsconfig.json", "include": ["src"], "compilerOptions": { - "noImplicitAny": false, - "experimentalDecorators": true + "noImplicitAny": false } } From 0b49bad43d70a707bbf21731530fe9403f70e4df Mon Sep 17 00:00:00 2001 From: Bilawal Hameed Date: Thu, 26 Mar 2020 16:58:00 +0100 Subject: [PATCH 16/35] [core/api] switch getItem from bool to enum --- packages/core/src/api/apis/definitions/featureFlags.ts | 8 +++++++- packages/core/src/api/app/FeatureFlags.tsx | 9 ++++++--- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/packages/core/src/api/apis/definitions/featureFlags.ts b/packages/core/src/api/apis/definitions/featureFlags.ts index dbebec643b..5874bfb052 100644 --- a/packages/core/src/api/apis/definitions/featureFlags.ts +++ b/packages/core/src/api/apis/definitions/featureFlags.ts @@ -28,13 +28,19 @@ import { FeatureFlagName } from '../../plugin/types'; * or unstable upcoming features. Although there will be a common interface for users * to enable and disable feature flags, this API acts as another way to enable/disable. */ + +export enum FeatureFlagState { + NotEnabled = false, + Enabled = true, +} + export type FeatureFlagsApi = { /** * Get the current user's status of a Feature Flag * * @returns bool True if the current user has enabled the feature flag */ - getItem(name: FeatureFlagName): boolean; + getItem(name: FeatureFlagName): FeatureFlagState; /** * Enable an registered feature flag. diff --git a/packages/core/src/api/app/FeatureFlags.tsx b/packages/core/src/api/app/FeatureFlags.tsx index 6459b38f0d..8373e538c2 100644 --- a/packages/core/src/api/app/FeatureFlags.tsx +++ b/packages/core/src/api/app/FeatureFlags.tsx @@ -16,7 +16,10 @@ import React, { ComponentType, createContext, FC } from 'react'; import { FeatureFlagName } from '../plugin/types'; -import { FeatureFlagsApi } from '../apis/definitions/featureFlags'; +import { + FeatureFlagState, + FeatureFlagsApi, +} from '../apis/definitions/featureFlags'; // TODO: figure out where to put implementations of APIs, both inside apps // but also in core/separate package. @@ -55,8 +58,8 @@ class FeatureFlagsImpl implements FeatureFlagsApi { ); } - getItem(name: FeatureFlagName): boolean { - return this.getUserEnabledFeatureFlags().has(name); + getItem(name: FeatureFlagName): FeatureFlagState { + return this.getUserEnabledFeatureFlags().has(name) as FeatureFlagState; } enable(name: FeatureFlagName): void { From 92d091dec86d99c136a169ad40c2d1438f97a8a6 Mon Sep 17 00:00:00 2001 From: Bilawal Hameed Date: Thu, 26 Mar 2020 17:05:53 +0100 Subject: [PATCH 17/35] [core/apis] consolidated methods to get/set --- docs/reference/createPlugin-feature-flags.md | 4 +-- .../src/api/apis/definitions/featureFlags.ts | 16 ++------- packages/core/src/api/app/FeatureFlags.tsx | 35 +++++-------------- .../components/WelcomePage/WelcomePage.tsx | 8 +++-- 4 files changed, 19 insertions(+), 44 deletions(-) diff --git a/docs/reference/createPlugin-feature-flags.md b/docs/reference/createPlugin-feature-flags.md index 6da8d6b084..8226efc129 100644 --- a/docs/reference/createPlugin-feature-flags.md +++ b/docs/reference/createPlugin-feature-flags.md @@ -35,7 +35,7 @@ import { featureFlagsApiRef, useApi } from '@backstage/core'; const ExampleButton: FC<{}> = () => { const featureFlagsApi = useApi(featureFlagsApiRef); - const handleClick = () => featureFlagsApi.enable('enable-example-feature'); + const handleClick = () => featureFlagsApi.set('enable-example-feature', .Enabled); return ( + ); +}; + +export default ToggleFeatureFlagButton; diff --git a/plugins/welcome/src/components/WelcomePage/WelcomePage.tsx b/plugins/welcome/src/components/WelcomePage/WelcomePage.tsx index 7d64b1470a..597ef5044b 100644 --- a/plugins/welcome/src/components/WelcomePage/WelcomePage.tsx +++ b/plugins/welcome/src/components/WelcomePage/WelcomePage.tsx @@ -23,7 +23,6 @@ import { ListItem, ListItemText, Link, - Button, } from '@material-ui/core'; import Timer from '../Timer'; import { @@ -34,13 +33,11 @@ import { pageTheme, ContentHeader, SupportButton, - featureFlagsApiRef, - useApi, } from '@backstage/core'; import ErrorButton from './ErrorButton'; +import ToggleFeatureFlagButton from './ToggleFeatureFlagButton'; const WelcomePage: FC<{}> = () => { - const featureFlagsApi = useApi(featureFlagsApiRef); const profile = { givenName: '' }; return ( @@ -127,22 +124,7 @@ const WelcomePage: FC<{}> = () => {

- + From abe68bc55b9561f32f0812fad92de2dfc33fc077 Mon Sep 17 00:00:00 2001 From: Bilawal Hameed Date: Thu, 26 Mar 2020 21:21:41 +0100 Subject: [PATCH 20/35] [core] fixed typescript issues --- .../src/api/apis/definitions/featureFlags.ts | 4 ++-- packages/core/src/api/app/AppBuilder.tsx | 2 +- packages/core/src/api/app/FeatureFlags.tsx | 16 +++++++++++----- packages/core/src/api/plugin/Plugin.tsx | 4 ++++ packages/core/src/testUtils/index.js | 1 - 5 files changed, 18 insertions(+), 9 deletions(-) diff --git a/packages/core/src/api/apis/definitions/featureFlags.ts b/packages/core/src/api/apis/definitions/featureFlags.ts index 750798d4b1..952e2b0121 100644 --- a/packages/core/src/api/apis/definitions/featureFlags.ts +++ b/packages/core/src/api/apis/definitions/featureFlags.ts @@ -30,8 +30,8 @@ import { FeatureFlagName } from '../../plugin/types'; */ export enum FeatureFlagState { - NotEnabled = false, - Enabled = true, + NotEnabled = 0, + Enabled = 1, } export type FeatureFlagsApi = { diff --git a/packages/core/src/api/app/AppBuilder.tsx b/packages/core/src/api/app/AppBuilder.tsx index c8a56876c9..ccd9982249 100644 --- a/packages/core/src/api/app/AppBuilder.tsx +++ b/packages/core/src/api/app/AppBuilder.tsx @@ -91,7 +91,7 @@ export default class AppBuilder { } case 'feature-flag': { registeredFeatureFlags.push({ - pluginId: plugin.config.id, + pluginId: plugin.getId(), name: output.name, }); break; diff --git a/packages/core/src/api/app/FeatureFlags.tsx b/packages/core/src/api/app/FeatureFlags.tsx index 708f4f50fc..74060beb6d 100644 --- a/packages/core/src/api/app/FeatureFlags.tsx +++ b/packages/core/src/api/app/FeatureFlags.tsx @@ -14,7 +14,7 @@ * limitations under the License. */ -import React, { ComponentType, createContext, FC } from 'react'; +import React, { ReactNode, createContext, FC } from 'react'; import { FeatureFlagName } from '../plugin/types'; import { FeatureFlagState, @@ -37,14 +37,20 @@ class FeatureFlagsImpl implements FeatureFlagsApi { const featureFlagsJson = window.localStorage.getItem( this.localStorageKey, ); - return new Set<>(Object.keys(JSON.parse(featureFlagsJson!))); + return new Set( + Object.keys(JSON.parse(featureFlagsJson!)), + ); } catch (err) { - return new Set<>(); + return new Set(); } } get(name: FeatureFlagName): FeatureFlagState { - return this.getUserEnabledFeatureFlags().has(name) as FeatureFlagState; + if (this.getUserEnabledFeatureFlags().has(name)) { + return FeatureFlagState.Enabled; + } + + return FeatureFlagState.NotEnabled; } set(name: FeatureFlagName, state: FeatureFlagState): void { @@ -83,7 +89,7 @@ export const FeatureFlagsContext = createContext<{ interface Props { registeredFeatureFlags: FeatureFlagsEntry[]; - children: ComponentType; + children: ReactNode; } export const FeatureFlagsContextProvider: FC = ({ diff --git a/packages/core/src/api/plugin/Plugin.tsx b/packages/core/src/api/plugin/Plugin.tsx index aa1ff417b7..ddb95bfff8 100644 --- a/packages/core/src/api/plugin/Plugin.tsx +++ b/packages/core/src/api/plugin/Plugin.tsx @@ -64,6 +64,10 @@ export default class Plugin { constructor(private readonly config: PluginConfig) {} + getId(): string { + return this.config.id; + } + output(): PluginOutput[] { if (this.storedOutput) { return this.storedOutput; diff --git a/packages/core/src/testUtils/index.js b/packages/core/src/testUtils/index.js index 902ba5d92d..a0eab3e72b 100644 --- a/packages/core/src/testUtils/index.js +++ b/packages/core/src/testUtils/index.js @@ -28,7 +28,6 @@ import { render } from '@testing-library/react'; export { default as Keyboard } from './Keyboard'; export { default as mockBreakpoint } from './mockBreakpoint'; export * from './logCollector'; -export * from './decorators'; export function wrapInTestApp(Component, initialRouterEntries) { const Wrapper = Component instanceof Function ? Component : () => Component; From 4e38f03221cb4247d302645e35e9e325484a2ef4 Mon Sep 17 00:00:00 2001 From: Bilawal Hameed Date: Fri, 27 Mar 2020 11:09:07 +0100 Subject: [PATCH 21/35] [core/api] added tests for FeatureFlags.tsx --- .../core/src/api/app/FeatureFlags.test.tsx | 155 ++++++++++++++++++ packages/core/src/api/app/FeatureFlags.tsx | 30 +++- 2 files changed, 183 insertions(+), 2 deletions(-) create mode 100644 packages/core/src/api/app/FeatureFlags.test.tsx diff --git a/packages/core/src/api/app/FeatureFlags.test.tsx b/packages/core/src/api/app/FeatureFlags.test.tsx new file mode 100644 index 0000000000..2159d05e65 --- /dev/null +++ b/packages/core/src/api/app/FeatureFlags.test.tsx @@ -0,0 +1,155 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import React, { useContext } from 'react'; +import { render } from '@testing-library/react'; +import { + FeatureFlags, + FeatureFlagsContext, + FeatureFlagsContextProvider, +} from './FeatureFlags'; +import { FeatureFlagState } from '../apis/definitions/featureFlags'; + +describe('FeatureFlags', () => { + beforeEach(() => { + window.localStorage.clear(); + }); + + describe('#get', () => { + it('defaults to .NotEnabled', () => { + expect(FeatureFlags.get('enable-feature-flag')).toBe( + FeatureFlagState.NotEnabled, + ); + }); + + it('returns a .Enabled state', () => { + FeatureFlags.set('enable-feature-flag', FeatureFlagState.Enabled); + expect(FeatureFlags.get('enable-feature-flag')).toBe( + FeatureFlagState.Enabled, + ); + }); + + it('returns a .NotEnabled state', () => { + FeatureFlags.set('enable-feature-flag', FeatureFlagState.NotEnabled); + expect(FeatureFlags.get('enable-feature-flag')).toBe( + FeatureFlagState.NotEnabled, + ); + }); + }); + + describe('#set', () => { + it('fails if name is less than three characters', () => { + expect(() => { + FeatureFlags.set('ab', FeatureFlagState.NotEnabled); + }).toThrow(/minimum length of three characters/i); + }); + + it('fails if name is greater than 150 characters', () => { + expect(() => { + FeatureFlags.set( + 'loremipsumdolorsitametconsecteturadipiscingelitnuncvitaeportaexaullamcorperturpismaurisutmattisnequemorbisediaculisauguevivamuspulvinarcursuseratblandithendreritquisqueuttinciduntmagnavestibulumblanditaugueat', + FeatureFlagState.NotEnabled, + ); + }).toThrow(/not exceed 150 characters/i); + }); + + it('fails if name does not start with a lowercase letter', () => { + expect(() => { + FeatureFlags.set('123456789', FeatureFlagState.NotEnabled); + }).toThrow(/start with a lowercase letter/i); + }); + + it('fails if name contains characters other than lowercase letters, numbers and hyphens', () => { + expect(() => { + FeatureFlags.set('Invalid_Feature_Flag', FeatureFlagState.NotEnabled); + }).toThrow(/only contain lowercase letters, numbers and hyphens/i); + }); + + it('fails if state is not recognized from FeatureFlagState', () => { + expect(() => { + // @ts-ignore + FeatureFlags.set('valid-feature-flag', 'invalid state'); + }).toThrow(/requires a recognized value from the FeatureFlagState/i); + }); + + it('sets a feature flag to enabled', () => { + expect(window.localStorage.featureFlags).toBeUndefined(); + FeatureFlags.set('valid-feature-flag', FeatureFlagState.Enabled); + expect(window.localStorage.featureFlags).toBe( + '{"valid-feature-flag":true}', + ); + }); + + it('sets a feature flag to disabled', () => { + expect(window.localStorage.featureFlags).toBeUndefined(); + FeatureFlags.set('valid-feature-flag', FeatureFlagState.NotEnabled); + expect(window.localStorage.featureFlags).toBe('{}'); + }); + + it('sets a feature flag to disabled then enabled', () => { + expect(window.localStorage.featureFlags).toBeUndefined(); + FeatureFlags.set('valid-feature-flag', FeatureFlagState.NotEnabled); + FeatureFlags.set('valid-feature-flag', FeatureFlagState.Enabled); + expect(window.localStorage.featureFlags).toBe( + '{"valid-feature-flag":true}', + ); + }); + + it('sets multiple feature flags', () => { + expect(window.localStorage.featureFlags).toBeUndefined(); + FeatureFlags.set('valid-feature-flag', FeatureFlagState.Enabled); + FeatureFlags.set('another-valid-feature-flag', FeatureFlagState.Enabled); + expect(window.localStorage.featureFlags).toBe( + '{"valid-feature-flag":true,"another-valid-feature-flag":true}', + ); + }); + }); +}); + +describe('FeatureFlagsContext', () => { + it('returns an empty list without the context', () => { + expect.assertions(1); + + const Component = () => { + const { registeredFeatureFlags } = useContext(FeatureFlagsContext); + expect(registeredFeatureFlags).toEqual([]); + return null; + }; + + render(); + }); + + it('returns a list of registered feature flags', () => { + expect.assertions(1); + + const mockFeatureFlags = [ + { name: 'feature-flag-one', pluginId: 'plugin-one' }, + { name: 'feature-flag-two', pluginId: 'plugin-two' }, + ]; + + const Component = () => { + const { registeredFeatureFlags } = useContext(FeatureFlagsContext); + expect(registeredFeatureFlags).toBe(mockFeatureFlags); + return null; + }; + + render( + + + , + ); + }); +}); diff --git a/packages/core/src/api/app/FeatureFlags.tsx b/packages/core/src/api/app/FeatureFlags.tsx index 74060beb6d..2b543db216 100644 --- a/packages/core/src/api/app/FeatureFlags.tsx +++ b/packages/core/src/api/app/FeatureFlags.tsx @@ -56,8 +56,34 @@ class FeatureFlagsImpl implements FeatureFlagsApi { set(name: FeatureFlagName, state: FeatureFlagState): void { const flags = this.getUserEnabledFeatureFlags(); - if (state === FeatureFlagState.NotEnabled) flags.delete(name); - if (state === FeatureFlagState.Enabled) flags.add(name); + if (name.length < 3) { + throw new Error( + 'The `name` argument must have a minimum length of three characters.', + ); + } + + if (name.length > 150) { + throw new Error('The `name` argument must not exceed 150 characters.'); + } + + if (!name.match(/^[a-z]+[a-z0-9-]+$/)) { + throw new Error( + 'The `name` argument must start with a lowercase letter and only contain lowercase letters, numbers and hyphens.' + + 'Examples: feature-flag-one, alpha, release-2020', + ); + } + + if (state === FeatureFlagState.Enabled) { + flags.add(name); + } else if (state === FeatureFlagState.NotEnabled) { + flags.delete(name); + } else { + throw new Error( + 'The `state` argument requires a recognized value from the FeatureFlagState enum. ' + + 'Please check the Backstage documentation to see all the available options.' + + 'Example values: FeatureFlagState.NotEnabled, FeatureFlagState.Enabled', + ); + } window.localStorage.setItem( this.localStorageKey, From 24f987b29b1340389bc322e50c72e9e95f4203c6 Mon Sep 17 00:00:00 2001 From: Bilawal Hameed Date: Fri, 27 Mar 2020 11:35:38 +0100 Subject: [PATCH 22/35] [core/api/app] split name validation into function --- .../src/api/apis/definitions/featureFlags.ts | 8 ++++ packages/core/src/api/app/FeatureFlags.tsx | 43 ++++++++++++------- 2 files changed, 36 insertions(+), 15 deletions(-) diff --git a/packages/core/src/api/apis/definitions/featureFlags.ts b/packages/core/src/api/apis/definitions/featureFlags.ts index 952e2b0121..15c9ecb084 100644 --- a/packages/core/src/api/apis/definitions/featureFlags.ts +++ b/packages/core/src/api/apis/definitions/featureFlags.ts @@ -35,6 +35,14 @@ export enum FeatureFlagState { } export type FeatureFlagsApi = { + /** + * Check the feature flag name convention. Used in the + * `registerFeatureFlag` method as well as in the `set` method. + * + * @returns string[] errors List of errors as string. Empty array if no errors. + */ + checkFeatureFlagNameErrors(name: FeatureFlagName): string[]; + /** * Get the current user's status of a Feature Flag * diff --git a/packages/core/src/api/app/FeatureFlags.tsx b/packages/core/src/api/app/FeatureFlags.tsx index 2b543db216..8d3d366906 100644 --- a/packages/core/src/api/app/FeatureFlags.tsx +++ b/packages/core/src/api/app/FeatureFlags.tsx @@ -45,6 +45,31 @@ class FeatureFlagsImpl implements FeatureFlagsApi { } } + // We don't make this private as we need this to validate + // in the `registerFeatureFlag` method in the Plugin API. + checkFeatureFlagNameErrors(name: FeatureFlagName): string[] { + const errors = []; + + if (name.length < 3) { + errors.push( + 'The `name` argument must have a minimum length of three characters.', + ); + } + + if (name.length > 150) { + errors.push('The `name` argument must not exceed 150 characters.'); + } + + if (!name.match(/^[a-z]+[a-z0-9-]+$/)) { + errors.push( + 'The `name` argument must start with a lowercase letter and only contain lowercase letters, numbers and hyphens.' + + 'Examples: feature-flag-one, alpha, release-2020', + ); + } + + return errors; + } + get(name: FeatureFlagName): FeatureFlagState { if (this.getUserEnabledFeatureFlags().has(name)) { return FeatureFlagState.Enabled; @@ -54,23 +79,11 @@ class FeatureFlagsImpl implements FeatureFlagsApi { } set(name: FeatureFlagName, state: FeatureFlagState): void { + const errors = this.checkFeatureFlagNameErrors(name); const flags = this.getUserEnabledFeatureFlags(); - if (name.length < 3) { - throw new Error( - 'The `name` argument must have a minimum length of three characters.', - ); - } - - if (name.length > 150) { - throw new Error('The `name` argument must not exceed 150 characters.'); - } - - if (!name.match(/^[a-z]+[a-z0-9-]+$/)) { - throw new Error( - 'The `name` argument must start with a lowercase letter and only contain lowercase letters, numbers and hyphens.' + - 'Examples: feature-flag-one, alpha, release-2020', - ); + if (errors.length > 0) { + throw new Error(errors[0]); } if (state === FeatureFlagState.Enabled) { From 9aaad5d6608c8cb9f588d2ad85f8ae057de7a167 Mon Sep 17 00:00:00 2001 From: Bilawal Hameed Date: Fri, 27 Mar 2020 11:52:14 +0100 Subject: [PATCH 23/35] [core/api] add name validation in registerFeatureFlag --- packages/core/src/api/app/FeatureFlags.tsx | 2 +- packages/core/src/api/plugin/Plugin.tsx | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/packages/core/src/api/app/FeatureFlags.tsx b/packages/core/src/api/app/FeatureFlags.tsx index 8d3d366906..b8799e5a71 100644 --- a/packages/core/src/api/app/FeatureFlags.tsx +++ b/packages/core/src/api/app/FeatureFlags.tsx @@ -62,7 +62,7 @@ class FeatureFlagsImpl implements FeatureFlagsApi { if (!name.match(/^[a-z]+[a-z0-9-]+$/)) { errors.push( - 'The `name` argument must start with a lowercase letter and only contain lowercase letters, numbers and hyphens.' + + 'The `name` argument must start with a lowercase letter and only contain lowercase letters, numbers and hyphens. ' + 'Examples: feature-flag-one, alpha, release-2020', ); } diff --git a/packages/core/src/api/plugin/Plugin.tsx b/packages/core/src/api/plugin/Plugin.tsx index ddb95bfff8..d342cb2429 100644 --- a/packages/core/src/api/plugin/Plugin.tsx +++ b/packages/core/src/api/plugin/Plugin.tsx @@ -21,6 +21,7 @@ import { RouteOptions, FeatureFlagName, } from './types'; +import { FeatureFlags } from '../app/FeatureFlags'; import { Widget } from '../widgetView/types'; export type PluginConfig = { @@ -76,6 +77,7 @@ export default class Plugin { return []; } + const pluginId = this.getId(); const outputs = new Array(); this.config.register({ @@ -94,6 +96,14 @@ export default class Plugin { }, featureFlags: { registerFeatureFlag(name) { + const errors = FeatureFlags.checkFeatureFlagNameErrors(name); + + if (errors.length > 0) { + throw new Error( + `Failed to register '${name}' feature flag in '${pluginId}' plugin: ${errors[0]}`, + ); + } + outputs.push({ type: 'feature-flag', name }); }, }, From 5157904031b5101aa46d87f5615faeae575f595e Mon Sep 17 00:00:00 2001 From: Bilawal Hameed Date: Fri, 27 Mar 2020 12:15:50 +0100 Subject: [PATCH 24/35] [core/api] s/registeredFeatureFlags/featureFlags/g --- packages/core/src/api/app/AppBuilder.tsx | 2 +- .../core/src/api/app/FeatureFlags.test.tsx | 10 +-- packages/core/src/api/app/FeatureFlags.tsx | 78 +++++++++++-------- 3 files changed, 51 insertions(+), 39 deletions(-) diff --git a/packages/core/src/api/app/AppBuilder.tsx b/packages/core/src/api/app/AppBuilder.tsx index ccd9982249..84317a735a 100644 --- a/packages/core/src/api/app/AppBuilder.tsx +++ b/packages/core/src/api/app/AppBuilder.tsx @@ -119,7 +119,7 @@ export default class AppBuilder { rendered = ( ); diff --git a/packages/core/src/api/app/FeatureFlags.test.tsx b/packages/core/src/api/app/FeatureFlags.test.tsx index 2159d05e65..3e3bb3e50d 100644 --- a/packages/core/src/api/app/FeatureFlags.test.tsx +++ b/packages/core/src/api/app/FeatureFlags.test.tsx @@ -124,8 +124,8 @@ describe('FeatureFlagsContext', () => { expect.assertions(1); const Component = () => { - const { registeredFeatureFlags } = useContext(FeatureFlagsContext); - expect(registeredFeatureFlags).toEqual([]); + const { featureFlags } = useContext(FeatureFlagsContext); + expect(featureFlags).toEqual([]); return null; }; @@ -141,13 +141,13 @@ describe('FeatureFlagsContext', () => { ]; const Component = () => { - const { registeredFeatureFlags } = useContext(FeatureFlagsContext); - expect(registeredFeatureFlags).toBe(mockFeatureFlags); + const { featureFlags } = useContext(FeatureFlagsContext); + expect(featureFlags).toBe(mockFeatureFlags); return null; }; render( - + , ); diff --git a/packages/core/src/api/app/FeatureFlags.tsx b/packages/core/src/api/app/FeatureFlags.tsx index b8799e5a71..9921486999 100644 --- a/packages/core/src/api/app/FeatureFlags.tsx +++ b/packages/core/src/api/app/FeatureFlags.tsx @@ -14,13 +14,57 @@ * limitations under the License. */ -import React, { ReactNode, createContext, FC } from 'react'; +import React, { + ReactNode, + createContext, + useContext, + useState, + FC, +} from 'react'; import { FeatureFlagName } from '../plugin/types'; import { FeatureFlagState, FeatureFlagsApi, } from '../apis/definitions/featureFlags'; +/** + * Create a shared React context for Feature Flags. + * + * This will be used to propagate all available feature flags to + * Backstage components. This enables viewing all of the available flags. + */ +export interface FeatureFlagsEntry { + pluginId: string; + name: FeatureFlagName; + userEnabled: boolean; +} + +export interface IFeatureFlagsContext { + featureFlags: FeatureFlagsEntry[]; +} + +export const FeatureFlagsContext = createContext({ + featureFlags: [], +}); + +export const FeatureFlagsContextProvider: FC<{ + featureFlags: FeatureFlagsEntry[]; + children: ReactNode; +}> = ({ featureFlags, children }) => { + const [userEnabledFlags, setUserEnabledFlags] = useState([]); + + return ( + + ); +}; + +/** + * Create the FeatureFlags implementation based on the API. + */ + // TODO: figure out where to put implementations of APIs, both inside apps // but also in core/separate package. class FeatureFlagsImpl implements FeatureFlagsApi { @@ -108,35 +152,3 @@ class FeatureFlagsImpl implements FeatureFlagsApi { } export const FeatureFlags = new FeatureFlagsImpl(); - -/** - * Create a shared React context for Feature Flags. - * - * This will be used to propagate all available feature flags to - * Backstage components. This enables viewing all of the available flags. - */ -export interface FeatureFlagsEntry { - pluginId: string; - name: FeatureFlagName; -} - -export const FeatureFlagsContext = createContext<{ - registeredFeatureFlags: FeatureFlagsEntry[]; -}>({ - registeredFeatureFlags: [], -}); - -interface Props { - registeredFeatureFlags: FeatureFlagsEntry[]; - children: ReactNode; -} - -export const FeatureFlagsContextProvider: FC = ({ - registeredFeatureFlags, - children, -}) => ( - -); From c1f5b2abc9e2a2a5284407c87d7f234481b892c3 Mon Sep 17 00:00:00 2001 From: Bilawal Hameed Date: Fri, 27 Mar 2020 12:49:48 +0100 Subject: [PATCH 25/35] [core/apis] switch to useFeatureFlag React Hook --- .../src/api/apis/definitions/featureFlags.ts | 25 ++-- packages/core/src/api/app/AppBuilder.tsx | 4 +- packages/core/src/api/app/FeatureFlags.tsx | 132 ++++++++++++++---- .../WelcomePage/ToggleFeatureFlagButton.tsx | 19 ++- 4 files changed, 126 insertions(+), 54 deletions(-) diff --git a/packages/core/src/api/apis/definitions/featureFlags.ts b/packages/core/src/api/apis/definitions/featureFlags.ts index 15c9ecb084..e3eaeefdfb 100644 --- a/packages/core/src/api/apis/definitions/featureFlags.ts +++ b/packages/core/src/api/apis/definitions/featureFlags.ts @@ -36,24 +36,27 @@ export enum FeatureFlagState { export type FeatureFlagsApi = { /** - * Check the feature flag name convention. Used in the - * `registerFeatureFlag` method as well as in the `set` method. + * Get a list of the user's currently enabled feature flags. + * Reads directly from window.localStorage Browser API. + * + * @returns string[] enabledFeatureFlags List of feature flags enabled by the current user + */ + getEnabledFeatureFlags(): Set; + + /** + * Check the feature flag name convention. + * Used in the `registerFeatureFlag` method as well as in the `set` method. * * @returns string[] errors List of errors as string. Empty array if no errors. */ checkFeatureFlagNameErrors(name: FeatureFlagName): string[]; /** - * Get the current user's status of a Feature Flag - * - * @returns bool True if the current user has enabled the feature flag + * Use Feature Flags as a React Hook. */ - get(name: FeatureFlagName): FeatureFlagState; - - /** - * Set the state of a Feature Flag - */ - set(name: FeatureFlagName, state: FeatureFlagState): void; + useFeatureFlag( + name: FeatureFlagName, + ): [FeatureFlagState, (state: FeatureFlagState) => void]; }; export const featureFlagsApiRef = new ApiRef({ diff --git a/packages/core/src/api/app/AppBuilder.tsx b/packages/core/src/api/app/AppBuilder.tsx index 84317a735a..bd40565260 100644 --- a/packages/core/src/api/app/AppBuilder.tsx +++ b/packages/core/src/api/app/AppBuilder.tsx @@ -63,7 +63,7 @@ export default class AppBuilder { const app = new AppImpl(this.systemIcons); const routes = new Array(); - const registeredFeatureFlags = new Array(); + const registeredFeatureFlags = new Set(); for (const plugin of this.plugins.values()) { for (const output of plugin.output()) { @@ -90,7 +90,7 @@ export default class AppBuilder { break; } case 'feature-flag': { - registeredFeatureFlags.push({ + registeredFeatureFlags.add({ pluginId: plugin.getId(), name: output.name, }); diff --git a/packages/core/src/api/app/FeatureFlags.tsx b/packages/core/src/api/app/FeatureFlags.tsx index 9921486999..e5a2063621 100644 --- a/packages/core/src/api/app/FeatureFlags.tsx +++ b/packages/core/src/api/app/FeatureFlags.tsx @@ -19,6 +19,7 @@ import React, { createContext, useContext, useState, + useEffect, FC, } from 'react'; import { FeatureFlagName } from '../plugin/types'; @@ -36,26 +37,50 @@ import { export interface FeatureFlagsEntry { pluginId: string; name: FeatureFlagName; - userEnabled: boolean; } export interface IFeatureFlagsContext { - featureFlags: FeatureFlagsEntry[]; + featureFlags: Set; + enabledFeatureFlags: Set; + refreshEnabledFeatureFlags: () => void; } export const FeatureFlagsContext = createContext({ - featureFlags: [], + featureFlags: new Set(), + enabledFeatureFlags: new Set(), + refreshEnabledFeatureFlags: () => { + throw new Error( + 'The refreshEnabledFeatureFlags method is not implemented as it is not called from within the FeatureFlagsContext context within React. ' + + 'See the Backstage documentation for examples on how to use the Feature Flags API.', + ); + }, }); export const FeatureFlagsContextProvider: FC<{ - featureFlags: FeatureFlagsEntry[]; + featureFlags: Set; children: ReactNode; }> = ({ featureFlags, children }) => { - const [userEnabledFlags, setUserEnabledFlags] = useState([]); + const [enabledFeatureFlags, setEnabledFeatureFlags] = useState< + Set + >(new Set()); + + const refreshEnabledFeatureFlags = () => { + // eslint-disable-next-line no-use-before-define + setEnabledFeatureFlags(FeatureFlags.getEnabledFeatureFlags()); + }; + + // Initially populate our setEnabledFeatureFlags + useEffect(() => { + refreshEnabledFeatureFlags(); + }, []); return ( ); @@ -70,7 +95,46 @@ export const FeatureFlagsContextProvider: FC<{ class FeatureFlagsImpl implements FeatureFlagsApi { private readonly localStorageKey = 'featureFlags'; - private getUserEnabledFeatureFlags(): Set { + private get( + enabledFeatureFlags: Set, + name: FeatureFlagName, + ): FeatureFlagState { + if (enabledFeatureFlags.has(name)) { + return FeatureFlagState.Enabled; + } + + return FeatureFlagState.NotEnabled; + } + + private set(name: FeatureFlagName, state: FeatureFlagState): void { + const errors = this.checkFeatureFlagNameErrors(name); + const flags = this.getEnabledFeatureFlags(); + + if (errors.length > 0) { + throw new Error(errors[0]); + } + + if (state === FeatureFlagState.Enabled) { + flags.add(name); + } else if (state === FeatureFlagState.NotEnabled) { + flags.delete(name); + } else { + throw new Error( + 'The `state` argument requires a recognized value from the FeatureFlagState enum. ' + + 'Please check the Backstage documentation to see all the available options.' + + 'Example values: FeatureFlagState.NotEnabled, FeatureFlagState.Enabled', + ); + } + + window.localStorage.setItem( + this.localStorageKey, + JSON.stringify( + [...flags].reduce((list, flag) => ({ ...list, [flag]: true }), {}), + ), + ); + } + + getEnabledFeatureFlags(): Set { if (!('localStorage' in window)) { throw new Error( 'Feature Flags are not supported on browsers without the Local Storage API', @@ -114,40 +178,46 @@ class FeatureFlagsImpl implements FeatureFlagsApi { return errors; } - get(name: FeatureFlagName): FeatureFlagState { - if (this.getUserEnabledFeatureFlags().has(name)) { - return FeatureFlagState.Enabled; + useFeatureFlag( + name: FeatureFlagName, + ): [FeatureFlagState, (state: FeatureFlagState) => void] { + // Check for context + const context = useContext(FeatureFlagsContext); + if (!context) { + throw new Error( + 'No FeatureFlagsContext found. ' + + 'Please use this React Hook in the context of your ', + ); } - return FeatureFlagState.NotEnabled; - } - - set(name: FeatureFlagName, state: FeatureFlagState): void { - const errors = this.checkFeatureFlagNameErrors(name); - const flags = this.getUserEnabledFeatureFlags(); - + // Check for errors + // eslint-disable-next-line no-use-before-define + const errors = FeatureFlags.checkFeatureFlagNameErrors(name); if (errors.length > 0) { throw new Error(errors[0]); } - if (state === FeatureFlagState.Enabled) { - flags.add(name); - } else if (state === FeatureFlagState.NotEnabled) { - flags.delete(name); - } else { + // Check if the feature flag is registered + const allFlagNames = [...context.featureFlags].map(flag => flag.name); + if (!allFlagNames.includes(name)) { throw new Error( - 'The `state` argument requires a recognized value from the FeatureFlagState enum. ' + - 'Please check the Backstage documentation to see all the available options.' + - 'Example values: FeatureFlagState.NotEnabled, FeatureFlagState.Enabled', + `The '${name}' feature flag is not registered by any plugin. ` + + `See the 'registerFeatureFlag' method in the Plugin API (or in your plugin.ts file) on how to register Feature Flags.`, ); } - window.localStorage.setItem( - this.localStorageKey, - JSON.stringify( - [...flags].reduce((list, flag) => ({ ...list, [flag]: true }), {}), - ), - ); + // eslint-disable-next-line no-use-before-define + const currentState = FeatureFlags.get(context.enabledFeatureFlags, name); + const setState = (state: FeatureFlagState): void => { + // Set the value + // eslint-disable-next-line no-use-before-define + FeatureFlags.set(name, state); + + // Now update the global state + context.refreshEnabledFeatureFlags(); + }; + + return [currentState, setState]; } } diff --git a/plugins/welcome/src/components/WelcomePage/ToggleFeatureFlagButton.tsx b/plugins/welcome/src/components/WelcomePage/ToggleFeatureFlagButton.tsx index f9cf68f20c..a940b76263 100644 --- a/plugins/welcome/src/components/WelcomePage/ToggleFeatureFlagButton.tsx +++ b/plugins/welcome/src/components/WelcomePage/ToggleFeatureFlagButton.tsx @@ -19,15 +19,15 @@ import { Button } from '@material-ui/core'; import { FeatureFlagState, featureFlagsApiRef, useApi } from '@backstage/core'; const ToggleFeatureFlagButton: FC<{}> = () => { - const featureFlagsApi = useApi(featureFlagsApiRef); + const { useFeatureFlag } = useApi(featureFlagsApiRef); + const [flagState, setFlagState] = useFeatureFlag('enable-welcome-box'); const handleClick = () => { - const isEnabled = featureFlagsApi.get('enable-welcome-box'); - featureFlagsApi.set( - 'enable-welcome-box', - isEnabled ? FeatureFlagState.NotEnabled : FeatureFlagState.Enabled, - ); - window.location.reload(); + if (flagState === FeatureFlagState.Enabled) { + setFlagState(FeatureFlagState.NotEnabled); + } else { + setFlagState(FeatureFlagState.Enabled); + } }; return ( @@ -37,10 +37,9 @@ const ToggleFeatureFlagButton: FC<{}> = () => { onClick={handleClick} data-testid="button-switch-feature-flag-state" > - {featureFlagsApi.get('enable-welcome-box') === - FeatureFlagState.NotEnabled && + {flagState === FeatureFlagState.NotEnabled && 'Enable "enable-welcome-box" feature flag'} - {featureFlagsApi.get('enable-welcome-box') === FeatureFlagState.Enabled && + {flagState === FeatureFlagState.Enabled && 'Disable "enable-welcome-box" feature flag'} ); From e78680fe0340fdc984df797d290275c98f3d1c1e Mon Sep 17 00:00:00 2001 From: Bilawal Hameed Date: Fri, 27 Mar 2020 13:04:59 +0100 Subject: [PATCH 26/35] [plugins/welcome] fixed ToggleFeatureFlagButton test --- packages/core/src/api/index.ts | 6 +++- .../ToggleFeatureFlagButton.test.tsx | 35 +++++++++++++------ 2 files changed, 29 insertions(+), 12 deletions(-) diff --git a/packages/core/src/api/index.ts b/packages/core/src/api/index.ts index e1bca9814e..4c77e94984 100644 --- a/packages/core/src/api/index.ts +++ b/packages/core/src/api/index.ts @@ -16,5 +16,9 @@ export * from './api'; export * from './apis'; -export { FeatureFlags, FeatureFlagsContext } from './app/FeatureFlags'; +export { + FeatureFlags, + FeatureFlagsContext, + FeatureFlagsContextProvider, +} from './app/FeatureFlags'; export { useApp } from './app/AppContext'; diff --git a/plugins/welcome/src/components/WelcomePage/ToggleFeatureFlagButton.test.tsx b/plugins/welcome/src/components/WelcomePage/ToggleFeatureFlagButton.test.tsx index 8efb25f0a6..f9be280f50 100644 --- a/plugins/welcome/src/components/WelcomePage/ToggleFeatureFlagButton.test.tsx +++ b/plugins/welcome/src/components/WelcomePage/ToggleFeatureFlagButton.test.tsx @@ -14,7 +14,7 @@ * limitations under the License. */ -import React from 'react'; +import React, { ReactNode } from 'react'; import { render, fireEvent } from '@testing-library/react'; import ToggleFeatureFlagButton from './ToggleFeatureFlagButton'; import { @@ -22,8 +22,29 @@ import { featureFlagsApiRef, ApiProvider, FeatureFlags, + FeatureFlagsContextProvider, } from '@backstage/core'; +function withFeatureFlags(children: ReactNode) { + const featureFlags = new Set([ + { pluginId: 'welcome', name: 'enable-welcome-box' }, + ]); + + return ( + + {children} + + ); +} + +function withApiRegistry(children: ReactNode) { + return ( + + {children} + + ); +} + describe('ToggleFeatureFlagButton', () => { beforeEach(() => { window.localStorage.clear(); @@ -31,11 +52,7 @@ describe('ToggleFeatureFlagButton', () => { it('should enable the feature flag', () => { const rendered = render( - - - , + withFeatureFlags(withApiRegistry()), ); const button = rendered.getByTestId('button-switch-feature-flag-state'); @@ -50,11 +67,7 @@ describe('ToggleFeatureFlagButton', () => { it('should disable the feature flag', () => { const rendered = render( - - - , + withFeatureFlags(withApiRegistry()), ); const button = rendered.getByTestId('button-switch-feature-flag-state'); From a8f304dbfbb7ad96da67a8c490e64e80136b920d Mon Sep 17 00:00:00 2001 From: Bilawal Hameed Date: Fri, 27 Mar 2020 13:13:46 +0100 Subject: [PATCH 27/35] [docs/reference] updated to reflect React Hooks changes --- docs/reference/createPlugin-feature-flags.md | 44 +- .../core/src/api/app/FeatureFlags.test.tsx | 378 ++++++++++++++---- packages/core/src/api/app/FeatureFlags.tsx | 10 +- 3 files changed, 325 insertions(+), 107 deletions(-) diff --git a/docs/reference/createPlugin-feature-flags.md b/docs/reference/createPlugin-feature-flags.md index 8226efc129..bbf7f17c32 100644 --- a/docs/reference/createPlugin-feature-flags.md +++ b/docs/reference/createPlugin-feature-flags.md @@ -8,9 +8,23 @@ export type FeatureFlagsHooks = { }; ``` +Here's a code sample: + +```typescript +import { createPlugin } from '@backstage/core'; + +export default createPlugin({ + id: 'welcome', + register({ router, featureFlags }) { + // router.registerRoute('/', Component); + featureFlags.registerFeatureFlag('enable-example-feature'); + }, +}); +``` + ## Using with useApi -This is the **recommended** way of using the API. It's officially supported by Backstage. To use it, you'll first need to register the `FeatureFlags` API via `ApiRegistry` in your `apis.ts` in your App: +To use it, you'll first need to register the `FeatureFlags` API via `ApiRegistry` in your `apis.ts` in your App: ```tsx import { @@ -34,8 +48,12 @@ import { Button } from '@material-ui/core'; import { featureFlagsApiRef, useApi } from '@backstage/core'; const ExampleButton: FC<{}> = () => { - const featureFlagsApi = useApi(featureFlagsApiRef); - const handleClick = () => featureFlagsApi.set('enable-example-feature', .Enabled); + const { useFeatureFlag } = useApi(featureFlagsApiRef); + const [flagState, setFlagState] = useFeatureFlag('enable-example-feature'); + + const handleClick = () => { + setFlagState(FeatureFlagState.Enabled); + }; return ( - ); -}; -``` +Note that you must register it with `registerFeatureFlag` otherwise the `useFeatureFlag` will throw an error. [Back to References](README.md) diff --git a/packages/core/src/api/app/FeatureFlags.test.tsx b/packages/core/src/api/app/FeatureFlags.test.tsx index 3e3bb3e50d..b803b27b3a 100644 --- a/packages/core/src/api/app/FeatureFlags.test.tsx +++ b/packages/core/src/api/app/FeatureFlags.test.tsx @@ -14,142 +14,362 @@ * limitations under the License. */ -import React, { useContext } from 'react'; +import React, { ReactNode, useContext, useEffect, useRef } from 'react'; import { render } from '@testing-library/react'; import { FeatureFlags, + FeatureFlagsEntry, FeatureFlagsContext, FeatureFlagsContextProvider, } from './FeatureFlags'; import { FeatureFlagState } from '../apis/definitions/featureFlags'; +function useRenderCount() { + const renderCount = useRef(-1); + renderCount.current += 1; + return renderCount.current; +} + +function withFeatureFlags( + children: ReactNode, + featureFlags: Set = new Set([ + { name: 'feature-flag-one', pluginId: 'plugin-one' }, + { name: 'feature-flag-two', pluginId: 'plugin-two' }, + { name: 'feature-flag-three', pluginId: 'plugin-two' }, + ]), +) { + return ( + + {children} + + ); +} + describe('FeatureFlags', () => { beforeEach(() => { window.localStorage.clear(); }); - describe('#get', () => { - it('defaults to .NotEnabled', () => { - expect(FeatureFlags.get('enable-feature-flag')).toBe( - FeatureFlagState.NotEnabled, - ); + describe('#getEnabledFeatureFlags', () => { + it('returns an empty set', () => { + expect(FeatureFlags.getEnabledFeatureFlags()).toEqual(new Set()); }); - it('returns a .Enabled state', () => { - FeatureFlags.set('enable-feature-flag', FeatureFlagState.Enabled); - expect(FeatureFlags.get('enable-feature-flag')).toBe( - FeatureFlagState.Enabled, + it('returns enabled feature flags', () => { + window.localStorage.setItem( + 'featureFlags', + JSON.stringify({ + 'feature-flag-one': true, + 'feature-flag-three': true, + }), ); - }); - it('returns a .NotEnabled state', () => { - FeatureFlags.set('enable-feature-flag', FeatureFlagState.NotEnabled); - expect(FeatureFlags.get('enable-feature-flag')).toBe( - FeatureFlagState.NotEnabled, + expect(FeatureFlags.getEnabledFeatureFlags()).toEqual( + new Set(['feature-flag-one', 'feature-flag-three']), ); }); }); - describe('#set', () => { - it('fails if name is less than three characters', () => { - expect(() => { - FeatureFlags.set('ab', FeatureFlagState.NotEnabled); - }).toThrow(/minimum length of three characters/i); + describe('#checkFeatureFlagNameErrors', () => { + it('returns an error if less than three characters', () => { + const errors = FeatureFlags.checkFeatureFlagNameErrors('ab'); + expect(errors[0]).toMatch(/minimum length of three characters/i); }); - it('fails if name is greater than 150 characters', () => { - expect(() => { - FeatureFlags.set( - 'loremipsumdolorsitametconsecteturadipiscingelitnuncvitaeportaexaullamcorperturpismaurisutmattisnequemorbisediaculisauguevivamuspulvinarcursuseratblandithendreritquisqueuttinciduntmagnavestibulumblanditaugueat', - FeatureFlagState.NotEnabled, + it('returns an error if greater than 150 characters', () => { + const errors = FeatureFlags.checkFeatureFlagNameErrors( + 'loremipsumdolorsitametconsecteturadipiscingelitnuncvitaeportaexaullamcorperturpismaurisutmattisnequemorbisediaculisauguevivamuspulvinarcursuseratblandithendreritquisqueuttinciduntmagnavestibulumblanditaugueat', + ); + expect(errors[0]).toMatch(/not exceed 150 characters/i); + }); + + it('returns an error if name does not start with a lowercase letter', () => { + const errors = FeatureFlags.checkFeatureFlagNameErrors('123456789'); + expect(errors[0]).toMatch(/start with a lowercase letter/i); + }); + + it('returns an error if name contains characters other than lowercase letters, numbers and hyphens', () => { + const errors = FeatureFlags.checkFeatureFlagNameErrors( + 'Invalid_Feature_Flag', + ); + expect(errors[0]).toMatch( + /only contain lowercase letters, numbers and hyphens/i, + ); + }); + + it('returns no errors', () => { + const errors = FeatureFlags.checkFeatureFlagNameErrors( + 'valid-feature-flag', + ); + expect(errors.length).toBe(0); + }); + }); + + describe('#useFeatureFlag', () => { + it('throws an error if the feature flag is not registered', () => { + const Component = () => { + expect(() => { + FeatureFlags.useFeatureFlag('feature-flag-four'); + }).toThrow(/'feature-flag-four' feature flag is not registered/i); + + return null; + }; + + render(withFeatureFlags()); + }); + + it('throws an error if changing value is not recognized', () => { + const Component = () => { + const [, setState] = FeatureFlags.useFeatureFlag('feature-flag-one'); + const renderCount = useRenderCount(); + if (renderCount === 1) { + // @ts-ignore + expect(() => setState('not valid')).toThrow( + /requires a recognized value from the FeatureFlagState/i, + ); + } + return null; + }; + + render(withFeatureFlags()); + }); + + it('defaults to .NotEnabled', () => { + const Component = () => { + const renderCount = useRenderCount(); + const [state] = FeatureFlags.useFeatureFlag('feature-flag-one'); + if (renderCount === 1) { + expect(state).toEqual(FeatureFlagState.NotEnabled); + } + return null; + }; + + render(withFeatureFlags()); + }); + + it('returns an .Enabled state', () => { + window.localStorage.setItem('featureFlags', '{"feature-flag-one":true}'); + + const Component = () => { + const [state] = FeatureFlags.useFeatureFlag('feature-flag-one'); + const renderCount = useRenderCount(); + if (renderCount === 1) { + expect(state).toEqual(FeatureFlagState.Enabled); + } + return null; + }; + + render(withFeatureFlags()); + }); + + it('returns an .NotEnabled state', () => { + const Component = () => { + const [state] = FeatureFlags.useFeatureFlag('feature-flag-one'); + const renderCount = useRenderCount(); + if (renderCount === 1) { + expect(state).toEqual(FeatureFlagState.NotEnabled); + } + return null; + }; + + render(withFeatureFlags()); + }); + + it('changes state to .Enabled', () => { + const Component = () => { + const [state, setState] = FeatureFlags.useFeatureFlag( + 'feature-flag-one', ); - }).toThrow(/not exceed 150 characters/i); + const renderCount = useRenderCount(); + if (renderCount === 1) setState(FeatureFlagState.Enabled); + if (renderCount === 2) expect(state).toEqual(FeatureFlagState.Enabled); + return null; + }; + + render(withFeatureFlags()); }); - it('fails if name does not start with a lowercase letter', () => { - expect(() => { - FeatureFlags.set('123456789', FeatureFlagState.NotEnabled); - }).toThrow(/start with a lowercase letter/i); + it('changes state to .NotEnabled', () => { + window.localStorage.setItem('featureFlags', '{"feature-flag-one":true}'); + + const Component = () => { + const [state, setState] = FeatureFlags.useFeatureFlag( + 'feature-flag-one', + ); + const renderCount = useRenderCount(); + if (renderCount === 1) setState(FeatureFlagState.NotEnabled); + if (renderCount === 2) + expect(state).toEqual(FeatureFlagState.NotEnabled); + return null; + }; + + render(withFeatureFlags()); }); - it('fails if name contains characters other than lowercase letters, numbers and hyphens', () => { - expect(() => { - FeatureFlags.set('Invalid_Feature_Flag', FeatureFlagState.NotEnabled); - }).toThrow(/only contain lowercase letters, numbers and hyphens/i); + it('changes state to .Enabled then .NotEnabled', () => { + const Component = () => { + const [state, setState] = FeatureFlags.useFeatureFlag( + 'feature-flag-one', + ); + const renderCount = useRenderCount(); + if (renderCount === 1) setState(FeatureFlagState.Enabled); + if (renderCount === 2) setState(FeatureFlagState.NotEnabled); + if (renderCount === 3) + expect(state).toEqual(FeatureFlagState.NotEnabled); + return null; + }; + + render(withFeatureFlags()); }); - it('fails if state is not recognized from FeatureFlagState', () => { - expect(() => { - // @ts-ignore - FeatureFlags.set('valid-feature-flag', 'invalid state'); - }).toThrow(/requires a recognized value from the FeatureFlagState/i); - }); + it('changes multiple feature flag states', () => { + expect.assertions(3); - it('sets a feature flag to enabled', () => { - expect(window.localStorage.featureFlags).toBeUndefined(); - FeatureFlags.set('valid-feature-flag', FeatureFlagState.Enabled); - expect(window.localStorage.featureFlags).toBe( - '{"valid-feature-flag":true}', - ); - }); + const Component = () => { + const renderCount = useRenderCount(); + const [stateA, setStateA] = FeatureFlags.useFeatureFlag( + 'feature-flag-one', + ); + const [stateB, setStateB] = FeatureFlags.useFeatureFlag( + 'feature-flag-two', + ); - it('sets a feature flag to disabled', () => { - expect(window.localStorage.featureFlags).toBeUndefined(); - FeatureFlags.set('valid-feature-flag', FeatureFlagState.NotEnabled); - expect(window.localStorage.featureFlags).toBe('{}'); - }); + useEffect(() => { + if (renderCount === 1) { + setStateA(FeatureFlagState.Enabled); + setStateB(FeatureFlagState.Enabled); + } + if (renderCount === 2) { + expect(stateA).toEqual(FeatureFlagState.Enabled); + expect(stateB).toEqual(FeatureFlagState.Enabled); + expect(window.localStorage.getItem('featureFlags')).toEqual( + '{"feature-flag-one":true,"feature-flag-two":true}', + ); + } + }, [renderCount]); - it('sets a feature flag to disabled then enabled', () => { - expect(window.localStorage.featureFlags).toBeUndefined(); - FeatureFlags.set('valid-feature-flag', FeatureFlagState.NotEnabled); - FeatureFlags.set('valid-feature-flag', FeatureFlagState.Enabled); - expect(window.localStorage.featureFlags).toBe( - '{"valid-feature-flag":true}', - ); - }); + return null; + }; - it('sets multiple feature flags', () => { - expect(window.localStorage.featureFlags).toBeUndefined(); - FeatureFlags.set('valid-feature-flag', FeatureFlagState.Enabled); - FeatureFlags.set('another-valid-feature-flag', FeatureFlagState.Enabled); - expect(window.localStorage.featureFlags).toBe( - '{"valid-feature-flag":true,"another-valid-feature-flag":true}', - ); + render(withFeatureFlags()); }); }); }); describe('FeatureFlagsContext', () => { - it('returns an empty list without the context', () => { - expect.assertions(1); + beforeEach(() => { + window.localStorage.clear(); + }); + it('returns an empty set without the context', () => { const Component = () => { const { featureFlags } = useContext(FeatureFlagsContext); - expect(featureFlags).toEqual([]); + expect(featureFlags).toEqual(new Set()); return null; }; render(); }); - it('returns a list of registered feature flags', () => { - expect.assertions(1); + it('returns a set of registered feature flags', () => { + expect.assertions(2); - const mockFeatureFlags = [ + const mockFeatureFlags = new Set([ { name: 'feature-flag-one', pluginId: 'plugin-one' }, { name: 'feature-flag-two', pluginId: 'plugin-two' }, - ]; + ]); const Component = () => { const { featureFlags } = useContext(FeatureFlagsContext); - expect(featureFlags).toBe(mockFeatureFlags); + expect(featureFlags).toEqual(mockFeatureFlags); return null; }; - render( - - - , + render(withFeatureFlags(, mockFeatureFlags)); + }); + + it('returns a set of user enabled feature flags', () => { + expect.assertions(1); + + window.localStorage.setItem( + 'featureFlags', + JSON.stringify({ + 'feature-flag-one': true, + 'feature-flag-three': true, + }), ); + + const Component = () => { + const { enabledFeatureFlags } = useContext(FeatureFlagsContext); + + if (enabledFeatureFlags.size > 0) { + expect(enabledFeatureFlags).toEqual( + new Set(['feature-flag-one', 'feature-flag-three']), + ); + } + + return null; + }; + + render(withFeatureFlags()); + }); + + it('correctly re-renders when calling refreshEnabledFeatureFlags', () => { + // First is the initial context + // Second is the context with the state from FeatureFlagsContextProvider + // Third is from calling refreshEnabledFeatureFlags + expect.assertions(3); + + const FirstRender = ({ context: { enabledFeatureFlags } }) => { + useEffect(() => { + expect(enabledFeatureFlags).toEqual(new Set()); + }, []); + return null; + }; + + const SecondRender = ({ + context: { enabledFeatureFlags, refreshEnabledFeatureFlags }, + }) => { + useEffect(() => { + expect(enabledFeatureFlags).toEqual(new Set()); + + // Change localStorage + window.localStorage.setItem( + 'featureFlags', + JSON.stringify({ + 'feature-flag-one': true, + 'feature-flag-three': true, + }), + ); + + // Refresh + refreshEnabledFeatureFlags(); + }, []); + + return null; + }; + + const ThirdRender = ({ context: { enabledFeatureFlags } }) => { + useEffect(() => { + expect(enabledFeatureFlags).toEqual( + new Set(['feature-flag-one', 'feature-flag-three']), + ); + }, []); + + return null; + }; + + const Component = () => { + const context = useContext(FeatureFlagsContext); + const renderCount = useRenderCount(); + + if (renderCount === 0) return ; + if (renderCount === 1) return ; + if (renderCount === 2) return ; + + return null; + }; + + render(withFeatureFlags()); }); }); diff --git a/packages/core/src/api/app/FeatureFlags.tsx b/packages/core/src/api/app/FeatureFlags.tsx index e5a2063621..4f2b7da7d0 100644 --- a/packages/core/src/api/app/FeatureFlags.tsx +++ b/packages/core/src/api/app/FeatureFlags.tsx @@ -156,21 +156,19 @@ class FeatureFlagsImpl implements FeatureFlagsApi { // We don't make this private as we need this to validate // in the `registerFeatureFlag` method in the Plugin API. checkFeatureFlagNameErrors(name: FeatureFlagName): string[] { - const errors = []; + const errors: string[] = []; if (name.length < 3) { - errors.push( - 'The `name` argument must have a minimum length of three characters.', - ); + errors.push('The name must have a minimum length of three characters.'); } if (name.length > 150) { - errors.push('The `name` argument must not exceed 150 characters.'); + errors.push('The name must not exceed 150 characters.'); } if (!name.match(/^[a-z]+[a-z0-9-]+$/)) { errors.push( - 'The `name` argument must start with a lowercase letter and only contain lowercase letters, numbers and hyphens. ' + + 'The name must start with a lowercase letter and only contain lowercase letters, numbers and hyphens. ' + 'Examples: feature-flag-one, alpha, release-2020', ); } From 6fa6373536c00d4375f93c89fe0b57367e637756 Mon Sep 17 00:00:00 2001 From: Bilawal Hameed Date: Fri, 27 Mar 2020 18:53:54 +0100 Subject: [PATCH 28/35] [core/apis] replaced FeatureFlagsContext with FeatureFlags.registeredFlags --- packages/core/src/api/app/AppBuilder.tsx | 11 +- .../core/src/api/app/FeatureFlags.test.tsx | 133 +----------------- packages/core/src/api/app/FeatureFlags.tsx | 102 +------------- packages/core/src/api/index.ts | 6 +- 4 files changed, 8 insertions(+), 244 deletions(-) diff --git a/packages/core/src/api/app/AppBuilder.tsx b/packages/core/src/api/app/AppBuilder.tsx index bd40565260..6964c45a26 100644 --- a/packages/core/src/api/app/AppBuilder.tsx +++ b/packages/core/src/api/app/AppBuilder.tsx @@ -19,7 +19,7 @@ import { Route, Switch, Redirect } from 'react-router-dom'; import { AppContextProvider } from './AppContext'; import { App } from './types'; import BackstagePlugin from '../plugin/Plugin'; -import { FeatureFlagsEntry, FeatureFlagsContextProvider } from './FeatureFlags'; +import { FeatureFlags, FeatureFlagsEntry } from './FeatureFlags'; import { IconComponent, SystemIcons, @@ -102,6 +102,8 @@ export default class AppBuilder { } } + FeatureFlags.registeredFeatureFlags = registeredFeatureFlags; + routes.push( , ); @@ -117,13 +119,6 @@ export default class AppBuilder { rendered = ; } - rendered = ( - - ); - return () => ; } } diff --git a/packages/core/src/api/app/FeatureFlags.test.tsx b/packages/core/src/api/app/FeatureFlags.test.tsx index b803b27b3a..cb5be32394 100644 --- a/packages/core/src/api/app/FeatureFlags.test.tsx +++ b/packages/core/src/api/app/FeatureFlags.test.tsx @@ -16,12 +16,7 @@ import React, { ReactNode, useContext, useEffect, useRef } from 'react'; import { render } from '@testing-library/react'; -import { - FeatureFlags, - FeatureFlagsEntry, - FeatureFlagsContext, - FeatureFlagsContextProvider, -} from './FeatureFlags'; +import { FeatureFlags } from './FeatureFlags'; import { FeatureFlagState } from '../apis/definitions/featureFlags'; function useRenderCount() { @@ -37,13 +32,7 @@ function withFeatureFlags( { name: 'feature-flag-two', pluginId: 'plugin-two' }, { name: 'feature-flag-three', pluginId: 'plugin-two' }, ]), -) { - return ( - - {children} - - ); -} +) {} describe('FeatureFlags', () => { beforeEach(() => { @@ -255,121 +244,3 @@ describe('FeatureFlags', () => { }); }); }); - -describe('FeatureFlagsContext', () => { - beforeEach(() => { - window.localStorage.clear(); - }); - - it('returns an empty set without the context', () => { - const Component = () => { - const { featureFlags } = useContext(FeatureFlagsContext); - expect(featureFlags).toEqual(new Set()); - return null; - }; - - render(); - }); - - it('returns a set of registered feature flags', () => { - expect.assertions(2); - - const mockFeatureFlags = new Set([ - { name: 'feature-flag-one', pluginId: 'plugin-one' }, - { name: 'feature-flag-two', pluginId: 'plugin-two' }, - ]); - - const Component = () => { - const { featureFlags } = useContext(FeatureFlagsContext); - expect(featureFlags).toEqual(mockFeatureFlags); - return null; - }; - - render(withFeatureFlags(, mockFeatureFlags)); - }); - - it('returns a set of user enabled feature flags', () => { - expect.assertions(1); - - window.localStorage.setItem( - 'featureFlags', - JSON.stringify({ - 'feature-flag-one': true, - 'feature-flag-three': true, - }), - ); - - const Component = () => { - const { enabledFeatureFlags } = useContext(FeatureFlagsContext); - - if (enabledFeatureFlags.size > 0) { - expect(enabledFeatureFlags).toEqual( - new Set(['feature-flag-one', 'feature-flag-three']), - ); - } - - return null; - }; - - render(withFeatureFlags()); - }); - - it('correctly re-renders when calling refreshEnabledFeatureFlags', () => { - // First is the initial context - // Second is the context with the state from FeatureFlagsContextProvider - // Third is from calling refreshEnabledFeatureFlags - expect.assertions(3); - - const FirstRender = ({ context: { enabledFeatureFlags } }) => { - useEffect(() => { - expect(enabledFeatureFlags).toEqual(new Set()); - }, []); - return null; - }; - - const SecondRender = ({ - context: { enabledFeatureFlags, refreshEnabledFeatureFlags }, - }) => { - useEffect(() => { - expect(enabledFeatureFlags).toEqual(new Set()); - - // Change localStorage - window.localStorage.setItem( - 'featureFlags', - JSON.stringify({ - 'feature-flag-one': true, - 'feature-flag-three': true, - }), - ); - - // Refresh - refreshEnabledFeatureFlags(); - }, []); - - return null; - }; - - const ThirdRender = ({ context: { enabledFeatureFlags } }) => { - useEffect(() => { - expect(enabledFeatureFlags).toEqual( - new Set(['feature-flag-one', 'feature-flag-three']), - ); - }, []); - - return null; - }; - - const Component = () => { - const context = useContext(FeatureFlagsContext); - const renderCount = useRenderCount(); - - if (renderCount === 0) return ; - if (renderCount === 1) return ; - if (renderCount === 2) return ; - - return null; - }; - - render(withFeatureFlags()); - }); -}); diff --git a/packages/core/src/api/app/FeatureFlags.tsx b/packages/core/src/api/app/FeatureFlags.tsx index 4f2b7da7d0..172daeb878 100644 --- a/packages/core/src/api/app/FeatureFlags.tsx +++ b/packages/core/src/api/app/FeatureFlags.tsx @@ -14,78 +14,17 @@ * limitations under the License. */ -import React, { - ReactNode, - createContext, - useContext, - useState, - useEffect, - FC, -} from 'react'; import { FeatureFlagName } from '../plugin/types'; import { FeatureFlagState, FeatureFlagsApi, } from '../apis/definitions/featureFlags'; -/** - * Create a shared React context for Feature Flags. - * - * This will be used to propagate all available feature flags to - * Backstage components. This enables viewing all of the available flags. - */ export interface FeatureFlagsEntry { pluginId: string; name: FeatureFlagName; } -export interface IFeatureFlagsContext { - featureFlags: Set; - enabledFeatureFlags: Set; - refreshEnabledFeatureFlags: () => void; -} - -export const FeatureFlagsContext = createContext({ - featureFlags: new Set(), - enabledFeatureFlags: new Set(), - refreshEnabledFeatureFlags: () => { - throw new Error( - 'The refreshEnabledFeatureFlags method is not implemented as it is not called from within the FeatureFlagsContext context within React. ' + - 'See the Backstage documentation for examples on how to use the Feature Flags API.', - ); - }, -}); - -export const FeatureFlagsContextProvider: FC<{ - featureFlags: Set; - children: ReactNode; -}> = ({ featureFlags, children }) => { - const [enabledFeatureFlags, setEnabledFeatureFlags] = useState< - Set - >(new Set()); - - const refreshEnabledFeatureFlags = () => { - // eslint-disable-next-line no-use-before-define - setEnabledFeatureFlags(FeatureFlags.getEnabledFeatureFlags()); - }; - - // Initially populate our setEnabledFeatureFlags - useEffect(() => { - refreshEnabledFeatureFlags(); - }, []); - - return ( - - ); -}; - /** * Create the FeatureFlags implementation based on the API. */ @@ -95,46 +34,9 @@ export const FeatureFlagsContextProvider: FC<{ class FeatureFlagsImpl implements FeatureFlagsApi { private readonly localStorageKey = 'featureFlags'; - private get( - enabledFeatureFlags: Set, - name: FeatureFlagName, - ): FeatureFlagState { - if (enabledFeatureFlags.has(name)) { - return FeatureFlagState.Enabled; - } + public constructor(public registeredFeatureFlags: FeatureFlagsEntry[] = []) {} - return FeatureFlagState.NotEnabled; - } - - private set(name: FeatureFlagName, state: FeatureFlagState): void { - const errors = this.checkFeatureFlagNameErrors(name); - const flags = this.getEnabledFeatureFlags(); - - if (errors.length > 0) { - throw new Error(errors[0]); - } - - if (state === FeatureFlagState.Enabled) { - flags.add(name); - } else if (state === FeatureFlagState.NotEnabled) { - flags.delete(name); - } else { - throw new Error( - 'The `state` argument requires a recognized value from the FeatureFlagState enum. ' + - 'Please check the Backstage documentation to see all the available options.' + - 'Example values: FeatureFlagState.NotEnabled, FeatureFlagState.Enabled', - ); - } - - window.localStorage.setItem( - this.localStorageKey, - JSON.stringify( - [...flags].reduce((list, flag) => ({ ...list, [flag]: true }), {}), - ), - ); - } - - getEnabledFeatureFlags(): Set { + private getUserEnabledFeatureFlags(): Set { if (!('localStorage' in window)) { throw new Error( 'Feature Flags are not supported on browsers without the Local Storage API', diff --git a/packages/core/src/api/index.ts b/packages/core/src/api/index.ts index 4c77e94984..e60a7efa18 100644 --- a/packages/core/src/api/index.ts +++ b/packages/core/src/api/index.ts @@ -16,9 +16,5 @@ export * from './api'; export * from './apis'; -export { - FeatureFlags, - FeatureFlagsContext, - FeatureFlagsContextProvider, -} from './app/FeatureFlags'; +export { FeatureFlags } from './app/FeatureFlags'; export { useApp } from './app/AppContext'; From 64a14b2191e3d94dbaa3a7dd406b9c55605807e7 Mon Sep 17 00:00:00 2001 From: Bilawal Hameed Date: Fri, 27 Mar 2020 19:45:59 +0100 Subject: [PATCH 29/35] [core/apis] adapted to build on existing data structures --- docs/reference/createPlugin-feature-flags.md | 11 +- docs/reference/createPlugin.md | 2 +- packages/app/src/apis.ts | 4 +- .../src/api/apis/definitions/featureFlags.ts | 30 +- packages/core/src/api/app/AppBuilder.tsx | 12 +- .../core/src/api/app/FeatureFlags.test.tsx | 372 +++++++++--------- packages/core/src/api/app/FeatureFlags.tsx | 233 ++++++----- packages/core/src/api/plugin/Plugin.tsx | 17 +- .../ToggleFeatureFlagButton.test.tsx | 38 +- .../WelcomePage/ToggleFeatureFlagButton.tsx | 22 +- plugins/welcome/src/plugin.ts | 2 +- 11 files changed, 387 insertions(+), 356 deletions(-) diff --git a/docs/reference/createPlugin-feature-flags.md b/docs/reference/createPlugin-feature-flags.md index bbf7f17c32..6006dc6cb7 100644 --- a/docs/reference/createPlugin-feature-flags.md +++ b/docs/reference/createPlugin-feature-flags.md @@ -4,7 +4,7 @@ The `featureFlags` object passed to the `register` function makes it possible fo ```typescript export type FeatureFlagsHooks = { - registerFeatureFlag(name: FeatureFlagName): void; + register(name: FeatureFlagName): void; }; ``` @@ -17,7 +17,7 @@ export default createPlugin({ id: 'welcome', register({ router, featureFlags }) { // router.registerRoute('/', Component); - featureFlags.registerFeatureFlag('enable-example-feature'); + featureFlags.register('enable-example-feature'); }, }); ``` @@ -48,11 +48,10 @@ import { Button } from '@material-ui/core'; import { featureFlagsApiRef, useApi } from '@backstage/core'; const ExampleButton: FC<{}> = () => { - const { useFeatureFlag } = useApi(featureFlagsApiRef); - const [flagState, setFlagState] = useFeatureFlag('enable-example-feature'); + const flags = useApi(featureFlagsApiRef).getFlags(); const handleClick = () => { - setFlagState(FeatureFlagState.Enabled); + flags.set('enable-example-feature', FeatureFlagState.Enabled); }; return ( @@ -63,6 +62,4 @@ const ExampleButton: FC<{}> = () => { }; ``` -Note that you must register it with `registerFeatureFlag` otherwise the `useFeatureFlag` will throw an error. - [Back to References](README.md) diff --git a/docs/reference/createPlugin.md b/docs/reference/createPlugin.md index b19df50c22..de4636e11d 100644 --- a/docs/reference/createPlugin.md +++ b/docs/reference/createPlugin.md @@ -33,7 +33,7 @@ import ExampleComponent from './components/ExampleComponent'; export default createPlugin({ id: 'new-plugin', register({ router, featureFlags }) { - featureFlags.registerFeatureFlag('enable-example-component'); + featureFlags.register('enable-example-component'); router.registerRoute('/new-plugin', ExampleComponent); }, diff --git a/packages/app/src/apis.ts b/packages/app/src/apis.ts index 4d339c3734..494b469312 100644 --- a/packages/app/src/apis.ts +++ b/packages/app/src/apis.ts @@ -26,8 +26,8 @@ import { ErrorDisplayForwarder } from './components/ErrorDisplay/ErrorDisplay'; const builder = ApiRegistry.builder(); export const errorDialogForwarder = new ErrorDisplayForwarder(); - builder.add(errorApiRef, errorDialogForwarder); -builder.add(featureFlagsApiRef, FeatureFlags); + +builder.add(featureFlagsApiRef, new FeatureFlags()); export default builder.build() as ApiHolder; diff --git a/packages/core/src/api/apis/definitions/featureFlags.ts b/packages/core/src/api/apis/definitions/featureFlags.ts index e3eaeefdfb..95fd02a1cb 100644 --- a/packages/core/src/api/apis/definitions/featureFlags.ts +++ b/packages/core/src/api/apis/definitions/featureFlags.ts @@ -15,7 +15,11 @@ */ import ApiRef from '../ApiRef'; -import { FeatureFlagName } from '../../plugin/types'; +import { + UserFlags, + FeatureFlagsRegistry, + FeatureFlagsRegistryItem, +} from '../../app/FeatureFlags'; /** * The feature flags API is used to toggle functionality to users across plugins and Backstage. @@ -34,30 +38,22 @@ export enum FeatureFlagState { Enabled = 1, } -export type FeatureFlagsApi = { +export interface FeatureFlagsApi { /** - * Get a list of the user's currently enabled feature flags. - * Reads directly from window.localStorage Browser API. - * - * @returns string[] enabledFeatureFlags List of feature flags enabled by the current user + * Store a list of registered feature flags. */ - getEnabledFeatureFlags(): Set; + registeredFeatureFlags: FeatureFlagsRegistryItem[]; /** - * Check the feature flag name convention. - * Used in the `registerFeatureFlag` method as well as in the `set` method. - * - * @returns string[] errors List of errors as string. Empty array if no errors. + * Get a list of all feature flags from the current user. */ - checkFeatureFlagNameErrors(name: FeatureFlagName): string[]; + getFlags(): UserFlags; /** - * Use Feature Flags as a React Hook. + * Get a list of all registered flags. */ - useFeatureFlag( - name: FeatureFlagName, - ): [FeatureFlagState, (state: FeatureFlagState) => void]; -}; + getRegisteredFlags(): FeatureFlagsRegistry; +} export const featureFlagsApiRef = new ApiRef({ id: 'core.featureflags', diff --git a/packages/core/src/api/app/AppBuilder.tsx b/packages/core/src/api/app/AppBuilder.tsx index 6964c45a26..9aca19b1fe 100644 --- a/packages/core/src/api/app/AppBuilder.tsx +++ b/packages/core/src/api/app/AppBuilder.tsx @@ -19,7 +19,8 @@ import { Route, Switch, Redirect } from 'react-router-dom'; import { AppContextProvider } from './AppContext'; import { App } from './types'; import BackstagePlugin from '../plugin/Plugin'; -import { FeatureFlags, FeatureFlagsEntry } from './FeatureFlags'; +import { FeatureFlagsRegistryItem } from './FeatureFlags'; +import { featureFlagsApiRef } from '../apis/definitions/featureFlags'; import { IconComponent, SystemIcons, @@ -63,7 +64,7 @@ export default class AppBuilder { const app = new AppImpl(this.systemIcons); const routes = new Array(); - const registeredFeatureFlags = new Set(); + const registeredFeatureFlags = new Array(); for (const plugin of this.plugins.values()) { for (const output of plugin.output()) { @@ -90,7 +91,7 @@ export default class AppBuilder { break; } case 'feature-flag': { - registeredFeatureFlags.add({ + registeredFeatureFlags.push({ pluginId: plugin.getId(), name: output.name, }); @@ -102,7 +103,10 @@ export default class AppBuilder { } } - FeatureFlags.registeredFeatureFlags = registeredFeatureFlags; + const FeatureFlags = this.apis && this.apis.get(featureFlagsApiRef); + if (FeatureFlags) { + FeatureFlags!.registeredFeatureFlags = registeredFeatureFlags; + } routes.push( , diff --git a/packages/core/src/api/app/FeatureFlags.test.tsx b/packages/core/src/api/app/FeatureFlags.test.tsx index cb5be32394..1ad247634c 100644 --- a/packages/core/src/api/app/FeatureFlags.test.tsx +++ b/packages/core/src/api/app/FeatureFlags.test.tsx @@ -14,233 +14,231 @@ * limitations under the License. */ -import React, { ReactNode, useContext, useEffect, useRef } from 'react'; -import { render } from '@testing-library/react'; -import { FeatureFlags } from './FeatureFlags'; +import { FeatureFlags as FeatureFlagsImpl } from './FeatureFlags'; import { FeatureFlagState } from '../apis/definitions/featureFlags'; -function useRenderCount() { - const renderCount = useRef(-1); - renderCount.current += 1; - return renderCount.current; -} - -function withFeatureFlags( - children: ReactNode, - featureFlags: Set = new Set([ - { name: 'feature-flag-one', pluginId: 'plugin-one' }, - { name: 'feature-flag-two', pluginId: 'plugin-two' }, - { name: 'feature-flag-three', pluginId: 'plugin-two' }, - ]), -) {} - describe('FeatureFlags', () => { beforeEach(() => { window.localStorage.clear(); }); - describe('#getEnabledFeatureFlags', () => { - it('returns an empty set', () => { - expect(FeatureFlags.getEnabledFeatureFlags()).toEqual(new Set()); + describe('#getFlags', () => { + let FeatureFlags; + + beforeEach(() => { + FeatureFlags = new FeatureFlagsImpl(); }); - it('returns enabled feature flags', () => { + it('returns no flags', () => { + expect(FeatureFlags.getFlags().toObject()).toMatchObject({}); + }); + + it('returns the correct flags', () => { window.localStorage.setItem( 'featureFlags', JSON.stringify({ - 'feature-flag-one': true, - 'feature-flag-three': true, + 'feature-flag-one': 1, + 'feature-flag-two': 1, + 'feature-flag-three': 0, }), ); - expect(FeatureFlags.getEnabledFeatureFlags()).toEqual( - new Set(['feature-flag-one', 'feature-flag-three']), + expect(FeatureFlags.getFlags().toObject()).toMatchObject({ + 'feature-flag-one': FeatureFlagState.Enabled, + 'feature-flag-two': FeatureFlagState.Enabled, + 'feature-flag-three': FeatureFlagState.NotEnabled, + }); + }); + + it('gets the correct values', () => { + window.localStorage.setItem( + 'featureFlags', + JSON.stringify({ + 'feature-flag-one': 1, + 'feature-flag-two': 0, + }), ); + + expect(FeatureFlags.getFlags().get('feature-flag-one')).toEqual( + FeatureFlagState.Enabled, + ); + expect(FeatureFlags.getFlags().get('feature-flag-two')).toEqual( + FeatureFlagState.NotEnabled, + ); + expect(FeatureFlags.getFlags().get('feature-flag-three')).toEqual( + FeatureFlagState.NotEnabled, + ); + }); + + it('sets the correct values', () => { + const flags = FeatureFlags.getFlags(); + flags.set('feature-flag-zero', FeatureFlagState.Enabled); + + expect(flags.get('feature-flag-zero')).toEqual(FeatureFlagState.Enabled); + expect(window.localStorage.getItem('featureFlags')).toEqual( + '{"feature-flag-zero":1}', + ); + }); + + it('deletes the correct values', () => { + window.localStorage.setItem( + 'featureFlags', + JSON.stringify({ + 'feature-flag-one': 1, + 'feature-flag-two': 0, + }), + ); + + const flags = FeatureFlags.getFlags(); + flags.delete('feature-flag-one'); + + expect(flags.get('feature-flag-one')).toEqual( + FeatureFlagState.NotEnabled, + ); + expect(window.localStorage.getItem('featureFlags')).toEqual( + '{"feature-flag-two":0}', + ); + }); + + it('clears all values', () => { + window.localStorage.setItem( + 'featureFlags', + JSON.stringify({ + 'feature-flag-one': 1, + 'feature-flag-two': 1, + 'feature-flag-three': 0, + }), + ); + + const flags = FeatureFlags.getFlags(); + flags.clear(); + + expect(flags.toObject()).toEqual({}); + expect(window.localStorage.getItem('featureFlags')).toEqual('{}'); }); }); - describe('#checkFeatureFlagNameErrors', () => { - it('returns an error if less than three characters', () => { - const errors = FeatureFlags.checkFeatureFlagNameErrors('ab'); - expect(errors[0]).toMatch(/minimum length of three characters/i); + describe('#getRegisteredFlags', () => { + let FeatureFlags; + + beforeEach(() => { + FeatureFlags = new FeatureFlagsImpl(); + FeatureFlags.registeredFeatureFlags = [ + { name: 'registered-flag-1', pluginId: 'plugin-one' }, + { name: 'registered-flag-2', pluginId: 'plugin-one' }, + { name: 'registered-flag-3', pluginId: 'plugin-two' }, + ]; }); - it('returns an error if greater than 150 characters', () => { - const errors = FeatureFlags.checkFeatureFlagNameErrors( - 'loremipsumdolorsitametconsecteturadipiscingelitnuncvitaeportaexaullamcorperturpismaurisutmattisnequemorbisediaculisauguevivamuspulvinarcursuseratblandithendreritquisqueuttinciduntmagnavestibulumblanditaugueat', - ); - expect(errors[0]).toMatch(/not exceed 150 characters/i); + it('should return an empty list', () => { + FeatureFlags.registeredFeatureFlags = []; + expect(FeatureFlags.getRegisteredFlags().toObject()).toEqual([]); }); - it('returns an error if name does not start with a lowercase letter', () => { - const errors = FeatureFlags.checkFeatureFlagNameErrors('123456789'); - expect(errors[0]).toMatch(/start with a lowercase letter/i); + it('should return an valid list', () => { + expect(FeatureFlags.getRegisteredFlags().toObject()).toMatchObject([ + { name: 'registered-flag-1', pluginId: 'plugin-one' }, + { name: 'registered-flag-2', pluginId: 'plugin-one' }, + { name: 'registered-flag-3', pluginId: 'plugin-two' }, + ]); }); - it('returns an error if name contains characters other than lowercase letters, numbers and hyphens', () => { - const errors = FeatureFlags.checkFeatureFlagNameErrors( - 'Invalid_Feature_Flag', - ); - expect(errors[0]).toMatch( - /only contain lowercase letters, numbers and hyphens/i, - ); + it('should get the correct values', () => { + const getByName = name => + FeatureFlags.getRegisteredFlags().find(flag => flag.name === name); + + expect(getByName('registered-flag-0')).toBeUndefined(); + expect(getByName('registered-flag-1')).toEqual({ + name: 'registered-flag-1', + pluginId: 'plugin-one', + }); + expect(getByName('registered-flag-2')).toEqual({ + name: 'registered-flag-2', + pluginId: 'plugin-one', + }); + expect(getByName('registered-flag-3')).toEqual({ + name: 'registered-flag-3', + pluginId: 'plugin-two', + }); }); - it('returns no errors', () => { - const errors = FeatureFlags.checkFeatureFlagNameErrors( - 'valid-feature-flag', - ); - expect(errors.length).toBe(0); - }); - }); + it('should append the correct value', () => { + const flags = FeatureFlags.getRegisteredFlags(); - describe('#useFeatureFlag', () => { - it('throws an error if the feature flag is not registered', () => { - const Component = () => { - expect(() => { - FeatureFlags.useFeatureFlag('feature-flag-four'); - }).toThrow(/'feature-flag-four' feature flag is not registered/i); + flags.push({ + name: 'registered-flag-4', + pluginId: 'plugin-three', + }); - return null; - }; - - render(withFeatureFlags()); + expect(flags.toObject()).toMatchObject([ + { name: 'registered-flag-1', pluginId: 'plugin-one' }, + { name: 'registered-flag-2', pluginId: 'plugin-one' }, + { name: 'registered-flag-3', pluginId: 'plugin-two' }, + { name: 'registered-flag-4', pluginId: 'plugin-three' }, + ]); }); - it('throws an error if changing value is not recognized', () => { - const Component = () => { - const [, setState] = FeatureFlags.useFeatureFlag('feature-flag-one'); - const renderCount = useRenderCount(); - if (renderCount === 1) { - // @ts-ignore - expect(() => setState('not valid')).toThrow( - /requires a recognized value from the FeatureFlagState/i, - ); - } - return null; - }; + it('should concat the correct values', () => { + const flags = FeatureFlags.getRegisteredFlags(); + const concatValues = flags.concat([ + { + name: 'registered-flag-4', + pluginId: 'plugin-three', + }, + { + name: 'registered-flag-5', + pluginId: 'plugin-four', + }, + ]); - render(withFeatureFlags()); + expect(concatValues).toMatchObject([ + { name: 'registered-flag-1', pluginId: 'plugin-one' }, + { name: 'registered-flag-2', pluginId: 'plugin-one' }, + { name: 'registered-flag-3', pluginId: 'plugin-two' }, + { name: 'registered-flag-4', pluginId: 'plugin-three' }, + { name: 'registered-flag-5', pluginId: 'plugin-four' }, + ]); }); - it('defaults to .NotEnabled', () => { - const Component = () => { - const renderCount = useRenderCount(); - const [state] = FeatureFlags.useFeatureFlag('feature-flag-one'); - if (renderCount === 1) { - expect(state).toEqual(FeatureFlagState.NotEnabled); - } - return null; - }; - - render(withFeatureFlags()); + it('throws an error if length is less than three characters', () => { + const flags = FeatureFlags.getRegisteredFlags(); + expect(() => + flags.push({ + name: 'ab', + pluginId: 'plugin-three', + }), + ).toThrow(/minimum length of three characters/i); }); - it('returns an .Enabled state', () => { - window.localStorage.setItem('featureFlags', '{"feature-flag-one":true}'); - - const Component = () => { - const [state] = FeatureFlags.useFeatureFlag('feature-flag-one'); - const renderCount = useRenderCount(); - if (renderCount === 1) { - expect(state).toEqual(FeatureFlagState.Enabled); - } - return null; - }; - - render(withFeatureFlags()); + it('throws an error if length is greater than 150 characters', () => { + const flags = FeatureFlags.getRegisteredFlags(); + expect(() => + flags.push({ + name: + 'loremipsumdolorsitametconsecteturadipiscingelitnuncvitaeportaexaullamcorperturpismaurisutmattisnequemorbisediaculisauguevivamuspulvinarcursuseratblandithendreritquisqueuttinciduntmagnavestibulumblanditaugueat', + pluginId: 'plugin-three', + }), + ).toThrow(/not exceed 150 characters/i); }); - it('returns an .NotEnabled state', () => { - const Component = () => { - const [state] = FeatureFlags.useFeatureFlag('feature-flag-one'); - const renderCount = useRenderCount(); - if (renderCount === 1) { - expect(state).toEqual(FeatureFlagState.NotEnabled); - } - return null; - }; - - render(withFeatureFlags()); + it('throws an error if name does not start with a lowercase letter', () => { + const flags = FeatureFlags.getRegisteredFlags(); + expect(() => + flags.push({ + name: '123456789', + pluginId: 'plugin-three', + }), + ).toThrow(/start with a lowercase letter/i); }); - it('changes state to .Enabled', () => { - const Component = () => { - const [state, setState] = FeatureFlags.useFeatureFlag( - 'feature-flag-one', - ); - const renderCount = useRenderCount(); - if (renderCount === 1) setState(FeatureFlagState.Enabled); - if (renderCount === 2) expect(state).toEqual(FeatureFlagState.Enabled); - return null; - }; - - render(withFeatureFlags()); - }); - - it('changes state to .NotEnabled', () => { - window.localStorage.setItem('featureFlags', '{"feature-flag-one":true}'); - - const Component = () => { - const [state, setState] = FeatureFlags.useFeatureFlag( - 'feature-flag-one', - ); - const renderCount = useRenderCount(); - if (renderCount === 1) setState(FeatureFlagState.NotEnabled); - if (renderCount === 2) - expect(state).toEqual(FeatureFlagState.NotEnabled); - return null; - }; - - render(withFeatureFlags()); - }); - - it('changes state to .Enabled then .NotEnabled', () => { - const Component = () => { - const [state, setState] = FeatureFlags.useFeatureFlag( - 'feature-flag-one', - ); - const renderCount = useRenderCount(); - if (renderCount === 1) setState(FeatureFlagState.Enabled); - if (renderCount === 2) setState(FeatureFlagState.NotEnabled); - if (renderCount === 3) - expect(state).toEqual(FeatureFlagState.NotEnabled); - return null; - }; - - render(withFeatureFlags()); - }); - - it('changes multiple feature flag states', () => { - expect.assertions(3); - - const Component = () => { - const renderCount = useRenderCount(); - const [stateA, setStateA] = FeatureFlags.useFeatureFlag( - 'feature-flag-one', - ); - const [stateB, setStateB] = FeatureFlags.useFeatureFlag( - 'feature-flag-two', - ); - - useEffect(() => { - if (renderCount === 1) { - setStateA(FeatureFlagState.Enabled); - setStateB(FeatureFlagState.Enabled); - } - if (renderCount === 2) { - expect(stateA).toEqual(FeatureFlagState.Enabled); - expect(stateB).toEqual(FeatureFlagState.Enabled); - expect(window.localStorage.getItem('featureFlags')).toEqual( - '{"feature-flag-one":true,"feature-flag-two":true}', - ); - } - }, [renderCount]); - - return null; - }; - - render(withFeatureFlags()); + it('throws an error if name contains characters other than lowercase letters, numbers and hyphens', () => { + const flags = FeatureFlags.getRegisteredFlags(); + expect(() => + flags.push({ + name: 'Invalid_Feature_Flag', + pluginId: 'plugin-three', + }), + ).toThrow(/only contain lowercase letters, numbers and hyphens/i); }); }); }); diff --git a/packages/core/src/api/app/FeatureFlags.tsx b/packages/core/src/api/app/FeatureFlags.tsx index 172daeb878..3c2983e63f 100644 --- a/packages/core/src/api/app/FeatureFlags.tsx +++ b/packages/core/src/api/app/FeatureFlags.tsx @@ -20,105 +20,158 @@ import { FeatureFlagsApi, } from '../apis/definitions/featureFlags'; -export interface FeatureFlagsEntry { +/** + * Helper method for validating compatibility and flag name. + */ +export function validateBrowserCompat(): void { + if (!('localStorage' in window)) { + throw new Error( + 'Feature Flags are not supported on browsers without the Local Storage API', + ); + } +} + +export function validateFlagName(name: FeatureFlagName): void { + if (name.length < 3) { + throw new Error(`The '${name}' feature flag must have a minimum length of three characters.`); + } + + if (name.length > 150) { + throw new Error(`The '${name}' feature flag must not exceed 150 characters.`); + } + + if (!name.match(/^[a-z]+[a-z0-9-]+$/)) { + throw new Error( + `The '${name}' feature flag must start with a lowercase letter and only contain lowercase letters, numbers and hyphens. ` + + 'Examples: feature-flag-one, alpha, release-2020', + ); + } +} + +/** + * The UserFlags class. + * + * This acts as a data structure for the user's feature flags. You + * can use this to retrieve, add, edit, delete, clear and save the user's + * feature flags to the local browser for persisted storage. + */ +export class UserFlags extends Map { + constructor() { + validateBrowserCompat(); + + try { + const jsonString = window.localStorage.getItem('featureFlags') as string; + const json = JSON.parse(jsonString); + super(Object.entries(json)); + } catch (err) { + super([]); + } + } + + get(name: FeatureFlagName): FeatureFlagState { + validateFlagName(name); + return super.get(name) || FeatureFlagState.NotEnabled; + } + + set(name: FeatureFlagName, state: FeatureFlagState): this { + validateFlagName(name); + super.set(name, state); + this.save(); + return this; + } + + delete(name: FeatureFlagName): boolean { + const exists = !!this.get(name); + super.delete(name); + this.save(); + return exists; + } + + clear(): void { + super.clear(); + this.save(); + } + + save(): void { + window.localStorage.setItem( + 'featureFlags', + JSON.stringify(this.toObject()), + ); + } + + toObject() { + return Array.from(this.entries()).reduce( + (obj, [key, value]) => ({ ...obj, [key]: value }), + {}, + ); + } + + toJSON() { + return JSON.stringify(this.toObject()); + } + + toString() { + return this.toJSON(); + } +} + +/** + * The FeatureFlagsRegistry class. + * + * This acts as a holding data structure for feature flags + * that plugins wish to register for use in Backstage. + */ +export interface FeatureFlagsRegistryItem { pluginId: string; name: FeatureFlagName; } +export class FeatureFlagsRegistry extends Array { + static from(entries: FeatureFlagsRegistryItem[]) { + Array.from(entries).forEach(entry => validateFlagName(entry.name)); + return new FeatureFlagsRegistry(...entries); + } + + push(...entries: FeatureFlagsRegistryItem[]): number { + Array.from(entries).forEach(entry => validateFlagName(entry.name)); + return super.push(...entries); + } + + concat( + ...entries: ( + | FeatureFlagsRegistryItem + | ConcatArray + )[] + ): FeatureFlagsRegistryItem[] { + const _concat = super.concat(...entries); + Array.from(_concat).forEach(entry => validateFlagName(entry.name)); + return _concat; + } + + toObject() { + return [...this.values()]; + } + + toJSON() { + return JSON.stringify(this.toObject()); + } + + toString() { + return this.toJSON(); + } +} + /** * Create the FeatureFlags implementation based on the API. */ +export class FeatureFlags implements FeatureFlagsApi { + public registeredFeatureFlags: FeatureFlagsRegistryItem[] = []; -// TODO: figure out where to put implementations of APIs, both inside apps -// but also in core/separate package. -class FeatureFlagsImpl implements FeatureFlagsApi { - private readonly localStorageKey = 'featureFlags'; - - public constructor(public registeredFeatureFlags: FeatureFlagsEntry[] = []) {} - - private getUserEnabledFeatureFlags(): Set { - if (!('localStorage' in window)) { - throw new Error( - 'Feature Flags are not supported on browsers without the Local Storage API', - ); - } - - try { - const featureFlagsJson = window.localStorage.getItem( - this.localStorageKey, - ); - return new Set( - Object.keys(JSON.parse(featureFlagsJson!)), - ); - } catch (err) { - return new Set(); - } + getFlags(): UserFlags { + return new UserFlags(); } - // We don't make this private as we need this to validate - // in the `registerFeatureFlag` method in the Plugin API. - checkFeatureFlagNameErrors(name: FeatureFlagName): string[] { - const errors: string[] = []; - - if (name.length < 3) { - errors.push('The name must have a minimum length of three characters.'); - } - - if (name.length > 150) { - errors.push('The name must not exceed 150 characters.'); - } - - if (!name.match(/^[a-z]+[a-z0-9-]+$/)) { - errors.push( - 'The name must start with a lowercase letter and only contain lowercase letters, numbers and hyphens. ' + - 'Examples: feature-flag-one, alpha, release-2020', - ); - } - - return errors; - } - - useFeatureFlag( - name: FeatureFlagName, - ): [FeatureFlagState, (state: FeatureFlagState) => void] { - // Check for context - const context = useContext(FeatureFlagsContext); - if (!context) { - throw new Error( - 'No FeatureFlagsContext found. ' + - 'Please use this React Hook in the context of your ', - ); - } - - // Check for errors - // eslint-disable-next-line no-use-before-define - const errors = FeatureFlags.checkFeatureFlagNameErrors(name); - if (errors.length > 0) { - throw new Error(errors[0]); - } - - // Check if the feature flag is registered - const allFlagNames = [...context.featureFlags].map(flag => flag.name); - if (!allFlagNames.includes(name)) { - throw new Error( - `The '${name}' feature flag is not registered by any plugin. ` + - `See the 'registerFeatureFlag' method in the Plugin API (or in your plugin.ts file) on how to register Feature Flags.`, - ); - } - - // eslint-disable-next-line no-use-before-define - const currentState = FeatureFlags.get(context.enabledFeatureFlags, name); - const setState = (state: FeatureFlagState): void => { - // Set the value - // eslint-disable-next-line no-use-before-define - FeatureFlags.set(name, state); - - // Now update the global state - context.refreshEnabledFeatureFlags(); - }; - - return [currentState, setState]; + getRegisteredFlags(): FeatureFlagsRegistry { + return FeatureFlagsRegistry.from(this.registeredFeatureFlags); } } - -export const FeatureFlags = new FeatureFlagsImpl(); diff --git a/packages/core/src/api/plugin/Plugin.tsx b/packages/core/src/api/plugin/Plugin.tsx index d342cb2429..aade5d0092 100644 --- a/packages/core/src/api/plugin/Plugin.tsx +++ b/packages/core/src/api/plugin/Plugin.tsx @@ -21,7 +21,7 @@ import { RouteOptions, FeatureFlagName, } from './types'; -import { FeatureFlags } from '../app/FeatureFlags'; +import { validateBrowserCompat, validateFlagName } from '../app/FeatureFlags'; import { Widget } from '../widgetView/types'; export type PluginConfig = { @@ -54,7 +54,7 @@ export type WidgetHooks = { }; export type FeatureFlagsHooks = { - registerFeatureFlag(name: FeatureFlagName): void; + register(name: FeatureFlagName): void; }; export const registerSymbol = Symbol('plugin-register'); @@ -77,7 +77,6 @@ export default class Plugin { return []; } - const pluginId = this.getId(); const outputs = new Array(); this.config.register({ @@ -95,15 +94,9 @@ export default class Plugin { }, }, featureFlags: { - registerFeatureFlag(name) { - const errors = FeatureFlags.checkFeatureFlagNameErrors(name); - - if (errors.length > 0) { - throw new Error( - `Failed to register '${name}' feature flag in '${pluginId}' plugin: ${errors[0]}`, - ); - } - + register(name) { + validateBrowserCompat(); + validateFlagName(name); outputs.push({ type: 'feature-flag', name }); }, }, diff --git a/plugins/welcome/src/components/WelcomePage/ToggleFeatureFlagButton.test.tsx b/plugins/welcome/src/components/WelcomePage/ToggleFeatureFlagButton.test.tsx index f9be280f50..e5c0b05ecf 100644 --- a/plugins/welcome/src/components/WelcomePage/ToggleFeatureFlagButton.test.tsx +++ b/plugins/welcome/src/components/WelcomePage/ToggleFeatureFlagButton.test.tsx @@ -22,37 +22,27 @@ import { featureFlagsApiRef, ApiProvider, FeatureFlags, - FeatureFlagsContextProvider, } from '@backstage/core'; -function withFeatureFlags(children: ReactNode) { - const featureFlags = new Set([ - { pluginId: 'welcome', name: 'enable-welcome-box' }, - ]); - +function withApiRegistry(component: ReactNode, featureFlags: FeatureFlags) { return ( - - {children} - - ); -} - -function withApiRegistry(children: ReactNode) { - return ( - - {children} + + {component} ); } describe('ToggleFeatureFlagButton', () => { + let featureFlags: FeatureFlags; + beforeEach(() => { + featureFlags = new FeatureFlags(); window.localStorage.clear(); }); it('should enable the feature flag', () => { const rendered = render( - withFeatureFlags(withApiRegistry()), + withApiRegistry(, featureFlags), ); const button = rendered.getByTestId('button-switch-feature-flag-state'); @@ -60,22 +50,22 @@ describe('ToggleFeatureFlagButton', () => { expect(window.localStorage.featureFlags).toBeUndefined(); fireEvent.click(button); - expect(window.localStorage.featureFlags).toBe( - '{"enable-welcome-box":true}', - ); + expect(window.localStorage.featureFlags).toBe('{"enable-welcome-box":1}'); }); it('should disable the feature flag', () => { - const rendered = render( - withFeatureFlags(withApiRegistry()), - ); + const Component = () => + withApiRegistry(, featureFlags); + const rendered = render(); const button = rendered.getByTestId('button-switch-feature-flag-state'); expect(button).toBeInTheDocument(); expect(window.localStorage.featureFlags).toBeUndefined(); fireEvent.click(button); + expect(window.localStorage.featureFlags).toBe('{"enable-welcome-box":1}'); + rendered.rerender(); fireEvent.click(button); - expect(window.localStorage.featureFlags).toBe('{}'); + expect(window.localStorage.featureFlags).toBe('{"enable-welcome-box":0}'); }); }); diff --git a/plugins/welcome/src/components/WelcomePage/ToggleFeatureFlagButton.tsx b/plugins/welcome/src/components/WelcomePage/ToggleFeatureFlagButton.tsx index a940b76263..995339850f 100644 --- a/plugins/welcome/src/components/WelcomePage/ToggleFeatureFlagButton.tsx +++ b/plugins/welcome/src/components/WelcomePage/ToggleFeatureFlagButton.tsx @@ -19,15 +19,16 @@ import { Button } from '@material-ui/core'; import { FeatureFlagState, featureFlagsApiRef, useApi } from '@backstage/core'; const ToggleFeatureFlagButton: FC<{}> = () => { - const { useFeatureFlag } = useApi(featureFlagsApiRef); - const [flagState, setFlagState] = useFeatureFlag('enable-welcome-box'); + const featureFlagsApi = useApi(featureFlagsApiRef); + const flags = featureFlagsApi.getFlags(); + const flagState = flags.get('enable-welcome-box'); const handleClick = () => { - if (flagState === FeatureFlagState.Enabled) { - setFlagState(FeatureFlagState.NotEnabled); - } else { - setFlagState(FeatureFlagState.Enabled); - } + const newValue = flagState + ? FeatureFlagState.NotEnabled + : FeatureFlagState.Enabled; + flags.set('enable-welcome-box', newValue); + window.location.reload(); }; return ( @@ -37,10 +38,9 @@ const ToggleFeatureFlagButton: FC<{}> = () => { onClick={handleClick} data-testid="button-switch-feature-flag-state" > - {flagState === FeatureFlagState.NotEnabled && - 'Enable "enable-welcome-box" feature flag'} - {flagState === FeatureFlagState.Enabled && - 'Disable "enable-welcome-box" feature flag'} + {flagState === FeatureFlagState.NotEnabled + ? 'Disable "enable-welcome-box" feature flag' + : 'Enable "enable-welcome-box" feature flag'} ); }; diff --git a/plugins/welcome/src/plugin.ts b/plugins/welcome/src/plugin.ts index a54a19d4af..e7755862ff 100644 --- a/plugins/welcome/src/plugin.ts +++ b/plugins/welcome/src/plugin.ts @@ -22,6 +22,6 @@ export default createPlugin({ register({ router, featureFlags }) { router.registerRoute('/', WelcomePage); - featureFlags.registerFeatureFlag('enable-welcome-box'); + featureFlags.register('enable-welcome-box'); }, }); From 70fa36ebb42b1d36d7142502d0eade6123a38540 Mon Sep 17 00:00:00 2001 From: Bilawal Hameed Date: Mon, 30 Mar 2020 10:53:46 +0200 Subject: [PATCH 30/35] [core/api/AppBuilder] remove typescript exclamation mark --- packages/core/src/api/app/AppBuilder.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/api/app/AppBuilder.tsx b/packages/core/src/api/app/AppBuilder.tsx index 9aca19b1fe..23d0e17672 100644 --- a/packages/core/src/api/app/AppBuilder.tsx +++ b/packages/core/src/api/app/AppBuilder.tsx @@ -105,7 +105,7 @@ export default class AppBuilder { const FeatureFlags = this.apis && this.apis.get(featureFlagsApiRef); if (FeatureFlags) { - FeatureFlags!.registeredFeatureFlags = registeredFeatureFlags; + FeatureFlags.registeredFeatureFlags = registeredFeatureFlags; } routes.push( From aecaa4e497470722ce02ca348c548c06cf0bf21e Mon Sep 17 00:00:00 2001 From: Bilawal Hameed Date: Mon, 30 Mar 2020 10:57:45 +0200 Subject: [PATCH 31/35] [core/api/FeatureFlags] no validation on get + re-think forwarding output --- packages/core/src/api/app/FeatureFlags.tsx | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/packages/core/src/api/app/FeatureFlags.tsx b/packages/core/src/api/app/FeatureFlags.tsx index 3c2983e63f..d1f4ab1313 100644 --- a/packages/core/src/api/app/FeatureFlags.tsx +++ b/packages/core/src/api/app/FeatureFlags.tsx @@ -33,11 +33,15 @@ export function validateBrowserCompat(): void { export function validateFlagName(name: FeatureFlagName): void { if (name.length < 3) { - throw new Error(`The '${name}' feature flag must have a minimum length of three characters.`); + throw new Error( + `The '${name}' feature flag must have a minimum length of three characters.`, + ); } if (name.length > 150) { - throw new Error(`The '${name}' feature flag must not exceed 150 characters.`); + throw new Error( + `The '${name}' feature flag must not exceed 150 characters.`, + ); } if (!name.match(/^[a-z]+[a-z0-9-]+$/)) { @@ -69,22 +73,20 @@ export class UserFlags extends Map { } get(name: FeatureFlagName): FeatureFlagState { - validateFlagName(name); return super.get(name) || FeatureFlagState.NotEnabled; } set(name: FeatureFlagName, state: FeatureFlagState): this { validateFlagName(name); - super.set(name, state); + const output = super.set(name, state); this.save(); - return this; + return output; } delete(name: FeatureFlagName): boolean { - const exists = !!this.get(name); - super.delete(name); + const output = super.delete(name); this.save(); - return exists; + return output; } clear(): void { From d2d8238330fbdec2d424954eab60ef5d90d0e2bf Mon Sep 17 00:00:00 2001 From: Bilawal Hameed Date: Mon, 30 Mar 2020 11:08:15 +0200 Subject: [PATCH 32/35] [core/api] FeatureFlags.test.tsx - s/FeatureFlags/featureFlags/g --- .../core/src/api/app/FeatureFlags.test.tsx | 46 +++++++++---------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/packages/core/src/api/app/FeatureFlags.test.tsx b/packages/core/src/api/app/FeatureFlags.test.tsx index 1ad247634c..727001b5e6 100644 --- a/packages/core/src/api/app/FeatureFlags.test.tsx +++ b/packages/core/src/api/app/FeatureFlags.test.tsx @@ -23,14 +23,14 @@ describe('FeatureFlags', () => { }); describe('#getFlags', () => { - let FeatureFlags; + let featureFlags; beforeEach(() => { - FeatureFlags = new FeatureFlagsImpl(); + featureFlags = new FeatureFlagsImpl(); }); it('returns no flags', () => { - expect(FeatureFlags.getFlags().toObject()).toMatchObject({}); + expect(featureFlags.getFlags().toObject()).toMatchObject({}); }); it('returns the correct flags', () => { @@ -43,7 +43,7 @@ describe('FeatureFlags', () => { }), ); - expect(FeatureFlags.getFlags().toObject()).toMatchObject({ + expect(featureFlags.getFlags().toObject()).toMatchObject({ 'feature-flag-one': FeatureFlagState.Enabled, 'feature-flag-two': FeatureFlagState.Enabled, 'feature-flag-three': FeatureFlagState.NotEnabled, @@ -59,19 +59,19 @@ describe('FeatureFlags', () => { }), ); - expect(FeatureFlags.getFlags().get('feature-flag-one')).toEqual( + expect(featureFlags.getFlags().get('feature-flag-one')).toEqual( FeatureFlagState.Enabled, ); - expect(FeatureFlags.getFlags().get('feature-flag-two')).toEqual( + expect(featureFlags.getFlags().get('feature-flag-two')).toEqual( FeatureFlagState.NotEnabled, ); - expect(FeatureFlags.getFlags().get('feature-flag-three')).toEqual( + expect(featureFlags.getFlags().get('feature-flag-three')).toEqual( FeatureFlagState.NotEnabled, ); }); it('sets the correct values', () => { - const flags = FeatureFlags.getFlags(); + const flags = featureFlags.getFlags(); flags.set('feature-flag-zero', FeatureFlagState.Enabled); expect(flags.get('feature-flag-zero')).toEqual(FeatureFlagState.Enabled); @@ -89,7 +89,7 @@ describe('FeatureFlags', () => { }), ); - const flags = FeatureFlags.getFlags(); + const flags = featureFlags.getFlags(); flags.delete('feature-flag-one'); expect(flags.get('feature-flag-one')).toEqual( @@ -110,7 +110,7 @@ describe('FeatureFlags', () => { }), ); - const flags = FeatureFlags.getFlags(); + const flags = featureFlags.getFlags(); flags.clear(); expect(flags.toObject()).toEqual({}); @@ -119,11 +119,11 @@ describe('FeatureFlags', () => { }); describe('#getRegisteredFlags', () => { - let FeatureFlags; + let featureFlags; beforeEach(() => { - FeatureFlags = new FeatureFlagsImpl(); - FeatureFlags.registeredFeatureFlags = [ + featureFlags = new FeatureFlagsImpl(); + featureFlags.registeredFeatureFlags = [ { name: 'registered-flag-1', pluginId: 'plugin-one' }, { name: 'registered-flag-2', pluginId: 'plugin-one' }, { name: 'registered-flag-3', pluginId: 'plugin-two' }, @@ -131,12 +131,12 @@ describe('FeatureFlags', () => { }); it('should return an empty list', () => { - FeatureFlags.registeredFeatureFlags = []; - expect(FeatureFlags.getRegisteredFlags().toObject()).toEqual([]); + featureFlags.registeredFeatureFlags = []; + expect(featureFlags.getRegisteredFlags().toObject()).toEqual([]); }); it('should return an valid list', () => { - expect(FeatureFlags.getRegisteredFlags().toObject()).toMatchObject([ + expect(featureFlags.getRegisteredFlags().toObject()).toMatchObject([ { name: 'registered-flag-1', pluginId: 'plugin-one' }, { name: 'registered-flag-2', pluginId: 'plugin-one' }, { name: 'registered-flag-3', pluginId: 'plugin-two' }, @@ -145,7 +145,7 @@ describe('FeatureFlags', () => { it('should get the correct values', () => { const getByName = name => - FeatureFlags.getRegisteredFlags().find(flag => flag.name === name); + featureFlags.getRegisteredFlags().find(flag => flag.name === name); expect(getByName('registered-flag-0')).toBeUndefined(); expect(getByName('registered-flag-1')).toEqual({ @@ -163,7 +163,7 @@ describe('FeatureFlags', () => { }); it('should append the correct value', () => { - const flags = FeatureFlags.getRegisteredFlags(); + const flags = featureFlags.getRegisteredFlags(); flags.push({ name: 'registered-flag-4', @@ -179,7 +179,7 @@ describe('FeatureFlags', () => { }); it('should concat the correct values', () => { - const flags = FeatureFlags.getRegisteredFlags(); + const flags = featureFlags.getRegisteredFlags(); const concatValues = flags.concat([ { name: 'registered-flag-4', @@ -201,7 +201,7 @@ describe('FeatureFlags', () => { }); it('throws an error if length is less than three characters', () => { - const flags = FeatureFlags.getRegisteredFlags(); + const flags = featureFlags.getRegisteredFlags(); expect(() => flags.push({ name: 'ab', @@ -211,7 +211,7 @@ describe('FeatureFlags', () => { }); it('throws an error if length is greater than 150 characters', () => { - const flags = FeatureFlags.getRegisteredFlags(); + const flags = featureFlags.getRegisteredFlags(); expect(() => flags.push({ name: @@ -222,7 +222,7 @@ describe('FeatureFlags', () => { }); it('throws an error if name does not start with a lowercase letter', () => { - const flags = FeatureFlags.getRegisteredFlags(); + const flags = featureFlags.getRegisteredFlags(); expect(() => flags.push({ name: '123456789', @@ -232,7 +232,7 @@ describe('FeatureFlags', () => { }); it('throws an error if name contains characters other than lowercase letters, numbers and hyphens', () => { - const flags = FeatureFlags.getRegisteredFlags(); + const flags = featureFlags.getRegisteredFlags(); expect(() => flags.push({ name: 'Invalid_Feature_Flag', From 0ff31e486d570eedce8729fa1743bfa1b84d887f Mon Sep 17 00:00:00 2001 From: Bilawal Hameed Date: Mon, 30 Mar 2020 11:11:57 +0200 Subject: [PATCH 33/35] [core/api] store single instance of UserFlags --- packages/core/src/api/app/FeatureFlags.test.tsx | 4 ++++ packages/core/src/api/app/FeatureFlags.tsx | 3 ++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/core/src/api/app/FeatureFlags.test.tsx b/packages/core/src/api/app/FeatureFlags.test.tsx index 727001b5e6..b145c3e52c 100644 --- a/packages/core/src/api/app/FeatureFlags.test.tsx +++ b/packages/core/src/api/app/FeatureFlags.test.tsx @@ -43,6 +43,7 @@ describe('FeatureFlags', () => { }), ); + featureFlags = new FeatureFlagsImpl(); expect(featureFlags.getFlags().toObject()).toMatchObject({ 'feature-flag-one': FeatureFlagState.Enabled, 'feature-flag-two': FeatureFlagState.Enabled, @@ -59,6 +60,8 @@ describe('FeatureFlags', () => { }), ); + featureFlags = new FeatureFlagsImpl(); + expect(featureFlags.getFlags().get('feature-flag-one')).toEqual( FeatureFlagState.Enabled, ); @@ -89,6 +92,7 @@ describe('FeatureFlags', () => { }), ); + featureFlags = new FeatureFlagsImpl(); const flags = featureFlags.getFlags(); flags.delete('feature-flag-one'); diff --git a/packages/core/src/api/app/FeatureFlags.tsx b/packages/core/src/api/app/FeatureFlags.tsx index d1f4ab1313..fcde669bdd 100644 --- a/packages/core/src/api/app/FeatureFlags.tsx +++ b/packages/core/src/api/app/FeatureFlags.tsx @@ -168,9 +168,10 @@ export class FeatureFlagsRegistry extends Array { */ export class FeatureFlags implements FeatureFlagsApi { public registeredFeatureFlags: FeatureFlagsRegistryItem[] = []; + private readonly userFlags: UserFlags = new UserFlags(); getFlags(): UserFlags { - return new UserFlags(); + return this.userFlags; } getRegisteredFlags(): FeatureFlagsRegistry { From d911da26d2582cfea56191f1ce508ccdfa1c23d5 Mon Sep 17 00:00:00 2001 From: Bilawal Hameed Date: Mon, 30 Mar 2020 11:18:18 +0200 Subject: [PATCH 34/35] [core/api] changed loading pattern to be a static factory method --- packages/core/src/api/app/FeatureFlags.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/core/src/api/app/FeatureFlags.tsx b/packages/core/src/api/app/FeatureFlags.tsx index fcde669bdd..f242683f63 100644 --- a/packages/core/src/api/app/FeatureFlags.tsx +++ b/packages/core/src/api/app/FeatureFlags.tsx @@ -60,15 +60,15 @@ export function validateFlagName(name: FeatureFlagName): void { * feature flags to the local browser for persisted storage. */ export class UserFlags extends Map { - constructor() { + static load(): UserFlags { validateBrowserCompat(); try { const jsonString = window.localStorage.getItem('featureFlags') as string; const json = JSON.parse(jsonString); - super(Object.entries(json)); + return new this(Object.entries(json)); } catch (err) { - super([]); + return new this([]); } } @@ -168,7 +168,7 @@ export class FeatureFlagsRegistry extends Array { */ export class FeatureFlags implements FeatureFlagsApi { public registeredFeatureFlags: FeatureFlagsRegistryItem[] = []; - private readonly userFlags: UserFlags = new UserFlags(); + private readonly userFlags: UserFlags = UserFlags.load(); getFlags(): UserFlags { return this.userFlags; From 70252160e59ecef235c1f8f370879a79bda541d3 Mon Sep 17 00:00:00 2001 From: Bilawal Hameed Date: Mon, 30 Mar 2020 11:39:15 +0200 Subject: [PATCH 35/35] [core] change FeatureFlagState to {On,Off} --- docs/reference/createPlugin-feature-flags.md | 2 +- .../src/api/apis/definitions/featureFlags.ts | 4 ++-- .../core/src/api/app/FeatureFlags.test.tsx | 20 +++++++++---------- packages/core/src/api/app/FeatureFlags.tsx | 5 +++-- .../ToggleFeatureFlagButton.test.tsx | 11 +++++----- .../WelcomePage/ToggleFeatureFlagButton.tsx | 9 +++++---- 6 files changed, 25 insertions(+), 26 deletions(-) diff --git a/docs/reference/createPlugin-feature-flags.md b/docs/reference/createPlugin-feature-flags.md index 6006dc6cb7..60fddaa8fd 100644 --- a/docs/reference/createPlugin-feature-flags.md +++ b/docs/reference/createPlugin-feature-flags.md @@ -51,7 +51,7 @@ const ExampleButton: FC<{}> = () => { const flags = useApi(featureFlagsApiRef).getFlags(); const handleClick = () => { - flags.set('enable-example-feature', FeatureFlagState.Enabled); + flags.set('enable-example-feature', FeatureFlagState.On); }; return ( diff --git a/packages/core/src/api/apis/definitions/featureFlags.ts b/packages/core/src/api/apis/definitions/featureFlags.ts index 95fd02a1cb..926fe091d5 100644 --- a/packages/core/src/api/apis/definitions/featureFlags.ts +++ b/packages/core/src/api/apis/definitions/featureFlags.ts @@ -34,8 +34,8 @@ import { */ export enum FeatureFlagState { - NotEnabled = 0, - Enabled = 1, + Off = 0, + On = 1, } export interface FeatureFlagsApi { diff --git a/packages/core/src/api/app/FeatureFlags.test.tsx b/packages/core/src/api/app/FeatureFlags.test.tsx index b145c3e52c..b5bbb4d68e 100644 --- a/packages/core/src/api/app/FeatureFlags.test.tsx +++ b/packages/core/src/api/app/FeatureFlags.test.tsx @@ -45,9 +45,9 @@ describe('FeatureFlags', () => { featureFlags = new FeatureFlagsImpl(); expect(featureFlags.getFlags().toObject()).toMatchObject({ - 'feature-flag-one': FeatureFlagState.Enabled, - 'feature-flag-two': FeatureFlagState.Enabled, - 'feature-flag-three': FeatureFlagState.NotEnabled, + 'feature-flag-one': FeatureFlagState.On, + 'feature-flag-two': FeatureFlagState.On, + 'feature-flag-three': FeatureFlagState.Off, }); }); @@ -63,21 +63,21 @@ describe('FeatureFlags', () => { featureFlags = new FeatureFlagsImpl(); expect(featureFlags.getFlags().get('feature-flag-one')).toEqual( - FeatureFlagState.Enabled, + FeatureFlagState.On, ); expect(featureFlags.getFlags().get('feature-flag-two')).toEqual( - FeatureFlagState.NotEnabled, + FeatureFlagState.Off, ); expect(featureFlags.getFlags().get('feature-flag-three')).toEqual( - FeatureFlagState.NotEnabled, + FeatureFlagState.Off, ); }); it('sets the correct values', () => { const flags = featureFlags.getFlags(); - flags.set('feature-flag-zero', FeatureFlagState.Enabled); + flags.set('feature-flag-zero', FeatureFlagState.On); - expect(flags.get('feature-flag-zero')).toEqual(FeatureFlagState.Enabled); + expect(flags.get('feature-flag-zero')).toEqual(FeatureFlagState.On); expect(window.localStorage.getItem('featureFlags')).toEqual( '{"feature-flag-zero":1}', ); @@ -96,9 +96,7 @@ describe('FeatureFlags', () => { const flags = featureFlags.getFlags(); flags.delete('feature-flag-one'); - expect(flags.get('feature-flag-one')).toEqual( - FeatureFlagState.NotEnabled, - ); + expect(flags.get('feature-flag-one')).toEqual(FeatureFlagState.Off); expect(window.localStorage.getItem('featureFlags')).toEqual( '{"feature-flag-two":0}', ); diff --git a/packages/core/src/api/app/FeatureFlags.tsx b/packages/core/src/api/app/FeatureFlags.tsx index f242683f63..8e5a06b209 100644 --- a/packages/core/src/api/app/FeatureFlags.tsx +++ b/packages/core/src/api/app/FeatureFlags.tsx @@ -73,7 +73,7 @@ export class UserFlags extends Map { } get(name: FeatureFlagName): FeatureFlagState { - return super.get(name) || FeatureFlagState.NotEnabled; + return super.get(name) || FeatureFlagState.Off; } set(name: FeatureFlagName, state: FeatureFlagState): this { @@ -168,9 +168,10 @@ export class FeatureFlagsRegistry extends Array { */ export class FeatureFlags implements FeatureFlagsApi { public registeredFeatureFlags: FeatureFlagsRegistryItem[] = []; - private readonly userFlags: UserFlags = UserFlags.load(); + private userFlags: UserFlags | undefined; getFlags(): UserFlags { + if (!this.userFlags) this.userFlags = UserFlags.load(); return this.userFlags; } diff --git a/plugins/welcome/src/components/WelcomePage/ToggleFeatureFlagButton.test.tsx b/plugins/welcome/src/components/WelcomePage/ToggleFeatureFlagButton.test.tsx index e5c0b05ecf..13d6741cc7 100644 --- a/plugins/welcome/src/components/WelcomePage/ToggleFeatureFlagButton.test.tsx +++ b/plugins/welcome/src/components/WelcomePage/ToggleFeatureFlagButton.test.tsx @@ -54,17 +54,16 @@ describe('ToggleFeatureFlagButton', () => { }); it('should disable the feature flag', () => { - const Component = () => - withApiRegistry(, featureFlags); - const rendered = render(); + window.localStorage.setItem('featureFlags', '{"enable-welcome-box":1}'); + + const rendered = render( + withApiRegistry(, featureFlags), + ); const button = rendered.getByTestId('button-switch-feature-flag-state'); expect(button).toBeInTheDocument(); - expect(window.localStorage.featureFlags).toBeUndefined(); - fireEvent.click(button); expect(window.localStorage.featureFlags).toBe('{"enable-welcome-box":1}'); - rendered.rerender(); fireEvent.click(button); expect(window.localStorage.featureFlags).toBe('{"enable-welcome-box":0}'); }); diff --git a/plugins/welcome/src/components/WelcomePage/ToggleFeatureFlagButton.tsx b/plugins/welcome/src/components/WelcomePage/ToggleFeatureFlagButton.tsx index 995339850f..3ce66ce1c7 100644 --- a/plugins/welcome/src/components/WelcomePage/ToggleFeatureFlagButton.tsx +++ b/plugins/welcome/src/components/WelcomePage/ToggleFeatureFlagButton.tsx @@ -24,9 +24,10 @@ const ToggleFeatureFlagButton: FC<{}> = () => { const flagState = flags.get('enable-welcome-box'); const handleClick = () => { - const newValue = flagState - ? FeatureFlagState.NotEnabled - : FeatureFlagState.Enabled; + const newValue = + flagState === FeatureFlagState.On + ? FeatureFlagState.Off + : FeatureFlagState.On; flags.set('enable-welcome-box', newValue); window.location.reload(); }; @@ -38,7 +39,7 @@ const ToggleFeatureFlagButton: FC<{}> = () => { onClick={handleClick} data-testid="button-switch-feature-flag-state" > - {flagState === FeatureFlagState.NotEnabled + {flagState === FeatureFlagState.On ? 'Disable "enable-welcome-box" feature flag' : 'Enable "enable-welcome-box" feature flag'}