diff --git a/.changeset/stupid-parents-smoke.md b/.changeset/stupid-parents-smoke.md new file mode 100644 index 0000000000..85a8d544be --- /dev/null +++ b/.changeset/stupid-parents-smoke.md @@ -0,0 +1,5 @@ +--- +'@backstage/core-api': patch +--- + +Internal refactor to allow for future package splits. As part of this `ApiRef`s are now identified by their ID rather than their reference. diff --git a/packages/core-api/package.json b/packages/core-api/package.json index cdc21264d3..dbeb2021df 100644 --- a/packages/core-api/package.json +++ b/packages/core-api/package.json @@ -47,6 +47,7 @@ "@backstage/test-utils-core": "^0.1.1", "@testing-library/jest-dom": "^5.10.1", "@testing-library/react": "^11.2.5", + "@testing-library/react-hooks": "^3.3.0", "@testing-library/user-event": "^12.0.7", "@types/jest": "^26.0.7", "@types/node": "^14.14.32", diff --git a/packages/core-api/src/apis/system/ApiFactoryRegistry.test.ts b/packages/core-api/src/apis/system/ApiFactoryRegistry.test.ts index 9f44adaef3..a51ed03fea 100644 --- a/packages/core-api/src/apis/system/ApiFactoryRegistry.test.ts +++ b/packages/core-api/src/apis/system/ApiFactoryRegistry.test.ts @@ -67,4 +67,25 @@ describe('ApiFactoryRegistry', () => { expect(registry.get(cRef)).toBe(cFactory); expect(registry.getAllApis()).toEqual(new Set([aRef, bRef, cRef])); }); + + it('should identify ApiRefs by id but still return the correct factory ref when listing all apis', () => { + const ref1 = createApiRef({ id: 'a', description: 'ref1' }); + const ref2 = createApiRef({ id: 'a', description: 'ref2' }); + + const factory1 = { api: ref1, deps: {}, factory: () => 3 }; + const factory2 = { api: ref2, deps: {}, factory: () => 3 }; + + const registry = new ApiFactoryRegistry(); + expect(registry.register('default', factory1)).toBe(true); + expect(registry.register('default', factory2)).toBe(false); + expect(registry.get(ref1)).toEqual(factory1); + expect(registry.get(ref2)).toEqual(factory1); + expect(registry.getAllApis()).toEqual(new Set([ref1])); + + expect(registry.register('app', factory2)).toBe(true); + expect(registry.get(ref1)).toEqual(factory2); + expect(registry.get(ref2)).toEqual(factory2); + expect(registry.getAllApis()).toEqual(new Set([ref2])); + expect(registry.getAllApis()).not.toEqual(new Set([ref1])); + }); }); diff --git a/packages/core-api/src/apis/system/ApiFactoryRegistry.ts b/packages/core-api/src/apis/system/ApiFactoryRegistry.ts index 9d9b058eac..c5a76ee1d4 100644 --- a/packages/core-api/src/apis/system/ApiFactoryRegistry.ts +++ b/packages/core-api/src/apis/system/ApiFactoryRegistry.ts @@ -46,7 +46,7 @@ type FactoryTuple = { * higher priority scopes override ones with lower priority. */ export class ApiFactoryRegistry implements ApiFactoryHolder { - private readonly factories = new Map(); + private readonly factories = new Map(); /** * Register a new API factory. Returns true if the factory was added @@ -60,19 +60,19 @@ export class ApiFactoryRegistry implements ApiFactoryHolder { factory: ApiFactory, ) { const priority = ScopePriority[scope]; - const existing = this.factories.get(factory.api); + const existing = this.factories.get(factory.api.id); if (existing && existing.priority >= priority) { return false; } - this.factories.set(factory.api, { priority, factory }); + this.factories.set(factory.api.id, { priority, factory }); return true; } get( api: ApiRef, ): ApiFactory | undefined { - const tuple = this.factories.get(api); + const tuple = this.factories.get(api.id); if (!tuple) { return undefined; } @@ -80,6 +80,10 @@ export class ApiFactoryRegistry implements ApiFactoryHolder { } getAllApis(): Set { - return new Set(this.factories.keys()); + const refs = new Set(); + for (const { factory } of this.factories.values()) { + refs.add(factory.api); + } + return refs; } } diff --git a/packages/core-api/src/apis/system/ApiProvider.test.tsx b/packages/core-api/src/apis/system/ApiProvider.test.tsx index 71269697c2..e95842aebc 100644 --- a/packages/core-api/src/apis/system/ApiProvider.test.tsx +++ b/packages/core-api/src/apis/system/ApiProvider.test.tsx @@ -14,12 +14,15 @@ * limitations under the License. */ -import React from 'react'; +import React, { Context, useContext } from 'react'; import { ApiProvider, useApi, withApis } from './ApiProvider'; import { createApiRef } from './ApiRef'; import { ApiRegistry } from './ApiRegistry'; import { render } from '@testing-library/react'; import { withLogCollector } from '@backstage/test-utils-core'; +import { getGlobalSingleton } from '../../lib/globalObject'; +import { ApiHolder, ApiRef } from './types'; +import { VersionedValue } from '../../lib/versionedValues'; describe('ApiProvider', () => { type Api = () => string; @@ -175,3 +178,35 @@ describe('ApiProvider', () => { ]); }); }); + +describe('v1 consumer', () => { + const ApiContext = getGlobalSingleton< + Context> + >('api-context'); + + function useMockApiV1(apiRef: ApiRef): T { + const impl = useContext(ApiContext)?.atVersion(1)?.get(apiRef); + if (!impl) { + throw new Error('no impl'); + } + return impl; + } + + type Api = () => string; + const apiRef = createApiRef({ id: 'x', description: '' }); + const registry = ApiRegistry.with(apiRef, () => 'hello'); + + const MyHookConsumerV1 = () => { + const api = useMockApiV1(apiRef); + return

hook message: {api()}

; + }; + + it('should provide apis', () => { + const renderedHook = render( + + + , + ); + renderedHook.getByText('hook message: hello'); + }); +}); diff --git a/packages/core-api/src/apis/system/ApiProvider.tsx b/packages/core-api/src/apis/system/ApiProvider.tsx index d41730cb59..807b2ff7e0 100644 --- a/packages/core-api/src/apis/system/ApiProvider.tsx +++ b/packages/core-api/src/apis/system/ApiProvider.tsx @@ -19,10 +19,20 @@ import React, { useContext, ReactNode, PropsWithChildren, + Context, + useMemo, } from 'react'; import PropTypes from 'prop-types'; import { ApiRef, ApiHolder, TypesToApiRefs } from './types'; import { ApiAggregator } from './ApiAggregator'; +import { + getGlobalSingleton, + getOrCreateGlobalSingleton, +} from '../../lib/globalObject'; +import { + VersionedValue, + createVersionedValueMap, +} from '../../lib/versionedValues'; const missingHolderMessage = 'No ApiProvider available in react context. ' + @@ -35,16 +45,25 @@ type ApiProviderProps = { children: ReactNode; }; -const Context = createContext(undefined); +type ApiContextType = VersionedValue<{ 1: ApiHolder }> | undefined; +const ApiContext = getOrCreateGlobalSingleton('api-context', () => + createContext(undefined), +); export const ApiProvider = ({ apis, children, }: PropsWithChildren) => { - const parentHolder = useContext(Context); - const holder = parentHolder ? new ApiAggregator(apis, parentHolder) : apis; + const parentHolder = useContext(ApiContext)?.atVersion(1); + const versionedValue = useMemo( + () => + createVersionedValueMap({ + 1: parentHolder ? new ApiAggregator(apis, parentHolder) : apis, + }), + [parentHolder, apis], + ); - return ; + return ; }; ApiProvider.propTypes = { @@ -53,12 +72,19 @@ ApiProvider.propTypes = { }; export function useApiHolder(): ApiHolder { - const apiHolder = useContext(Context); + const versionedHolder = useContext( + getGlobalSingleton>('api-context'), + ); - if (!apiHolder) { + if (!versionedHolder) { throw new Error(missingHolderMessage); } + const apiHolder = versionedHolder.atVersion(1); + if (!apiHolder) { + throw new Error('ApiContext v1 not available'); + } + return apiHolder; } @@ -77,11 +103,7 @@ export function withApis(apis: TypesToApiRefs) { WrappedComponent: React.ComponentType

, ) { const Hoc = (props: PropsWithChildren>) => { - const apiHolder = useContext(Context); - - if (!apiHolder) { - throw new Error(missingHolderMessage); - } + const apiHolder = useApiHolder(); const impls = {} as T; diff --git a/packages/core-api/src/apis/system/ApiRef.ts b/packages/core-api/src/apis/system/ApiRef.ts index 0e1eb177c8..e61036c61c 100644 --- a/packages/core-api/src/apis/system/ApiRef.ts +++ b/packages/core-api/src/apis/system/ApiRef.ts @@ -18,7 +18,7 @@ import type { ApiRef } from './types'; export type ApiRefConfig = { id: string; - description: string; + description?: string; }; class ApiRefImpl implements ApiRef { @@ -38,7 +38,7 @@ class ApiRefImpl implements ApiRef { return this.config.id; } - get description(): string { + get description() { return this.config.description; } diff --git a/packages/core-api/src/apis/system/ApiRegistry.test.ts b/packages/core-api/src/apis/system/ApiRegistry.test.ts index a859d51004..93dd6dc085 100644 --- a/packages/core-api/src/apis/system/ApiRegistry.test.ts +++ b/packages/core-api/src/apis/system/ApiRegistry.test.ts @@ -18,8 +18,9 @@ import { ApiRegistry } from './ApiRegistry'; import { createApiRef } from './ApiRef'; describe('ApiRegistry', () => { - const x1Ref = createApiRef({ id: 'x', description: '' }); - const x2Ref = createApiRef({ id: 'x', description: '' }); + const x1Ref = createApiRef({ id: 'x1', description: '' }); + const x1DuplicateRef = createApiRef({ id: 'x1', description: '' }); + const x2Ref = createApiRef({ id: 'x2', description: '' }); it('should be created', () => { const registry = ApiRegistry.from([]); @@ -32,12 +33,14 @@ describe('ApiRegistry', () => { [x2Ref, 'y'], ]); expect(registry.get(x1Ref)).toBe(3); + expect(registry.get(x1DuplicateRef)).toBe(3); expect(registry.get(x2Ref)).toBe('y'); }); it('should be built', () => { const registry = ApiRegistry.builder().build(); expect(registry.get(x1Ref)).toBe(undefined); + expect(registry.get(x1DuplicateRef)).toBe(undefined); }); it('should be built with APIs', () => { @@ -47,6 +50,7 @@ describe('ApiRegistry', () => { const registry = builder.build(); expect(registry.get(x1Ref)).toBe(3); + expect(registry.get(x1DuplicateRef)).toBe(3); expect(registry.get(x2Ref)).toBe('y'); }); @@ -55,6 +59,7 @@ describe('ApiRegistry', () => { const reg2 = reg1.with(x2Ref, 'y'); const reg3 = reg2.with(x2Ref, 'z'); const reg4 = reg3.with(x1Ref, 2); + const reg5 = reg3.with(x1DuplicateRef, 4); expect(reg1.get(x1Ref)).toBe(3); expect(reg1.get(x2Ref)).toBe(undefined); @@ -64,5 +69,7 @@ describe('ApiRegistry', () => { expect(reg3.get(x2Ref)).toBe('z'); expect(reg4.get(x1Ref)).toBe(2); expect(reg4.get(x2Ref)).toBe('z'); + expect(reg5.get(x1Ref)).toBe(4); + expect(reg5.get(x2Ref)).toBe('z'); }); }); diff --git a/packages/core-api/src/apis/system/ApiRegistry.ts b/packages/core-api/src/apis/system/ApiRegistry.ts index b105633c51..01101b2b62 100644 --- a/packages/core-api/src/apis/system/ApiRegistry.ts +++ b/packages/core-api/src/apis/system/ApiRegistry.ts @@ -19,10 +19,10 @@ import { ApiRef, ApiHolder } from './types'; type ApiImpl = readonly [ApiRef, T]; class ApiRegistryBuilder { - private apis: ApiImpl[] = []; + private apis: [string, unknown][] = []; add(api: ApiRef, impl: I): I { - this.apis.push([api, impl]); + this.apis.push([api.id, impl]); return impl; } @@ -38,7 +38,7 @@ export class ApiRegistry implements ApiHolder { } static from(apis: ApiImpl[]) { - return new ApiRegistry(new Map(apis)); + return new ApiRegistry(new Map(apis.map(([api, impl]) => [api.id, impl]))); } /** @@ -48,10 +48,10 @@ export class ApiRegistry implements ApiHolder { * @param impl Implementation of the API to add */ static with(api: ApiRef, impl: T): ApiRegistry { - return new ApiRegistry(new Map([[api, impl]])); + return new ApiRegistry(new Map([[api.id, impl]])); } - constructor(private readonly apis: Map, unknown>) {} + constructor(private readonly apis: Map) {} /** * Returns a new ApiRegistry with the provided API added to the existing ones. @@ -60,10 +60,10 @@ export class ApiRegistry implements ApiHolder { * @param impl Implementation of the API to add */ with(api: ApiRef, impl: T): ApiRegistry { - return new ApiRegistry(new Map([...this.apis, [api, impl]])); + return new ApiRegistry(new Map([...this.apis, [api.id, impl]])); } get(api: ApiRef): T | undefined { - return this.apis.get(api) as T | undefined; + return this.apis.get(api.id) as T | undefined; } } diff --git a/packages/core-api/src/apis/system/ApiResolver.test.ts b/packages/core-api/src/apis/system/ApiResolver.test.ts index 3cd40f289e..7a46d2db3b 100644 --- a/packages/core-api/src/apis/system/ApiResolver.test.ts +++ b/packages/core-api/src/apis/system/ApiResolver.test.ts @@ -19,8 +19,14 @@ import { createApiRef } from './ApiRef'; import { ApiFactoryRegistry } from './ApiFactoryRegistry'; const aRef = createApiRef({ id: 'a', description: '' }); +const otherARef = createApiRef({ id: 'a', description: 'other' }); const bRef = createApiRef({ id: 'b', description: '' }); +const otherBRef = createApiRef({ id: 'b', description: 'other' }); const cRef = createApiRef<{ x: string }>({ id: 'c', description: '' }); +const otherCRef = createApiRef<{ x: string }>({ + id: 'c', + description: 'other', +}); function createRegistry() { const registry = new ApiFactoryRegistry(); @@ -36,28 +42,18 @@ function createRegistry() { }); registry.register('default', { api: cRef, - deps: { b: bRef }, + deps: { b: otherBRef }, factory: ({ b }) => ({ x: 'x', b }), }); return registry; } -function createLongCyclicRegistry() { +function createSelfCyclicRegistry() { const registry = new ApiFactoryRegistry(); registry.register('default', { api: aRef, - deps: { b: bRef }, - factory: () => 1, - }); - registry.register('default', { - api: bRef, - deps: { c: cRef }, - factory: () => 'b', - }); - registry.register('default', { - api: cRef, deps: { a: aRef }, - factory: () => ({ x: 'x' }), + factory: () => 1, }); return registry; } @@ -66,7 +62,37 @@ function createShortCyclicRegistry() { const registry = new ApiFactoryRegistry(); registry.register('default', { api: aRef, + deps: { b: bRef }, + factory: () => 1, + }); + registry.register('default', { + api: bRef, deps: { a: aRef }, + factory: () => 'x', + }); + return registry; +} + +function createShortCyclicRegistryWithOther() { + const registry = new ApiFactoryRegistry(); + registry.register('default', { + api: aRef, + deps: { b: bRef }, + factory: () => 1, + }); + registry.register('default', { + api: otherBRef, + deps: { a: otherARef }, + factory: () => 'x', + }); + return registry; +} + +function createLongCyclicRegistry() { + const registry = new ApiFactoryRegistry(); + registry.register('default', { + api: aRef, + deps: { b: otherBRef }, factory: () => 1, }); registry.register('default', { @@ -76,7 +102,7 @@ function createShortCyclicRegistry() { }); registry.register('default', { api: cRef, - deps: { b: bRef }, + deps: { a: aRef }, factory: () => ({ x: 'x' }), }); return registry; @@ -87,15 +113,51 @@ describe('ApiResolver', () => { const resolver = new ApiResolver(new ApiFactoryRegistry()); expect(resolver.get(aRef)).toBe(undefined); expect(resolver.get(bRef)).toBe(undefined); + expect(resolver.get(otherBRef)).toBe(undefined); expect(resolver.get(cRef)).toBe(undefined); }); it('should instantiate APIs', () => { const resolver = new ApiResolver(createRegistry()); expect(resolver.get(aRef)).toBe(1); + expect(resolver.get(otherARef)).toBe(1); expect(resolver.get(bRef)).toBe('b'); + expect(resolver.get(otherBRef)).toBe('b'); expect(resolver.get(cRef)).toEqual({ x: 'x', b: 'b' }); - expect(resolver.get(cRef)).toBe(resolver.get(cRef)); + expect(resolver.get(cRef)).toBe(resolver.get(otherCRef)); + }); + + it('should detect self dependency cycles', () => { + const resolver = new ApiResolver(createSelfCyclicRegistry()); + expect(() => resolver.get(aRef)).toThrow( + 'Circular dependency of api factory for apiRef{a}', + ); + }); + + it('should detect short dependency cycles', () => { + const resolver = new ApiResolver(createShortCyclicRegistry()); + expect(() => resolver.get(aRef)).toThrow( + 'Circular dependency of api factory for apiRef{a}', + ); + expect(() => resolver.get(bRef)).toThrow( + 'Circular dependency of api factory for apiRef{b}', + ); + }); + + it('should detect short dependency cycles with other refs', () => { + const resolver = new ApiResolver(createShortCyclicRegistryWithOther()); + expect(() => resolver.get(aRef)).toThrow( + 'Circular dependency of api factory for apiRef{a}', + ); + expect(() => resolver.get(bRef)).toThrow( + 'Circular dependency of api factory for apiRef{b}', + ); + expect(() => resolver.get(otherARef)).toThrow( + 'Circular dependency of api factory for apiRef{a}', + ); + expect(() => resolver.get(otherBRef)).toThrow( + 'Circular dependency of api factory for apiRef{b}', + ); }); it('should detect long dependency cycles', () => { @@ -110,17 +172,7 @@ describe('ApiResolver', () => { expect(() => resolver.get(bRef)).toThrow( 'Circular dependency of api factory for apiRef{b}', ); - expect(() => resolver.get(cRef)).toThrow( - 'Circular dependency of api factory for apiRef{c}', - ); - }); - - it('should detect short dependency cycles', () => { - const resolver = new ApiResolver(createShortCyclicRegistry()); - expect(() => resolver.get(aRef)).toThrow( - 'Circular dependency of api factory for apiRef{a}', - ); - expect(() => resolver.get(bRef)).toThrow( + expect(() => resolver.get(otherBRef)).toThrow( 'Circular dependency of api factory for apiRef{b}', ); expect(() => resolver.get(cRef)).toThrow( @@ -130,22 +182,54 @@ describe('ApiResolver', () => { it('should validate a factory holder', () => { expect(() => { - ApiResolver.validateFactories(createRegistry(), [aRef, bRef, cRef]); + ApiResolver.validateFactories(createRegistry(), [ + aRef, + bRef, + otherBRef, + cRef, + ]); }).not.toThrow(); }); + it('should find self cycles with validation', () => { + const self = createSelfCyclicRegistry(); + expect(() => ApiResolver.validateFactories(self, [aRef])).toThrow( + 'Circular dependency of api factory for apiRef{a}', + ); + expect(() => ApiResolver.validateFactories(self, [otherARef])).toThrow( + 'Circular dependency of api factory for apiRef{a}', + ); + }); + it('should find dependency cycles with validation', () => { const short = createShortCyclicRegistry(); - expect(() => - ApiResolver.validateFactories(short, short.getAllApis()), - ).toThrow('Circular dependency of api factory for apiRef{a}'); + expect(() => ApiResolver.validateFactories(short, [aRef])).toThrow( + 'Circular dependency of api factory for apiRef{a}', + ); + expect(() => ApiResolver.validateFactories(short, [otherARef])).toThrow( + 'Circular dependency of api factory for apiRef{a}', + ); expect(() => ApiResolver.validateFactories(short, [bRef])).toThrow( 'Circular dependency of api factory for apiRef{b}', ); - expect(() => ApiResolver.validateFactories(short, [cRef])).toThrow( - 'Circular dependency of api factory for apiRef{c}', + expect(() => ApiResolver.validateFactories(short, [otherBRef])).toThrow( + 'Circular dependency of api factory for apiRef{b}', ); + const shortOther = createShortCyclicRegistryWithOther(); + expect(() => ApiResolver.validateFactories(shortOther, [aRef])).toThrow( + 'Circular dependency of api factory for apiRef{a}', + ); + expect(() => + ApiResolver.validateFactories(shortOther, [otherARef]), + ).toThrow('Circular dependency of api factory for apiRef{a}'); + expect(() => ApiResolver.validateFactories(shortOther, [bRef])).toThrow( + 'Circular dependency of api factory for apiRef{b}', + ); + expect(() => + ApiResolver.validateFactories(shortOther, [otherBRef]), + ).toThrow('Circular dependency of api factory for apiRef{b}'); + const long = createLongCyclicRegistry(); expect(() => ApiResolver.validateFactories(long, long.getAllApis()), @@ -153,6 +237,9 @@ describe('ApiResolver', () => { expect(() => ApiResolver.validateFactories(long, [bRef])).toThrow( 'Circular dependency of api factory for apiRef{b}', ); + expect(() => ApiResolver.validateFactories(long, [otherBRef])).toThrow( + 'Circular dependency of api factory for apiRef{b}', + ); expect(() => ApiResolver.validateFactories(long, [cRef])).toThrow( 'Circular dependency of api factory for apiRef{c}', ); @@ -173,5 +260,7 @@ describe('ApiResolver', () => { expect(factory).toHaveBeenCalledTimes(1); expect(resolver.get(aRef)).toBe(2); expect(factory).toHaveBeenCalledTimes(1); + expect(resolver.get(otherARef)).toBe(2); + expect(factory).toHaveBeenCalledTimes(1); }); }); diff --git a/packages/core-api/src/apis/system/ApiResolver.ts b/packages/core-api/src/apis/system/ApiResolver.ts index 0dfeaeedd0..9738e09622 100644 --- a/packages/core-api/src/apis/system/ApiResolver.ts +++ b/packages/core-api/src/apis/system/ApiResolver.ts @@ -23,8 +23,6 @@ import { } from './types'; export class ApiResolver implements ApiHolder { - private readonly apis = new Map(); - /** * Validate factories by making sure that each of the apis can be created * without hitting any circular dependencies. @@ -45,7 +43,7 @@ export class ApiResolver implements ApiHolder { } for (const dep of Object.values(factory.deps)) { - if (dep === api) { + if (dep.id === api.id) { throw new Error(`Circular dependency of api factory for ${api}`); } if (!allDeps.has(dep)) { @@ -57,6 +55,8 @@ export class ApiResolver implements ApiHolder { } } + private readonly apis = new Map(); + constructor(private readonly factories: ApiFactoryHolder) {} get(ref: ApiRef): T | undefined { @@ -64,7 +64,7 @@ export class ApiResolver implements ApiHolder { } private load(ref: ApiRef, loading: AnyApiRef[] = []): T | undefined { - const impl = this.apis.get(ref); + const impl = this.apis.get(ref.id); if (impl) { return impl as T; } @@ -80,7 +80,7 @@ export class ApiResolver implements ApiHolder { const deps = this.loadDeps(ref, factory.deps, [...loading, factory.api]); const api = factory.factory(deps); - this.apis.set(ref, api); + this.apis.set(ref.id, api); return api as T; } diff --git a/packages/core-api/src/apis/system/types.ts b/packages/core-api/src/apis/system/types.ts index 6234a55c71..a4bc95a3c3 100644 --- a/packages/core-api/src/apis/system/types.ts +++ b/packages/core-api/src/apis/system/types.ts @@ -16,7 +16,7 @@ export type ApiRef = { id: string; - description: string; + description?: string; T: T; }; diff --git a/packages/core-api/src/app/AppContext.test.tsx b/packages/core-api/src/app/AppContext.test.tsx new file mode 100644 index 0000000000..55d98c18c0 --- /dev/null +++ b/packages/core-api/src/app/AppContext.test.tsx @@ -0,0 +1,79 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import React, { useContext, Context } from 'react'; +import { renderHook } from '@testing-library/react-hooks'; +import { VersionedValue } from '../lib/versionedValues'; +import { getGlobalSingleton } from '../lib/globalObject'; +import { AppContext as AppContextV1 } from './types'; +import { AppContextProvider } from './AppContext'; + +describe('v1 consumer', () => { + const AppContext = getGlobalSingleton< + Context> + >('app-context'); + + function useMockAppV1(): AppContextV1 { + const impl = useContext(AppContext)?.atVersion(1); + if (!impl) { + throw new Error('no impl'); + } + return impl; + } + + it('should provide an app context', () => { + const mockContext: AppContextV1 = { + getComponents: jest.fn(), + getSystemIcon: jest.fn(), + getPlugins: jest.fn(), + getProvider: jest.fn(), + getRouter: jest.fn(), + getRoutes: jest.fn(), + }; + + const renderedHook = renderHook(() => useMockAppV1(), { + wrapper: ({ children }) => ( + + ), + }); + const result = renderedHook.result.current; + + expect(mockContext.getComponents).toHaveBeenCalledTimes(0); + result.getComponents(); + expect(mockContext.getComponents).toHaveBeenCalledTimes(1); + + expect(mockContext.getSystemIcon).toHaveBeenCalledTimes(0); + result.getSystemIcon('icon'); + expect(mockContext.getSystemIcon).toHaveBeenCalledTimes(1); + expect(mockContext.getSystemIcon).toHaveBeenCalledWith('icon'); + + expect(mockContext.getPlugins).toHaveBeenCalledTimes(0); + result.getPlugins(); + expect(mockContext.getPlugins).toHaveBeenCalledTimes(1); + + expect(mockContext.getProvider).toHaveBeenCalledTimes(0); + result.getProvider(); + expect(mockContext.getProvider).toHaveBeenCalledTimes(1); + + expect(mockContext.getRouter).toHaveBeenCalledTimes(0); + result.getRouter(); + expect(mockContext.getRouter).toHaveBeenCalledTimes(1); + + expect(mockContext.getRoutes).toHaveBeenCalledTimes(0); + result.getRoutes(); + expect(mockContext.getRoutes).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/core-api/src/app/AppContext.tsx b/packages/core-api/src/app/AppContext.tsx index c7c405a959..c143aa86bc 100644 --- a/packages/core-api/src/app/AppContext.tsx +++ b/packages/core-api/src/app/AppContext.tsx @@ -14,26 +14,54 @@ * limitations under the License. */ -import React, { createContext, PropsWithChildren, useContext } from 'react'; -import { AppContext } from './types'; +import React, { + createContext, + PropsWithChildren, + useContext, + Context, + useMemo, +} from 'react'; +import { + VersionedValue, + createVersionedValueMap, +} from '../lib/versionedValues'; +import { + getGlobalSingleton, + getOrCreateGlobalSingleton, +} from '../lib/globalObject'; +import { AppContext as AppContextV1 } from './types'; -const Context = createContext(undefined); +type AppContextType = VersionedValue<{ 1: AppContextV1 }> | undefined; +const AppContext = getOrCreateGlobalSingleton('app-context', () => + createContext(undefined), +); type Props = { - appContext: AppContext; + appContext: AppContextV1; }; export const AppContextProvider = ({ appContext, children, -}: PropsWithChildren) => ( - -); +}: PropsWithChildren) => { + const versionedValue = useMemo( + () => createVersionedValueMap({ 1: appContext }), + [appContext], + ); -export const useApp = (): AppContext => { - const appContext = useContext(Context); - if (!appContext) { + return ; +}; + +export const useApp = (): AppContextV1 => { + const versionedContext = useContext( + getGlobalSingleton>('app-context'), + ); + if (!versionedContext) { throw new Error('No app context available'); } + const appContext = versionedContext.atVersion(1); + if (!appContext) { + throw new Error('AppContext v1 not available'); + } return appContext; }; diff --git a/packages/core-api/src/extensions/componentData.tsx b/packages/core-api/src/extensions/componentData.tsx index 8b46d34705..fc8594039c 100644 --- a/packages/core-api/src/extensions/componentData.tsx +++ b/packages/core-api/src/extensions/componentData.tsx @@ -15,7 +15,7 @@ */ import { ComponentType, ReactNode } from 'react'; -import { getGlobalSingleton } from '../lib/globalObject'; +import { getOrCreateGlobalSingleton } from '../lib/globalObject'; // TODO(Rugvip): Access via symbol is deprecated, remove once on 0.3.x const DATA_KEY = Symbol('backstage-component-data'); @@ -33,7 +33,7 @@ type MaybeComponentNode = ReactNode & { }; // The store is bridged across versions using the global object -const store = getGlobalSingleton( +const store = getOrCreateGlobalSingleton( 'component-data-store', () => new WeakMap, DataContainer>(), ); diff --git a/packages/core-api/src/lib/globalObject.test.ts b/packages/core-api/src/lib/globalObject.test.ts index f539483a83..e72f027b46 100644 --- a/packages/core-api/src/lib/globalObject.test.ts +++ b/packages/core-api/src/lib/globalObject.test.ts @@ -14,7 +14,11 @@ * limitations under the License. */ -import { getGlobalSingleton } from './globalObject'; +import { + getGlobalSingleton, + getOrCreateGlobalSingleton, + setGlobalSingleton, +} from './globalObject'; const anyGlobal = global as any; @@ -23,20 +27,64 @@ describe('getGlobalSingleton', () => { delete anyGlobal['__@backstage/my-thing__']; }); + it('should return an existing value', () => { + const myThing = {}; + const myOtherThing = {}; + + anyGlobal['__@backstage/my-thing__'] = myThing; + expect(getGlobalSingleton('my-thing')).toBe(myThing); + expect(getGlobalSingleton('my-thing')).toBe(myThing); + anyGlobal['__@backstage/my-thing__'] = myOtherThing; + expect(getGlobalSingleton('my-thing')).toBe(myOtherThing); + }); + + it('should throw if the value is not set', () => { + expect(() => getGlobalSingleton('my-thing')).toThrow( + 'Global my-thing is not set', + ); + }); +}); + +describe('getOrCreateGlobalSingleton', () => { + beforeEach(() => { + delete anyGlobal['__@backstage/my-thing__']; + }); + it('should return an existing value', () => { const myThing = {}; anyGlobal['__@backstage/my-thing__'] = myThing; - expect(getGlobalSingleton('my-thing', () => ({}))).toBe(myThing); - expect(getGlobalSingleton('my-thing', () => ({}))).toBe(myThing); + expect(getOrCreateGlobalSingleton('my-thing', () => ({}))).toBe(myThing); + expect(getOrCreateGlobalSingleton('my-thing', () => ({}))).toBe(myThing); }); it('should should create a new value', () => { const myNewThing = {}; expect(anyGlobal['__@backstage/my-thing__']).toBe(undefined); - expect(getGlobalSingleton('my-thing', () => myNewThing)).toBe(myNewThing); + expect(getOrCreateGlobalSingleton('my-thing', () => myNewThing)).toBe( + myNewThing, + ); expect(anyGlobal['__@backstage/my-thing__']).toBe(myNewThing); - expect(getGlobalSingleton('my-thing', () => ({}))).toBe(myNewThing); + expect(getOrCreateGlobalSingleton('my-thing', () => ({}))).toBe(myNewThing); + }); +}); + +describe('setGlobalSingleton', () => { + beforeEach(() => { + delete anyGlobal['__@backstage/my-thing__']; + }); + + it('should set a global value', () => { + setGlobalSingleton('my-thing', 'global value'); + + expect(anyGlobal['__@backstage/my-thing__']).toBe('global value'); + }); + + it('should throw if global value is set', () => { + anyGlobal['__@backstage/my-thing__'] = 'already defined'; + expect(() => setGlobalSingleton('my-thing', () => 'global value')).toThrow( + 'Global my-thing is already se', + ); }); }); diff --git a/packages/core-api/src/lib/globalObject.ts b/packages/core-api/src/lib/globalObject.ts index c7f9f98882..87be58499d 100644 --- a/packages/core-api/src/lib/globalObject.ts +++ b/packages/core-api/src/lib/globalObject.ts @@ -26,10 +26,41 @@ function getGlobalObject() { return Function('return this')(); } -export const globalObject = getGlobalObject(); +const globalObject = getGlobalObject(); -export function getGlobalSingleton(id: string, supplier: () => T): T { - const key = `__@backstage/${id}__`; +const makeKey = (id: string) => `__@backstage/${id}__`; + +/** + * Used to provide a global singleton value, failing if it is already set. + */ +export function setGlobalSingleton(id: string, value: unknown): void { + const key = makeKey(id); + if (key in globalObject) { + throw new Error(`Global ${id} is already set`); // TODO some sort of special build err + } + globalObject[key] = value; +} + +/** + * Used to access a global singleton value, failing if it is not already set. + */ +export function getGlobalSingleton(id: string): T { + const key = makeKey(id); + if (!(key in globalObject)) { + throw new Error(`Global ${id} is not set`); // TODO some sort of special build err + } + + return globalObject[key]; +} + +/** + * Serializes access to a global singleton value, with the first caller creating the value. + */ +export function getOrCreateGlobalSingleton( + id: string, + supplier: () => T, +): T { + const key = makeKey(id); let value = globalObject[key]; if (value) { diff --git a/packages/core-api/src/lib/versionedValues.test.ts b/packages/core-api/src/lib/versionedValues.test.ts new file mode 100644 index 0000000000..6ba7db4970 --- /dev/null +++ b/packages/core-api/src/lib/versionedValues.test.ts @@ -0,0 +1,40 @@ +/* + * Copyright 2021 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { createVersionedValueMap } from './versionedValues'; + +describe('createVersionedValueMap', () => { + it('should be empty', () => { + const map = createVersionedValueMap({}); + + // @ts-expect-error + expect(map.atVersion(1 as any)).toBe(undefined); + }); + + it('should access values by version', () => { + const map = createVersionedValueMap({ 1: 'v1', 2: 'v2' }); + + expect(map.atVersion(1)).toBe('v1'); + expect(map.atVersion(2)).toBe('v2'); + + // @ts-expect-error + expect(map.atVersion(0)).toBe(undefined); + // @ts-expect-error + expect(map.atVersion(NaN)).toBe(undefined); + // @ts-expect-error + expect(map.atVersion(Infinity)).toBe(undefined); + }); +}); diff --git a/packages/core-api/src/lib/versionedValues.ts b/packages/core-api/src/lib/versionedValues.ts new file mode 100644 index 0000000000..88e4e90084 --- /dev/null +++ b/packages/core-api/src/lib/versionedValues.ts @@ -0,0 +1,40 @@ +/* + * Copyright 2021 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * The versioned value interface is a container for a set of values that + * can be looked up by version. It is intended to be used as a container + * for values that can be versioned independently of package versions. + */ +export type VersionedValue = { + atVersion( + version: Version, + ): Versions[Version] | undefined; +}; + +/** + * Creates a container for a map of versioned values that implements VersionedValue. + */ +export function createVersionedValueMap< + Versions extends { [version: number]: any } +>(versions: Versions): VersionedValue { + Object.freeze(versions); + return { + atVersion(version) { + return versions[version]; + }, + }; +} diff --git a/packages/core-api/src/routing/hooks.test.tsx b/packages/core-api/src/routing/hooks.test.tsx index 9a65286fd6..d16f9808ab 100644 --- a/packages/core-api/src/routing/hooks.test.tsx +++ b/packages/core-api/src/routing/hooks.test.tsx @@ -14,9 +14,17 @@ * limitations under the License. */ -import { render } from '@testing-library/react'; -import React, { PropsWithChildren, ReactElement } from 'react'; +import React, { + PropsWithChildren, + ReactElement, + useContext, + Context, +} from 'react'; import { MemoryRouter, Routes } from 'react-router-dom'; +import { render } from '@testing-library/react'; +import { renderHook } from '@testing-library/react-hooks'; +import { VersionedValue } from '../lib/versionedValues'; +import { getGlobalSingleton } from '../lib/globalObject'; import { createRoutableExtension } from '../extensions'; import { childDiscoverer, @@ -32,6 +40,7 @@ import { import { validateRoutes } from './validation'; import { useRouteRef, RoutingProvider } from './hooks'; import { createRouteRef, RouteRefConfig } from './RouteRef'; +import { RouteResolver } from './RouteResolver'; import { createExternalRouteRef } from './ExternalRouteRef'; import { AnyRouteRef, RouteFunc, RouteRef, ExternalRouteRef } from './types'; @@ -309,3 +318,55 @@ describe('discovery', () => { ); }); }); + +describe('v1 consumer', () => { + const RoutingContext = getGlobalSingleton< + Context> + >('routing-context'); + + function useMockRouteRefV1( + routeRef: AnyRouteRef, + location: string, + ): RouteFunc | undefined { + const resolver = useContext(RoutingContext)?.atVersion(1); + if (!resolver) { + throw new Error('no impl'); + } + return resolver.resolve(routeRef, location); + } + + it('should resolve routes', () => { + const routeRef1 = createRouteRef({ id: 'ref1' }); + const routeRef2 = createRouteRef({ id: 'ref2' }); + const routeRef3 = createRouteRef({ id: 'ref3', params: ['x'] }); + + const renderedHook = renderHook( + ({ routeRef }) => useMockRouteRefV1(routeRef, '/'), + { + initialProps: { + routeRef: routeRef1 as AnyRouteRef, + }, + wrapper: ({ children }) => ( + , string>([ + [routeRef2, '/foo'], + [routeRef3, '/bar/:x'], + ]) + } + routeParents={new Map()} + routeObjects={[]} + routeBindings={new Map()} + children={children} + /> + ), + }, + ); + + expect(renderedHook.result.current).toBe(undefined); + renderedHook.rerender({ routeRef: routeRef2 }); + expect(renderedHook.result.current?.()).toBe('/foo'); + renderedHook.rerender({ routeRef: routeRef3 }); + expect(renderedHook.result.current?.({ x: 'my-x' })).toBe('/bar/my-x'); + }); +}); diff --git a/packages/core-api/src/routing/hooks.tsx b/packages/core-api/src/routing/hooks.tsx index 2e65efaba6..4c4a7a6a05 100644 --- a/packages/core-api/src/routing/hooks.tsx +++ b/packages/core-api/src/routing/hooks.tsx @@ -14,7 +14,13 @@ * limitations under the License. */ -import React, { createContext, ReactNode, useContext, useMemo } from 'react'; +import React, { + createContext, + ReactNode, + useContext, + useMemo, + Context, +} from 'react'; import { useLocation } from 'react-router-dom'; import { BackstageRouteObject, @@ -25,8 +31,19 @@ import { RouteFunc, } from './types'; import { RouteResolver } from './RouteResolver'; +import { + VersionedValue, + createVersionedValueMap, +} from '../lib/versionedValues'; +import { + getGlobalSingleton, + getOrCreateGlobalSingleton, +} from '../lib/globalObject'; -const RoutingContext = createContext(undefined); +type RoutingContextType = VersionedValue<{ 1: RouteResolver }> | undefined; +const RoutingContext = getOrCreateGlobalSingleton('routing-context', () => + createContext(undefined), +); export function useRouteRef( routeRef: ExternalRouteRef, @@ -41,14 +58,20 @@ export function useRouteRef( | ExternalRouteRef, ): RouteFunc | undefined { const sourceLocation = useLocation(); - const resolver = useContext(RoutingContext); + const versionedContext = useContext( + getGlobalSingleton>('routing-context'), + ); + const resolver = versionedContext?.atVersion(1); const routeFunc = useMemo( () => resolver && resolver.resolve(routeRef, sourceLocation), [resolver, routeRef, sourceLocation], ); - if (!routeFunc && !resolver) { - throw new Error('No route resolver found in context'); + if (!versionedContext) { + throw new Error('useRouteRef used outside of routing context'); + } + if (!resolver) { + throw new Error('RoutingContext v1 not available'); } const isOptional = 'optional' in routeRef && routeRef.optional; @@ -80,8 +103,10 @@ export const RoutingProvider = ({ routeObjects, routeBindings, ); + + const versionedValue = createVersionedValueMap({ 1: resolver }); return ( - + {children} ); diff --git a/packages/core-api/src/routing/types.ts b/packages/core-api/src/routing/types.ts index 56b991f3af..1fabd39cc4 100644 --- a/packages/core-api/src/routing/types.ts +++ b/packages/core-api/src/routing/types.ts @@ -15,7 +15,7 @@ */ import { IconComponent } from '../icons'; -import { getGlobalSingleton } from '../lib/globalObject'; +import { getOrCreateGlobalSingleton } from '../lib/globalObject'; export type AnyParams = { [param in string]: string } | undefined; export type ParamKeys = keyof Params extends never @@ -34,7 +34,7 @@ export type RouteFunc = ( ...[params]: Params extends undefined ? readonly [] : readonly [Params] ) => string; -export const routeRefType: unique symbol = getGlobalSingleton( +export const routeRefType: unique symbol = getOrCreateGlobalSingleton( 'route-ref-type', () => Symbol('route-ref-type'), );