From 9793bebce4ac675e9cf377723d951baf58328a3c Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 17 Jun 2020 16:58:09 +0200 Subject: [PATCH 1/3] packages/core-api: re-use Config type from config package --- .../core-api/src/apis/definitions/ConfigApi.ts | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/packages/core-api/src/apis/definitions/ConfigApi.ts b/packages/core-api/src/apis/definitions/ConfigApi.ts index 20676df899..63a588fc10 100644 --- a/packages/core-api/src/apis/definitions/ConfigApi.ts +++ b/packages/core-api/src/apis/definitions/ConfigApi.ts @@ -14,20 +14,7 @@ * limitations under the License. */ import { createApiRef } from '../ApiRef'; - -export type Config = { - getConfig(key: string): Config; - - getConfigArray(key: string): Config[]; - - getNumber(key: string): number | undefined; - - getBoolean(key: string): boolean | undefined; - - getString(key: string): string | undefined; - - getStringArray(key: string): string[] | undefined; -}; +import { Config } from '@backstage/config'; // Using interface to make the ConfigApi name show up in docs export interface ConfigApi extends Config {} From 31585800a4f320a76731f92259b3112e9518be1f Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 17 Jun 2020 17:15:31 +0200 Subject: [PATCH 2/3] packages/config: added must* variant for reading required primitive values --- packages/config/src/reader.test.ts | 25 +++++++++++++++++++++++ packages/config/src/reader.ts | 32 ++++++++++++++++++++++++++++++ packages/config/src/types.ts | 4 ++++ 3 files changed, 61 insertions(+) diff --git a/packages/config/src/reader.test.ts b/packages/config/src/reader.test.ts index 3d3287dfe7..e33939bb4b 100644 --- a/packages/config/src/reader.test.ts +++ b/packages/config/src/reader.test.ts @@ -49,6 +49,10 @@ function expectValidValues(config: ConfigReader) { 'string1', 'string2', ]); + expect(config.mustNumber('zero')).toBe(0); + expect(config.mustBoolean('true')).toBe(true); + expect(config.mustString('string')).toBe('string'); + expect(config.mustStringArray('strings')).toEqual(['string1', 'string2']); const [config1, config2, config3] = config.getConfigArray('nestlings'); expect(config1.getBoolean('boolean')).toBe(true); @@ -57,6 +61,9 @@ function expectValidValues(config: ConfigReader) { } function expectInvalidValues(config: ConfigReader) { + expect(() => config.getBoolean('string')).toThrow( + 'Invalid type in config for key string, got string, wanted boolean', + ); expect(() => config.getNumber('string')).toThrow( 'Invalid type in config for key string, got string, wanted number', ); @@ -87,6 +94,18 @@ function expectInvalidValues(config: ConfigReader) { expect(() => config.getConfigArray('one')).toThrow( 'Invalid type in config for key one, got number, wanted object-array', ); + expect(() => config.mustBoolean('missing')).toThrow( + "Missing required config value at 'missing'", + ); + expect(() => config.mustNumber('missing')).toThrow( + "Missing required config value at 'missing'", + ); + expect(() => config.mustString('missing')).toThrow( + "Missing required config value at 'missing'", + ); + expect(() => config.mustStringArray('missing')).toThrow( + "Missing required config value at 'missing'", + ); } describe('ConfigReader', () => { @@ -210,6 +229,12 @@ describe('ConfigReader with fallback', () => { // Config arrays aren't merged either expect(config.getConfigArray('merged.configs').length).toBe(1); expect(config.getConfigArray('merged.configs')[0].getString('a')).toBe('a'); + expect(config.getConfigArray('merged.configs')[0].mustString('a')).toBe( + 'a', + ); + expect(() => + config.getConfigArray('merged.configs')[0].mustString('missing'), + ).toThrow("Missing required config value at 'missing'"); expect( config.getConfigArray('merged.configs')[0].getString('b'), ).toBeUndefined(); diff --git a/packages/config/src/reader.ts b/packages/config/src/reader.ts index 95c4f47383..7878a392ff 100644 --- a/packages/config/src/reader.ts +++ b/packages/config/src/reader.ts @@ -92,6 +92,14 @@ export class ConfigReader implements Config { return (configs ?? []).map(obj => new ConfigReader(obj)); } + mustNumber(key: string): number { + const value = this.getNumber(key); + if (value === undefined) { + throw new Error(`Missing required config value at '${key}'`); + } + return value; + } + getNumber(key: string): number | undefined { return this.readConfigValue( key, @@ -99,6 +107,14 @@ export class ConfigReader implements Config { ); } + mustBoolean(key: string): boolean { + const value = this.getBoolean(key); + if (value === undefined) { + throw new Error(`Missing required config value at '${key}'`); + } + return value; + } + getBoolean(key: string): boolean | undefined { return this.readConfigValue( key, @@ -106,6 +122,14 @@ export class ConfigReader implements Config { ); } + mustString(key: string): string { + const value = this.getString(key); + if (value === undefined) { + throw new Error(`Missing required config value at '${key}'`); + } + return value; + } + getString(key: string): string | undefined { return this.readConfigValue( key, @@ -114,6 +138,14 @@ export class ConfigReader implements Config { ); } + mustStringArray(key: string): string[] { + const value = this.getStringArray(key); + if (value === undefined) { + throw new Error(`Missing required config value at '${key}'`); + } + return value; + } + getStringArray(key: string): string[] | undefined { return this.readConfigValue(key, values => { if (!Array.isArray(values)) { diff --git a/packages/config/src/types.ts b/packages/config/src/types.ts index 9eda6c8b32..73b8e42130 100644 --- a/packages/config/src/types.ts +++ b/packages/config/src/types.ts @@ -32,10 +32,14 @@ export type Config = { getConfigArray(key: string): Config[]; getNumber(key: string): number | undefined; + mustNumber(key: string): number; getBoolean(key: string): boolean | undefined; + mustBoolean(key: string): boolean; getString(key: string): string | undefined; + mustString(key: string): string; getStringArray(key: string): string[] | undefined; + mustStringArray(key: string): string[]; }; From 3dcef70e3c100c059afe1de4c774c32564b8f938 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 17 Jun 2020 20:10:44 +0200 Subject: [PATCH 3/3] packages/config: flip around must* and get* to get* and getOptional* --- packages/cli/src/lib/bundler/config.ts | 6 --- packages/config/src/reader.test.ts | 45 ++++++++++--------- packages/config/src/reader.ts | 24 +++++----- packages/config/src/types.ts | 16 +++---- packages/core-api/src/app/App.tsx | 2 +- .../core/src/layout/SignInPage/SignInPage.tsx | 2 +- .../components/WelcomePage/WelcomePage.tsx | 3 +- 7 files changed, 47 insertions(+), 51 deletions(-) diff --git a/packages/cli/src/lib/bundler/config.ts b/packages/cli/src/lib/bundler/config.ts index 50d174368a..d5fb674ec1 100644 --- a/packages/cli/src/lib/bundler/config.ts +++ b/packages/cli/src/lib/bundler/config.ts @@ -33,9 +33,6 @@ import { BundlingOptions, BackendBundlingOptions } from './types'; export function resolveBaseUrl(config: Config): URL { const baseUrl = config.getString('app.baseUrl'); - if (!baseUrl) { - throw new Error('app.baseUrl must be set in config'); - } try { return new URL(baseUrl, 'http://localhost:3000'); } catch (error) { @@ -52,9 +49,6 @@ export function createConfig( const { plugins, loaders } = transforms(options); const baseUrl = options.config.getString('app.baseUrl'); - if (!baseUrl) { - throw new Error('app.baseUrl must be set in config'); - } const validBaseUrl = new URL(baseUrl, 'https://backstage-app.dev'); if (checksEnabled) { diff --git a/packages/config/src/reader.test.ts b/packages/config/src/reader.test.ts index e33939bb4b..f7804d5d49 100644 --- a/packages/config/src/reader.test.ts +++ b/packages/config/src/reader.test.ts @@ -49,10 +49,10 @@ function expectValidValues(config: ConfigReader) { 'string1', 'string2', ]); - expect(config.mustNumber('zero')).toBe(0); - expect(config.mustBoolean('true')).toBe(true); - expect(config.mustString('string')).toBe('string'); - expect(config.mustStringArray('strings')).toEqual(['string1', 'string2']); + expect(config.getNumber('zero')).toBe(0); + expect(config.getBoolean('true')).toBe(true); + expect(config.getString('string')).toBe('string'); + expect(config.getStringArray('strings')).toEqual(['string1', 'string2']); const [config1, config2, config3] = config.getConfigArray('nestlings'); expect(config1.getBoolean('boolean')).toBe(true); @@ -94,16 +94,16 @@ function expectInvalidValues(config: ConfigReader) { expect(() => config.getConfigArray('one')).toThrow( 'Invalid type in config for key one, got number, wanted object-array', ); - expect(() => config.mustBoolean('missing')).toThrow( + expect(() => config.getBoolean('missing')).toThrow( "Missing required config value at 'missing'", ); - expect(() => config.mustNumber('missing')).toThrow( + expect(() => config.getNumber('missing')).toThrow( "Missing required config value at 'missing'", ); - expect(() => config.mustString('missing')).toThrow( + expect(() => config.getString('missing')).toThrow( "Missing required config value at 'missing'", ); - expect(() => config.mustStringArray('missing')).toThrow( + expect(() => config.getStringArray('missing')).toThrow( "Missing required config value at 'missing'", ); } @@ -111,13 +111,13 @@ function expectInvalidValues(config: ConfigReader) { describe('ConfigReader', () => { it('should read empty config with valid keys', () => { const config = new ConfigReader({}); - expect(config.getString('x')).toBeUndefined(); - expect(config.getString('x_x')).toBeUndefined(); - expect(config.getString('x-X')).toBeUndefined(); - expect(config.getString('x0')).toBeUndefined(); - expect(config.getString('X-x2')).toBeUndefined(); - expect(config.getString('x0_x0')).toBeUndefined(); - expect(config.getString('x_x-x_x')).toBeUndefined(); + expect(config.getOptionalString('x')).toBeUndefined(); + expect(config.getOptionalString('x_x')).toBeUndefined(); + expect(config.getOptionalString('x-X')).toBeUndefined(); + expect(config.getOptionalString('x0')).toBeUndefined(); + expect(config.getOptionalString('X-x2')).toBeUndefined(); + expect(config.getOptionalString('x0_x0')).toBeUndefined(); + expect(config.getOptionalString('x_x-x_x')).toBeUndefined(); }); it('should throw on invalid keys', () => { @@ -154,7 +154,7 @@ describe('ConfigReader', () => { describe('ConfigReader with fallback', () => { it('should behave as if without fallback', () => { const config = new ConfigReader({}, new ConfigReader(DATA)); - expect(config.getString('x')).toBeUndefined(); + expect(config.getOptionalString('x')).toBeUndefined(); expect(() => config.getString('.')).toThrow(/^Invalid config key/); expect(() => config.getString('a.')).toThrow(/^Invalid config key/); }); @@ -229,14 +229,12 @@ describe('ConfigReader with fallback', () => { // Config arrays aren't merged either expect(config.getConfigArray('merged.configs').length).toBe(1); expect(config.getConfigArray('merged.configs')[0].getString('a')).toBe('a'); - expect(config.getConfigArray('merged.configs')[0].mustString('a')).toBe( - 'a', - ); + expect(config.getConfigArray('merged.configs')[0].getString('a')).toBe('a'); expect(() => - config.getConfigArray('merged.configs')[0].mustString('missing'), + config.getConfigArray('merged.configs')[0].getString('missing'), ).toThrow("Missing required config value at 'missing'"); expect( - config.getConfigArray('merged.configs')[0].getString('b'), + config.getConfigArray('merged.configs')[0].getOptionalString('b'), ).toBeUndefined(); // Config arrays aren't merged either @@ -245,7 +243,10 @@ describe('ConfigReader with fallback', () => { config.getConfig('merged').getConfigArray('configs')[0].getString('a'), ).toBe('a'); expect( - config.getConfig('merged').getConfigArray('configs')[0].getString('b'), + config + .getConfig('merged') + .getConfigArray('configs')[0] + .getOptionalString('b'), ).toBeUndefined(); }); }); diff --git a/packages/config/src/reader.ts b/packages/config/src/reader.ts index 7878a392ff..ff7c8be6e5 100644 --- a/packages/config/src/reader.ts +++ b/packages/config/src/reader.ts @@ -92,45 +92,45 @@ export class ConfigReader implements Config { return (configs ?? []).map(obj => new ConfigReader(obj)); } - mustNumber(key: string): number { - const value = this.getNumber(key); + getNumber(key: string): number { + const value = this.getOptionalNumber(key); if (value === undefined) { throw new Error(`Missing required config value at '${key}'`); } return value; } - getNumber(key: string): number | undefined { + getOptionalNumber(key: string): number | undefined { return this.readConfigValue( key, value => typeof value === 'number' || { expected: 'number' }, ); } - mustBoolean(key: string): boolean { - const value = this.getBoolean(key); + getBoolean(key: string): boolean { + const value = this.getOptionalBoolean(key); if (value === undefined) { throw new Error(`Missing required config value at '${key}'`); } return value; } - getBoolean(key: string): boolean | undefined { + getOptionalBoolean(key: string): boolean | undefined { return this.readConfigValue( key, value => typeof value === 'boolean' || { expected: 'boolean' }, ); } - mustString(key: string): string { - const value = this.getString(key); + getString(key: string): string { + const value = this.getOptionalString(key); if (value === undefined) { throw new Error(`Missing required config value at '${key}'`); } return value; } - getString(key: string): string | undefined { + getOptionalString(key: string): string | undefined { return this.readConfigValue( key, value => @@ -138,15 +138,15 @@ export class ConfigReader implements Config { ); } - mustStringArray(key: string): string[] { - const value = this.getStringArray(key); + getStringArray(key: string): string[] { + const value = this.getOptionalStringArray(key); if (value === undefined) { throw new Error(`Missing required config value at '${key}'`); } return value; } - getStringArray(key: string): string[] | undefined { + getOptionalStringArray(key: string): string[] | undefined { return this.readConfigValue(key, values => { if (!Array.isArray(values)) { return { expected: 'string-array' }; diff --git a/packages/config/src/types.ts b/packages/config/src/types.ts index 73b8e42130..03c9dc4383 100644 --- a/packages/config/src/types.ts +++ b/packages/config/src/types.ts @@ -31,15 +31,15 @@ export type Config = { getConfigArray(key: string): Config[]; - getNumber(key: string): number | undefined; - mustNumber(key: string): number; + getNumber(key: string): number; + getOptionalNumber(key: string): number | undefined; - getBoolean(key: string): boolean | undefined; - mustBoolean(key: string): boolean; + getBoolean(key: string): boolean; + getOptionalBoolean(key: string): boolean | undefined; - getString(key: string): string | undefined; - mustString(key: string): string; + getString(key: string): string; + getOptionalString(key: string): string | undefined; - getStringArray(key: string): string[] | undefined; - mustStringArray(key: string): string[]; + getStringArray(key: string): string[]; + getOptionalStringArray(key: string): string[] | undefined; }; diff --git a/packages/core-api/src/app/App.tsx b/packages/core-api/src/app/App.tsx index 163a546943..12f36a6e8b 100644 --- a/packages/core-api/src/app/App.tsx +++ b/packages/core-api/src/app/App.tsx @@ -282,7 +282,7 @@ export class PrivateAppImpl implements BackstageApp { const configApi = useApi(configApiRef); let { pathname } = new URL( - configApi.getString('app.baseUrl') ?? '/', + configApi.getOptionalString('app.baseUrl') ?? '/', 'http://dummy.dev', // baseUrl can be specified as just a path ); if (pathname.endsWith('/')) { diff --git a/packages/core/src/layout/SignInPage/SignInPage.tsx b/packages/core/src/layout/SignInPage/SignInPage.tsx index 91bc251f90..5911fe86b2 100644 --- a/packages/core/src/layout/SignInPage/SignInPage.tsx +++ b/packages/core/src/layout/SignInPage/SignInPage.tsx @@ -39,7 +39,7 @@ export const SignInPage: FC = ({ onResult, providers }) => { return ( -
+
{providerElements} diff --git a/plugins/welcome/src/components/WelcomePage/WelcomePage.tsx b/plugins/welcome/src/components/WelcomePage/WelcomePage.tsx index 6a1b0e0008..ce151737b3 100644 --- a/plugins/welcome/src/components/WelcomePage/WelcomePage.tsx +++ b/plugins/welcome/src/components/WelcomePage/WelcomePage.tsx @@ -39,7 +39,8 @@ import { } from '@backstage/core'; const WelcomePage: FC<{}> = () => { - const appTitle = useApi(configApiRef).getString('app.title') ?? 'Backstage'; + const appTitle = + useApi(configApiRef).getOptionalString('app.title') ?? 'Backstage'; const profile = { givenName: '' }; return (