From ce55c4c40c6eb78b86d76557bfcf7ed328311426 Mon Sep 17 00:00:00 2001 From: blam Date: Tue, 9 Mar 2021 16:24:49 +0100 Subject: [PATCH 1/3] chore: wait for the configApi to be initialised before using the featureFlagsApi to register plugin output Signed-off-by: ben@blam.sh Signed-off-by: blam --- packages/core-api/src/app/App.test.tsx | 67 ++++++++++++++++++++++++++ packages/core-api/src/app/App.tsx | 50 +++++++++++-------- 2 files changed, 97 insertions(+), 20 deletions(-) diff --git a/packages/core-api/src/app/App.test.tsx b/packages/core-api/src/app/App.test.tsx index b9fdf05444..40212495b5 100644 --- a/packages/core-api/src/app/App.test.tsx +++ b/packages/core-api/src/app/App.test.tsx @@ -14,6 +14,12 @@ * limitations under the License. */ +import { + configApiRef, + createApiFactory, + featureFlagsApiRef, + LocalStorageFeatureFlags, +} from '../apis'; import { renderWithEffects, withLogCollector } from '@backstage/test-utils'; import { lightTheme } from '@backstage/theme'; import { render, screen } from '@testing-library/react'; @@ -207,6 +213,67 @@ describe('Integration Test', () => { expect(screen.getByText('errParamsOptional: ')).toBeInTheDocument(); }); + it('should wait for the config to load before calling feature flags', async () => { + const storageFlags = new LocalStorageFeatureFlags(); + jest.spyOn(storageFlags, 'registerFlag'); + + const apis = [ + createApiFactory({ + api: featureFlagsApiRef, + deps: { configApi: configApiRef }, + factory() { + return storageFlags; + }, + }), + ]; + + const app = new PrivateAppImpl({ + apis, + defaultApis: [], + themes: [ + { + id: 'light', + title: 'Light Theme', + variant: 'light', + theme: lightTheme, + }, + ], + icons: defaultSystemIcons, + plugins: [ + createPlugin({ + id: 'test', + register: p => p.featureFlags.register('name'), + }), + ], + components, + bindRoutes: ({ bind }) => { + bind(plugin1.externalRoutes, { + err: plugin1RouteRef, + errParams: plugin2RouteRef, + }); + }, + }); + + const Provider = app.getProvider(); + const Router = app.getRouter(); + + await renderWithEffects( + + + + + + + + , + ); + + expect(storageFlags.registerFlag).toHaveBeenCalledWith({ + name: 'name', + pluginId: 'test', + }); + }); + it('should throw some error when the route has duplicate params', () => { const app = new PrivateAppImpl({ apis: [], diff --git a/packages/core-api/src/app/App.tsx b/packages/core-api/src/app/App.tsx index 3c58930709..f83fb9373d 100644 --- a/packages/core-api/src/app/App.tsx +++ b/packages/core-api/src/app/App.tsx @@ -14,10 +14,12 @@ * limitations under the License. */ +import { Config } from '@backstage/config'; import React, { ComponentType, PropsWithChildren, ReactElement, + useEffect, useMemo, useState, } from 'react'; @@ -277,24 +279,6 @@ export class PrivateAppImpl implements BackstageApp { const appContext = new AppContextImpl(this); const apiHolder = this.getApiHolder(); - 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; - } - default: - break; - } - } - } - const Provider = ({ children }: PropsWithChildren<{}>) => { const appThemeApi = useMemo( () => AppThemeSelector.createWithStorage(this.themes), @@ -323,13 +307,39 @@ export class PrivateAppImpl implements BackstageApp { appThemeApi, ); + const hasConfigApi = 'api' in loadedConfig; + if (hasConfigApi) { + const { api } = loadedConfig as { api: Config }; + this.configApi = api; + } + + useEffect(() => { + if (hasConfigApi) { + 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; + } + default: + break; + } + } + } + } + }, [hasConfigApi, loadedConfig]); + if ('node' in loadedConfig) { // Loading or error return loadedConfig.node; } - this.configApi = loadedConfig.api; - return ( From 395885905fb0133ec64340d01715c98f5baa9791 Mon Sep 17 00:00:00 2001 From: blam Date: Tue, 9 Mar 2021 16:27:48 +0100 Subject: [PATCH 2/3] changeset: add changeset for core-api Signed-off-by: blam --- .changeset/tame-clouds-confess.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/tame-clouds-confess.md diff --git a/.changeset/tame-clouds-confess.md b/.changeset/tame-clouds-confess.md new file mode 100644 index 0000000000..3b84f5bbb6 --- /dev/null +++ b/.changeset/tame-clouds-confess.md @@ -0,0 +1,5 @@ +--- +'@backstage/core-api': patch +--- + +Wait for configApi to be ready before using featureFlagsApi From 77580b1dee48ad3960b3f6d29cf522d4e401f1e4 Mon Sep 17 00:00:00 2001 From: blam Date: Tue, 9 Mar 2021 16:31:07 +0100 Subject: [PATCH 3/3] changeset: fix vale Signed-off-by: blam --- .changeset/tame-clouds-confess.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/tame-clouds-confess.md b/.changeset/tame-clouds-confess.md index 3b84f5bbb6..05af248eb1 100644 --- a/.changeset/tame-clouds-confess.md +++ b/.changeset/tame-clouds-confess.md @@ -2,4 +2,4 @@ '@backstage/core-api': patch --- -Wait for configApi to be ready before using featureFlagsApi +Wait for `configApi` to be ready before using `featureFlagsApi`