From 20b48dfed7a91d50a948d05e7212ae09077b88d0 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 24 May 2020 20:38:38 +0200 Subject: [PATCH] packages/core: only export createApiRef + dashes in IDs only factory objects for ApiTestRegistry --- .../core/src/api/apis/ApiAggregator.test.ts | 6 +++--- .../core/src/api/apis/ApiProvider.test.tsx | 6 +++--- packages/core/src/api/apis/ApiRef.test.ts | 14 ++++++------- packages/core/src/api/apis/ApiRef.ts | 20 ++++++++++++++----- .../core/src/api/apis/ApiRegistry.test.ts | 6 +++--- .../core/src/api/apis/ApiTestRegistry.test.ts | 20 +++++++++---------- packages/core/src/api/apis/ApiTestRegistry.ts | 17 ++-------------- .../core/src/api/apis/definitions/AlertApi.ts | 4 ++-- .../src/api/apis/definitions/AppThemeApi.ts | 4 ++-- .../core/src/api/apis/definitions/ErrorApi.ts | 4 ++-- .../api/apis/definitions/FeatureFlagsApi.ts | 4 ++-- .../api/apis/definitions/OAuthRequestApi.ts | 4 ++-- .../core/src/api/apis/definitions/auth.ts | 8 ++++---- plugins/circleci/src/api/index.ts | 4 ++-- plugins/graphiql/src/lib/api/types.ts | 4 ++-- plugins/lighthouse/src/api.ts | 4 ++-- plugins/tech-radar/src/api.ts | 4 ++-- 17 files changed, 65 insertions(+), 68 deletions(-) diff --git a/packages/core/src/api/apis/ApiAggregator.test.ts b/packages/core/src/api/apis/ApiAggregator.test.ts index a230e0cfff..2d14087fa2 100644 --- a/packages/core/src/api/apis/ApiAggregator.test.ts +++ b/packages/core/src/api/apis/ApiAggregator.test.ts @@ -15,12 +15,12 @@ */ import { ApiAggregator } from './ApiAggregator'; -import { ApiRef } from './ApiRef'; +import { createApiRef } from './ApiRef'; import { ApiRegistry } from './ApiRegistry'; describe('ApiAggregator', () => { - const apiARef = new ApiRef({ id: 'a', description: '' }); - const apiBRef = new ApiRef({ id: 'b', description: '' }); + const apiARef = createApiRef({ id: 'a', description: '' }); + const apiBRef = createApiRef({ id: 'b', description: '' }); it('should forward implementations', () => { const agg = new ApiAggregator( diff --git a/packages/core/src/api/apis/ApiProvider.test.tsx b/packages/core/src/api/apis/ApiProvider.test.tsx index e79e83a1ed..255b35a0d2 100644 --- a/packages/core/src/api/apis/ApiProvider.test.tsx +++ b/packages/core/src/api/apis/ApiProvider.test.tsx @@ -16,14 +16,14 @@ import React from 'react'; import { ApiProvider, useApi, withApis } from './ApiProvider'; -import { ApiRef } from './ApiRef'; +import { createApiRef } from './ApiRef'; import { ApiRegistry } from './ApiRegistry'; import { render } from '@testing-library/react'; import { withLogCollector } from '@backstage/test-utils-core'; describe('ApiProvider', () => { type Api = () => string; - const apiRef = new ApiRef({ id: 'x', description: '' }); + const apiRef = createApiRef({ id: 'x', description: '' }); const registry = ApiRegistry.from([[apiRef, () => 'hello']]); const MyHookConsumer = () => { @@ -53,7 +53,7 @@ describe('ApiProvider', () => { it('should ignore deps in prototype', () => { // 100% coverage + happy typescript = hasOwnProperty + this atrocity - const xRef = new ApiRef({ id: 'x', description: '' }); + const xRef = createApiRef({ id: 'x', description: '' }); const proto = { x: xRef }; const props = { getMessage: { enumerable: true, value: apiRef } }; diff --git a/packages/core/src/api/apis/ApiRef.test.ts b/packages/core/src/api/apis/ApiRef.test.ts index 93ca639b46..b9ea1470cd 100644 --- a/packages/core/src/api/apis/ApiRef.test.ts +++ b/packages/core/src/api/apis/ApiRef.test.ts @@ -14,11 +14,11 @@ * limitations under the License. */ -import { ApiRef } from './ApiRef'; +import { createApiRef } from './ApiRef'; describe('ApiRef', () => { it('should be created', () => { - const ref = new ApiRef({ id: 'abc', description: '123' }); + const ref = createApiRef({ id: 'abc', description: '123' }); expect(ref.id).toBe('abc'); expect(ref.description).toBe('123'); expect(String(ref)).toBe('apiRef{abc}'); @@ -26,13 +26,13 @@ describe('ApiRef', () => { }); it('should reject invalid ids', () => { - for (const id of ['a', 'abc', 'a.b.c', 'ab.c', 'abc.abc.abc3']) { - expect(new ApiRef({ id, description: '123' }).id).toBe(id); + for (const id of ['a', 'abc', 'ab-c', 'a.b.c', 'a-b.c', 'abc.a-b-c.abc3']) { + expect(createApiRef({ id, description: '123' }).id).toBe(id); } for (const id of [ '123', - 'ab-c', + 'ab-3', 'ab_c', '.', '2ac', @@ -43,8 +43,8 @@ describe('ApiRef', () => { '', '_', ]) { - expect(() => new ApiRef({ id, description: '123' }).id).toThrow( - `API id must only contain lowercase alphanums separated by dots, got '${id}'`, + expect(() => createApiRef({ id, description: '123' }).id).toThrow( + `API id must only contain period separated lowercase alphanum tokens with dashes, got '${id}'`, ); } }); diff --git a/packages/core/src/api/apis/ApiRef.ts b/packages/core/src/api/apis/ApiRef.ts index c346f89685..793c1e5386 100644 --- a/packages/core/src/api/apis/ApiRef.ts +++ b/packages/core/src/api/apis/ApiRef.ts @@ -19,11 +19,21 @@ export type ApiRefConfig = { description: string; }; -export class ApiRef { +export type ApiRef = { + id: string; + description: string; + T: T; +}; + +class ApiRefImpl implements ApiRef { constructor(private readonly config: ApiRefConfig) { - if (!config.id.match(/^[a-z][a-z0-9]*(\.[a-z][a-z0-9]*)*$/)) { + const valid = config.id + .split('.') + .flatMap(part => part.split('-')) + .every(part => part.match(/^[a-z][a-z0-9]*$/)); + if (!valid) { throw new Error( - `API id must only contain lowercase alphanums separated by dots, got '${config.id}'`, + `API id must only contain period separated lowercase alphanum tokens with dashes, got '${config.id}'`, ); } } @@ -46,6 +56,6 @@ export class ApiRef { } } -export function createApiRef(config: ApiRefConfig) { - return new ApiRef(config); +export function createApiRef(config: ApiRefConfig): ApiRef { + return new ApiRefImpl(config); } diff --git a/packages/core/src/api/apis/ApiRegistry.test.ts b/packages/core/src/api/apis/ApiRegistry.test.ts index a4043bbe1e..2971397e7a 100644 --- a/packages/core/src/api/apis/ApiRegistry.test.ts +++ b/packages/core/src/api/apis/ApiRegistry.test.ts @@ -15,11 +15,11 @@ */ import { ApiRegistry } from './ApiRegistry'; -import { ApiRef } from './ApiRef'; +import { createApiRef } from './ApiRef'; describe('ApiRegistry', () => { - const x1Ref = new ApiRef({ id: 'x', description: '' }); - const x2Ref = new ApiRef({ id: 'x', description: '' }); + const x1Ref = createApiRef({ id: 'x', description: '' }); + const x2Ref = createApiRef({ id: 'x', description: '' }); it('should be created', () => { const registry = ApiRegistry.from([]); diff --git a/packages/core/src/api/apis/ApiTestRegistry.test.ts b/packages/core/src/api/apis/ApiTestRegistry.test.ts index 6ae7a83d7e..fa42e64707 100644 --- a/packages/core/src/api/apis/ApiTestRegistry.test.ts +++ b/packages/core/src/api/apis/ApiTestRegistry.test.ts @@ -15,12 +15,12 @@ */ import { ApiTestRegistry } from './ApiTestRegistry'; -import { ApiRef } from './ApiRef'; +import { createApiRef } from './ApiRef'; describe('ApiTestRegistry', () => { - const aRef = new ApiRef({ id: 'a', description: '' }); - const bRef = new ApiRef({ id: 'b', description: '' }); - const cRef = new ApiRef({ id: 'c', description: '' }); + const aRef = createApiRef({ id: 'a', description: '' }); + const bRef = createApiRef({ id: 'b', description: '' }); + const cRef = createApiRef({ id: 'c', description: '' }); it('should be created', () => { const registry = new ApiTestRegistry(); @@ -31,7 +31,7 @@ describe('ApiTestRegistry', () => { it('should register a factory', () => { const registry = new ApiTestRegistry(); - registry.register(aRef, () => 3); + registry.register({ implements: aRef, deps: {}, factory: () => 3 }); expect(registry.get(aRef)).toBe(3); expect(registry.get(bRef)).toBe(undefined); expect(registry.get(cRef)).toBe(undefined); @@ -39,7 +39,7 @@ describe('ApiTestRegistry', () => { it('should remove factories when resetting', () => { const registry = new ApiTestRegistry(); - registry.register(aRef, () => 3); + registry.register({ implements: aRef, deps: {}, factory: () => 3 }); expect(registry.get(aRef)).toBe(3); registry.reset(); expect(registry.get(aRef)).toBe(undefined); @@ -47,9 +47,9 @@ describe('ApiTestRegistry', () => { it('should keep saved factories when resetting', () => { const registry = new ApiTestRegistry(); - registry.register(aRef, () => 3); + registry.register({ implements: aRef, deps: {}, factory: () => 3 }); registry.save(); - registry.register(bRef, () => 'x'); + registry.register({ implements: bRef, deps: {}, factory: () => 'x' }); expect(registry.get(aRef)).toBe(3); expect(registry.get(bRef)).toBe('x'); registry.reset(); @@ -127,7 +127,7 @@ describe('ApiTestRegistry', () => { it('should only call factory func once', () => { const registry = new ApiTestRegistry(); const factory = jest.fn().mockReturnValue(2); - registry.register(aRef, factory); + registry.register({ implements: aRef, deps: {}, factory }); expect(factory).toHaveBeenCalledTimes(0); expect(registry.get(aRef)).toBe(2); @@ -139,7 +139,7 @@ describe('ApiTestRegistry', () => { it('should call factory again after reset', () => { const registry = new ApiTestRegistry(); const factory = jest.fn().mockReturnValue(2); - registry.register(aRef, factory); + registry.register({ implements: aRef, deps: {}, factory }); registry.save(); expect(factory).toHaveBeenCalledTimes(0); diff --git a/packages/core/src/api/apis/ApiTestRegistry.ts b/packages/core/src/api/apis/ApiTestRegistry.ts index eacc4962ce..7aed3c7917 100644 --- a/packages/core/src/api/apis/ApiTestRegistry.ts +++ b/packages/core/src/api/apis/ApiTestRegistry.ts @@ -32,21 +32,8 @@ export class ApiTestRegistry implements ApiHolder { return this.load(ref); } - register(ref: ApiRef, factoryFunc: () => T): ApiTestRegistry; - register(factory: ApiFactory): ApiTestRegistry; - register( - factory: ApiRef | ApiFactory, - factoryFunc?: () => T, - ): ApiTestRegistry { - if (factory instanceof ApiRef) { - this.factories.set(factory, { - implements: factory, - deps: {}, - factory: factoryFunc!, - }); - } else { - this.factories.set(factory.implements, factory); - } + register(factory: ApiFactory): ApiTestRegistry { + this.factories.set(factory.implements, factory); return this; } diff --git a/packages/core/src/api/apis/definitions/AlertApi.ts b/packages/core/src/api/apis/definitions/AlertApi.ts index e5d7001b56..9776a96d84 100644 --- a/packages/core/src/api/apis/definitions/AlertApi.ts +++ b/packages/core/src/api/apis/definitions/AlertApi.ts @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { ApiRef } from '../ApiRef'; +import { createApiRef } from '../ApiRef'; export type AlertMessage = { message: string; @@ -32,7 +32,7 @@ export type AlertApi = { post(alert: AlertMessage): void; }; -export const alertApiRef = new ApiRef({ +export const alertApiRef = createApiRef({ id: 'core.alert', description: 'Used to report alerts and forward them to the app', }); diff --git a/packages/core/src/api/apis/definitions/AppThemeApi.ts b/packages/core/src/api/apis/definitions/AppThemeApi.ts index 738bae8a9f..de70adabd9 100644 --- a/packages/core/src/api/apis/definitions/AppThemeApi.ts +++ b/packages/core/src/api/apis/definitions/AppThemeApi.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { ApiRef } from '../ApiRef'; +import { createApiRef } from '../ApiRef'; import { BackstageTheme } from '@backstage/theme'; import { Observable } from '../../types'; @@ -71,7 +71,7 @@ export type AppThemeApi = { setActiveThemeId(themeId?: string): void; }; -export const appThemeApiRef = new ApiRef({ +export const appThemeApiRef = createApiRef({ id: 'core.apptheme', description: 'API Used to configure the app theme, and enumerate options', }); diff --git a/packages/core/src/api/apis/definitions/ErrorApi.ts b/packages/core/src/api/apis/definitions/ErrorApi.ts index 7d9816426e..c481547459 100644 --- a/packages/core/src/api/apis/definitions/ErrorApi.ts +++ b/packages/core/src/api/apis/definitions/ErrorApi.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { ApiRef } from '../ApiRef'; +import { createApiRef } from '../ApiRef'; /** * Mirrors the javascript Error class, for the purpose of @@ -56,7 +56,7 @@ export type ErrorApi = { post(error: Error, context?: ErrorContext): void; }; -export const errorApiRef = new ApiRef({ +export const errorApiRef = createApiRef({ id: 'core.error', description: 'Used to report errors and forward them to the app', }); diff --git a/packages/core/src/api/apis/definitions/FeatureFlagsApi.ts b/packages/core/src/api/apis/definitions/FeatureFlagsApi.ts index 8636c1f5c1..b6aac75620 100644 --- a/packages/core/src/api/apis/definitions/FeatureFlagsApi.ts +++ b/packages/core/src/api/apis/definitions/FeatureFlagsApi.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { ApiRef } from '../ApiRef'; +import { createApiRef } from '../ApiRef'; import { UserFlags, FeatureFlagsRegistry, @@ -55,7 +55,7 @@ export interface FeatureFlagsApi { getRegisteredFlags(): FeatureFlagsRegistry; } -export const featureFlagsApiRef = new ApiRef({ +export const featureFlagsApiRef = createApiRef({ id: 'core.featureflags', description: 'Used to toggle functionality in features across Backstage', }); diff --git a/packages/core/src/api/apis/definitions/OAuthRequestApi.ts b/packages/core/src/api/apis/definitions/OAuthRequestApi.ts index 52d30a763e..5a0f399a6d 100644 --- a/packages/core/src/api/apis/definitions/OAuthRequestApi.ts +++ b/packages/core/src/api/apis/definitions/OAuthRequestApi.ts @@ -16,7 +16,7 @@ import { IconComponent } from '../../../icons'; import { Observable } from '../../types'; -import { ApiRef } from '../ApiRef'; +import { createApiRef } from '../ApiRef'; /** * Information about the auth provider that we're requesting a login towards. @@ -127,7 +127,7 @@ export type OAuthRequestApi = { authRequest$(): Observable; }; -export const oauthRequestApiRef = new ApiRef({ +export const oauthRequestApiRef = createApiRef({ id: 'core.oauthrequest', description: 'An API for implementing unified OAuth flows in Backstage', }); diff --git a/packages/core/src/api/apis/definitions/auth.ts b/packages/core/src/api/apis/definitions/auth.ts index 092b496488..15c9e5d632 100644 --- a/packages/core/src/api/apis/definitions/auth.ts +++ b/packages/core/src/api/apis/definitions/auth.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { ApiRef } from '../ApiRef'; +import { createApiRef } from '../ApiRef'; /** * This file contains declarations for common interfaces of auth-related APIs. @@ -24,7 +24,7 @@ import { ApiRef } from '../ApiRef'; * For example, a Google OAuth provider that supports OAuth 2 and OpenID Connect, * would be declared as follows: * - * const googleAuthApiRef = new ApiRef({ ... }) + * const googleAuthApiRef = createApiRef({ ... }) */ /** @@ -148,7 +148,7 @@ export type OpenIdConnectApi = { * Note that the ID token payload is only guaranteed to contain the user's numerical Google ID, * email and expiration information. Do not rely on any other fields, as they might not be present. */ -export const googleAuthApiRef = new ApiRef({ +export const googleAuthApiRef = createApiRef({ id: 'core.auth.google', description: 'Provides authentication towards Google APIs and identities', }); @@ -159,7 +159,7 @@ export const googleAuthApiRef = new ApiRef({ * See https://developer.github.com/apps/building-oauth-apps/understanding-scopes-for-oauth-apps/ * for a full list of supported scopes. */ -export const githubAuthApiRef = new ApiRef({ +export const githubAuthApiRef = createApiRef({ id: 'core.auth.github', description: 'Provides authentication towards Github APIs', }); diff --git a/plugins/circleci/src/api/index.ts b/plugins/circleci/src/api/index.ts index f683f79ff9..26b0213e3a 100644 --- a/plugins/circleci/src/api/index.ts +++ b/plugins/circleci/src/api/index.ts @@ -26,11 +26,11 @@ import { BuildSummary, GitType, } from 'circleci-api'; -import { ApiRef } from '@backstage/core'; +import { createApiRef } from '@backstage/core'; export { BuildWithSteps, BuildStepAction, BuildSummary, GitType }; -export const circleCIApiRef = new ApiRef({ +export const circleCIApiRef = createApiRef({ id: 'plugin.circleci.service', description: 'Used by the CircleCI plugin to make requests', }); diff --git a/plugins/graphiql/src/lib/api/types.ts b/plugins/graphiql/src/lib/api/types.ts index 6c41e2aaa0..dad06b8359 100644 --- a/plugins/graphiql/src/lib/api/types.ts +++ b/plugins/graphiql/src/lib/api/types.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { ApiRef } from '@backstage/core'; +import { createApiRef } from '@backstage/core'; export type GraphQLEndpoint = { // Will be used as unique key for storing history and query data @@ -33,7 +33,7 @@ export type GraphQLBrowseApi = { getEndpoints(): Promise; }; -export const graphQlBrowseApiRef = new ApiRef({ +export const graphQlBrowseApiRef = createApiRef({ id: 'plugin.graphiql.browse', description: 'Used to supply GraphQL endpoints for browsing', }); diff --git a/plugins/lighthouse/src/api.ts b/plugins/lighthouse/src/api.ts index 0fb2139301..9c656c88ef 100644 --- a/plugins/lighthouse/src/api.ts +++ b/plugins/lighthouse/src/api.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { ApiRef } from '@backstage/core'; +import { createApiRef } from '@backstage/core'; export type LighthouseCategoryId = | 'pwa' @@ -105,7 +105,7 @@ export type LighthouseApi = { triggerAudit: (payload: TriggerAuditPayload) => Promise; }; -export const lighthouseApiRef = new ApiRef({ +export const lighthouseApiRef = createApiRef({ id: 'plugin.lighthouse.service', description: 'Used by the Lighthouse plugin to make requests', }); diff --git a/plugins/tech-radar/src/api.ts b/plugins/tech-radar/src/api.ts index 4893901342..2e068fe49a 100644 --- a/plugins/tech-radar/src/api.ts +++ b/plugins/tech-radar/src/api.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { ApiRef } from '@backstage/core'; +import { createApiRef } from '@backstage/core'; /** * Types related to the Radar's visualization. @@ -71,7 +71,7 @@ export interface TechRadarApi extends TechRadarComponentProps { subtitle?: string; } -export const techRadarApiRef = new ApiRef({ +export const techRadarApiRef = createApiRef({ id: 'plugin.techradar', description: 'Used by the Tech Radar to render the visualization', });