Merge pull request #8212 from backstage/mob/deprecate-register

core-plugin-api: Deprecate Plugin register and output

.changeset/slow-olives-confess.md

@@ -0,0 +1,7 @@
+---
+'@backstage/core-plugin-api': patch
+---
+
+Deprecated `register` option of `createPlugin` and the `outputs` methods of the plugin instance.
+
+Introduces the `featureFlags` property to define your feature flags instead.

.changeset/young-hats-shop.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-cost-insights': patch
+---
+
+Supply featureFlags using featureFlag config option.

packages/core-app-api/src/app/AppManager.test.tsx

@@ -349,6 +349,78 @@ describe('Integration Test', () => {
     });
   });
 
+  it('getFeatureFlags should return feature flags', async () => {
+    const storageFlags = new LocalStorageFeatureFlags();
+    jest.spyOn(storageFlags, 'registerFlag');
+
+    const apis = [
+      noOpAnalyticsApi,
+      createApiFactory({
+        api: featureFlagsApiRef,
+        deps: { configApi: configApiRef },
+        factory() {
+          return storageFlags;
+        },
+      }),
+    ];
+
+    const app = new AppManager({
+      apis,
+      defaultApis: [],
+      themes: [
+        {
+          id: 'light',
+          title: 'Light Theme',
+          variant: 'light',
+          Provider: ({ children }) => <>{children}</>,
+        },
+      ],
+      icons,
+      plugins: [
+        createPlugin({
+          id: 'test',
+          featureFlags: [
+            {
+              name: 'foo',
+            },
+          ],
+          register: p => p.featureFlags.register('name'),
+        }),
+      ],
+      components,
+      configLoader: async () => [],
+      bindRoutes: ({ bind }) => {
+        bind(plugin1.externalRoutes, {
+          extRouteRef1: plugin1RouteRef,
+          extRouteRef2: plugin2RouteRef,
+        });
+      },
+    });
+
+    const Provider = app.getProvider();
+    const Router = app.getRouter();
+
+    await renderWithEffects(
+      <Provider>
+        <Router>
+          <Routes>
+            <Route path="/" element={<ExposedComponent />} />
+            <Route path="/foo" element={<HiddenComponent />} />
+          </Routes>
+        </Router>
+      </Provider>,
+    );
+
+    expect(storageFlags.registerFlag).toHaveBeenCalledWith({
+      name: 'name',
+      pluginId: 'test',
+    });
+    expect(storageFlags.registerFlag).toHaveBeenCalledWith({
+      name: 'foo',
+      pluginId: 'test',
+    });
+  });
+
   it('should track route changes via analytics api', async () => {
     const mockAnalyticsApi = new MockAnalyticsApi();
     const apis = [createApiFactory(analyticsApiRef, mockAnalyticsApi)];

packages/core-app-api/src/app/AppManager.tsx

@@ -272,17 +272,26 @@ export class AppManager implements BackstageApp {
           const featureFlagsApi = this.getApiHolder().get(featureFlagsApiRef)!;
 
           for (const plugin of this.plugins.values()) {
-            for (const output of plugin.output()) {
-              switch (output.type) {
-                case 'feature-flag': {
-                  featureFlagsApi.registerFlag({
-                    name: output.name,
-                    pluginId: plugin.getId(),
-                  });
-                  break;
+            if ('getFeatureFlags' in plugin) {
+              for (const flag of plugin.getFeatureFlags()) {
+                featureFlagsApi.registerFlag({
+                  name: flag.name,
+                  pluginId: plugin.getId(),
+                });
+              }
+            } else {
+              for (const output of plugin.output()) {
+                switch (output.type) {
+                  case 'feature-flag': {
+                    featureFlagsApi.registerFlag({
+                      name: output.name,
+                      pluginId: plugin.getId(),
+                    });
+                    break;
+                  }
+                  default:
+                    break;
                 }
-                default:
-                  break;
               }
             }
           }

packages/core-plugin-api/api-report.md

@@ -258,6 +258,7 @@ export type BackstagePlugin<
   getId(): string;
   output(): PluginOutput[];
   getApis(): Iterable<AnyApiFactory>;
+  getFeatureFlags(): Iterable<PluginFeatureFlagConfig>;
   provide<T>(extension: Extension<T>): T;
   routes: Routes;
   externalRoutes: ExternalRoutes;
@@ -463,7 +464,7 @@ export type FeatureFlag = {
   pluginId: string;
 };
 
-// @public
+// @public @deprecated
 export type FeatureFlagOutput = {
   type: 'feature-flag';
   name: string;
@@ -682,14 +683,20 @@ export type PluginConfig<
   register?(hooks: PluginHooks): void;
   routes?: Routes;
   externalRoutes?: ExternalRoutes;
+  featureFlags?: PluginFeatureFlagConfig[];
 };
 
 // @public
+export type PluginFeatureFlagConfig = {
+  name: string;
+};
+
+// @public @deprecated
 export type PluginHooks = {
   featureFlags: FeatureFlagsHooks;
 };
 
-// @public
+// @public @deprecated
 export type PluginOutput = FeatureFlagOutput;
 
 // @public

packages/core-plugin-api/src/plugin/Plugin.test.tsx

@@ -0,0 +1,89 @@
+/*
+ * Copyright 2021 The Backstage Authors
+ *
+ * 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 { createPlugin } from './Plugin';
+
+describe('Plugin Feature Flag', () => {
+  it('should be able to register and receive feature flags', () => {
+    expect(
+      createPlugin({
+        id: 'test',
+        featureFlags: [{ name: 'test' }],
+      }).getFeatureFlags(),
+    ).toEqual([{ name: 'test' }]);
+
+    expect(
+      createPlugin({
+        id: 'test',
+        register({ featureFlags }) {
+          featureFlags.register('blob');
+        },
+      }).getFeatureFlags(),
+    ).toEqual([{ name: 'blob' }]);
+
+    expect(
+      createPlugin({
+        id: 'test',
+        register({ featureFlags }) {
+          featureFlags.register('blob');
+        },
+        featureFlags: [{ name: 'test' }],
+      }).getFeatureFlags(),
+    ).toEqual([{ name: 'test' }, { name: 'blob' }]);
+
+    expect(
+      createPlugin({
+        id: 'test',
+      }).getFeatureFlags(),
+    ).toEqual([]);
+
+    /* deprecated tests */
+
+    expect(
+      createPlugin({
+        id: 'test',
+        featureFlags: [{ name: 'test' }],
+      }).output(),
+    ).toEqual([{ name: 'test', type: 'feature-flag' }]);
+
+    expect(
+      createPlugin({
+        id: 'test',
+        register({ featureFlags }) {
+          featureFlags.register('blob');
+        },
+      }).output(),
+    ).toEqual([{ name: 'blob', type: 'feature-flag' }]);
+    expect(
+      createPlugin({
+        id: 'test',
+        register({ featureFlags }) {
+          featureFlags.register('blob');
+        },
+        featureFlags: [{ name: 'test' }],
+      }).output(),
+    ).toEqual([
+      { name: 'test', type: 'feature-flag' },
+      { name: 'blob', type: 'feature-flag' },
+    ]);
+
+    expect(
+      createPlugin({
+        id: 'test',
+      }).output(),
+    ).toEqual([]);
+  });
+});

packages/core-plugin-api/src/plugin/Plugin.tsx

@@ -21,6 +21,7 @@ import {
   Extension,
   AnyRoutes,
   AnyExternalRoutes,
+  PluginFeatureFlagConfig,
 } from './types';
 import { AnyApiFactory } from '../apis';
 
@@ -44,6 +45,14 @@ export class PluginImpl<
     return this.config.apis ?? [];
   }
 
+  getFeatureFlags(): Iterable<PluginFeatureFlagConfig> {
+    const registeredFlags = this.output()
+      .filter(({ type }) => type === 'feature-flag')
+      .map(({ name }) => ({ name }));
+
+    return registeredFlags;
+  }
+
   get routes(): Routes {
     return this.config.routes ?? ({} as Routes);
   }
@@ -56,11 +65,18 @@ export class PluginImpl<
     if (this.storedOutput) {
       return this.storedOutput;
     }
-    if (!this.config.register) {
-      return [];
+    const outputs = new Array<PluginOutput>();
+    this.storedOutput = outputs;
+
+    if (this.config.featureFlags) {
+      for (const flag of this.config.featureFlags) {
+        outputs.push({ type: 'feature-flag', name: flag.name });
+      }
     }
 
-    const outputs = new Array<PluginOutput>();
+    if (!this.config.register) {
+      return outputs;
+    }
 
     this.config.register({
       featureFlags: {
@@ -70,7 +86,6 @@ export class PluginImpl<
       },
     });
 
-    this.storedOutput = outputs;
     return this.storedOutput;
   }
 

packages/core-plugin-api/src/plugin/index.ts

@@ -25,4 +25,5 @@ export type {
   PluginConfig,
   PluginHooks,
   PluginOutput,
+  PluginFeatureFlagConfig,
 } from './types';

packages/core-plugin-api/src/plugin/types.ts

@@ -19,7 +19,7 @@ import { AnyApiFactory } from '../apis/system';
 
 /**
  * Replace with using {@link RouteRef}s.
- *
+ * @deprecated will be removed
  * @public
  */
 export type FeatureFlagOutput = {
@@ -31,6 +31,7 @@ export type FeatureFlagOutput = {
  * {@link FeatureFlagOutput} type.
  *
  * @public
+ * @deprecated Use {@link BackstagePlugin.getFeatureFlags} instead.
  */
 export type PluginOutput = FeatureFlagOutput;
 
@@ -71,13 +72,30 @@ export type BackstagePlugin<
   ExternalRoutes extends AnyExternalRoutes = {},
 > = {
   getId(): string;
+  /**
+   * @deprecated use getFeatureFlags instead.
+   * */
   output(): PluginOutput[];
   getApis(): Iterable<AnyApiFactory>;
+  /**
+   * Returns all registered feature flags for this plugin.
+   */
+  getFeatureFlags(): Iterable<PluginFeatureFlagConfig>;
   provide<T>(extension: Extension<T>): T;
   routes: Routes;
   externalRoutes: ExternalRoutes;
 };
 
+/**
+ * Plugin feature flag configuration.
+ *
+ * @public
+ */
+export type PluginFeatureFlagConfig = {
+  /** Feature flag name */
+  name: string;
+};
+
 /**
  * Plugin descriptor type.
  *
@@ -89,14 +107,17 @@ export type PluginConfig<
 > = {
   id: string;
   apis?: Iterable<AnyApiFactory>;
+  /** @deprecated use featureFlags property instead for defining feature flags */
   register?(hooks: PluginHooks): void;
   routes?: Routes;
   externalRoutes?: ExternalRoutes;
+  featureFlags?: PluginFeatureFlagConfig[];
 };
 
 /**
  * Holds hooks registered by the plugin.
  *
+ * @deprecated - feature flags are now registered in plugin config under featureFlags
  * @public
  */
 export type PluginHooks = {

plugins/cost-insights/src/plugin.ts

@@ -34,9 +34,7 @@ export const unlabeledDataflowAlertRef = createRouteRef({
 
 export const costInsightsPlugin = createPlugin({
   id: 'cost-insights',
-  register({ featureFlags }) {
-    featureFlags.register('cost-insights-currencies');
-  },
+  featureFlags: [{ name: 'cost-insights-currencies' }],
   routes: {
     root: rootRouteRef,
     growthAlerts: projectGrowthAlertRef,