Merge pull request #975 from spotify/rugvip/create

packages/core: switch to only using createApiRef

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<number>({ id: 'a', description: '' });
-  const apiBRef = new ApiRef<number>({ id: 'b', description: '' });
+  const apiARef = createApiRef<number>({ id: 'a', description: '' });
+  const apiBRef = createApiRef<number>({ id: 'b', description: '' });
 
   it('should forward implementations', () => {
     const agg = new ApiAggregator(

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<Api>({ id: 'x', description: '' });
+  const apiRef = createApiRef<Api>({ 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<number>({ id: 'x', description: '' });
+    const xRef = createApiRef<number>({ id: 'x', description: '' });
 
     const proto = { x: xRef };
     const props = { getMessage: { enumerable: true, value: apiRef } };

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}'`,
       );
     }
   });

packages/core/src/api/apis/ApiRef.ts

@@ -19,11 +19,21 @@ export type ApiRefConfig = {
   description: string;
 };
 
-export class ApiRef<T> {
+export type ApiRef<T> = {
+  id: string;
+  description: string;
+  T: T;
+};
+
+class ApiRefImpl<T> implements ApiRef<T> {
   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<T> {
   }
 }
 
-export function createApiRef<T>(config: ApiRefConfig) {
-  return new ApiRef<T>(config);
+export function createApiRef<T>(config: ApiRefConfig): ApiRef<T> {
+  return new ApiRefImpl<T>(config);
 }

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<number>({ id: 'x', description: '' });
-  const x2Ref = new ApiRef<string>({ id: 'x', description: '' });
+  const x1Ref = createApiRef<number>({ id: 'x', description: '' });
+  const x2Ref = createApiRef<string>({ id: 'x', description: '' });
 
   it('should be created', () => {
     const registry = ApiRegistry.from([]);

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<number>({ id: 'a', description: '' });
-  const bRef = new ApiRef<string>({ id: 'b', description: '' });
-  const cRef = new ApiRef<string>({ id: 'c', description: '' });
+  const aRef = createApiRef<number>({ id: 'a', description: '' });
+  const bRef = createApiRef<string>({ id: 'b', description: '' });
+  const cRef = createApiRef<string>({ 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);

packages/core/src/api/apis/ApiTestRegistry.ts

@@ -32,21 +32,8 @@ export class ApiTestRegistry implements ApiHolder {
     return this.load(ref);
   }
 
-  register<T>(ref: ApiRef<T>, factoryFunc: () => T): ApiTestRegistry;
-  register<A, I, D>(factory: ApiFactory<A, I, D>): ApiTestRegistry;
-  register<A, I, D, T>(
-    factory: ApiRef<T> | ApiFactory<A, I, D>,
-    factoryFunc?: () => T,
-  ): ApiTestRegistry {
-    if (factory instanceof ApiRef) {
-      this.factories.set(factory, {
-        implements: factory,
-        deps: {},
-        factory: factoryFunc!,
-      });
-    } else {
-      this.factories.set(factory.implements, factory);
-    }
+  register<A, I, D>(factory: ApiFactory<A, I, D>): ApiTestRegistry {
+    this.factories.set(factory.implements, factory);
     return this;
   }
 

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<AlertApi>({
+export const alertApiRef = createApiRef<AlertApi>({
   id: 'core.alert',
   description: 'Used to report alerts and forward them to the app',
 });

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<AppThemeApi>({
+export const appThemeApiRef = createApiRef<AppThemeApi>({
   id: 'core.apptheme',
   description: 'API Used to configure the app theme, and enumerate options',
 });

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<ErrorApi>({
+export const errorApiRef = createApiRef<ErrorApi>({
   id: 'core.error',
   description: 'Used to report errors and forward them to the app',
 });

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<FeatureFlagsApi>({
+export const featureFlagsApiRef = createApiRef<FeatureFlagsApi>({
   id: 'core.featureflags',
   description: 'Used to toggle functionality in features across Backstage',
 });

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<PendingAuthRequest[]>;
 };
 
-export const oauthRequestApiRef = new ApiRef<OAuthRequestApi>({
+export const oauthRequestApiRef = createApiRef<OAuthRequestApi>({
   id: 'core.oauthrequest',
   description: 'An API for implementing unified OAuth flows in Backstage',
 });

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<OAuthApi & OpenIDConnectApi>({ ... })
+ * const googleAuthApiRef = createApiRef<OAuthApi & OpenIDConnectApi>({ ... })
  */
 
 /**
@@ -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<OAuthApi & OpenIdConnectApi>({
+export const googleAuthApiRef = createApiRef<OAuthApi & OpenIdConnectApi>({
   id: 'core.auth.google',
   description: 'Provides authentication towards Google APIs and identities',
 });
@@ -159,7 +159,7 @@ export const googleAuthApiRef = new ApiRef<OAuthApi & OpenIdConnectApi>({
  * 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<OAuthApi>({
+export const githubAuthApiRef = createApiRef<OAuthApi>({
   id: 'core.auth.github',
   description: 'Provides authentication towards Github APIs',
 });

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<CircleCIApi>({
+export const circleCIApiRef = createApiRef<CircleCIApi>({
   id: 'plugin.circleci.service',
   description: 'Used by the CircleCI plugin to make requests',
 });

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<GraphQLEndpoint[]>;
 };
 
-export const graphQlBrowseApiRef = new ApiRef<GraphQLBrowseApi>({
+export const graphQlBrowseApiRef = createApiRef<GraphQLBrowseApi>({
   id: 'plugin.graphiql.browse',
   description: 'Used to supply GraphQL endpoints for browsing',
 });

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<Audit>;
 };
 
-export const lighthouseApiRef = new ApiRef<LighthouseApi>({
+export const lighthouseApiRef = createApiRef<LighthouseApi>({
   id: 'plugin.lighthouse.service',
   description: 'Used by the Lighthouse plugin to make requests',
 });

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<TechRadarApi>({
+export const techRadarApiRef = createApiRef<TechRadarApi>({
   id: 'plugin.techradar',
   description: 'Used by the Tech Radar to render the visualization',
 });