From ce55c4c40c6eb78b86d76557bfcf7ed328311426 Mon Sep 17 00:00:00 2001 From: blam Date: Tue, 9 Mar 2021 16:24:49 +0100 Subject: [PATCH] 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 (