From aab7bc21cd38662fe499234b6088e377a3c5109b Mon Sep 17 00:00:00 2001 From: benjdlambert Date: Mon, 4 Aug 2025 16:23:35 +0200 Subject: [PATCH] chore: enforce loader format for component refs Signed-off-by: benjdlambert --- .../src/wiring/InternalComponentRef.ts | 19 +++---- .../ComponentImplementationBlueprint.test.tsx | 5 +- .../ComponentImplementationBlueprint.ts | 22 +++----- .../components/createComponentRef.test.tsx | 34 +++---------- .../src/components/createComponentRef.tsx | 51 ++----------------- .../components/makeComponentFromRef.test.tsx | 20 +++----- .../src/components/makeComponentFromRef.tsx | 35 ++++++------- 7 files changed, 49 insertions(+), 137 deletions(-) diff --git a/packages/frontend-internal/src/wiring/InternalComponentRef.ts b/packages/frontend-internal/src/wiring/InternalComponentRef.ts index 93cdf38b1a..00d4a253d5 100644 --- a/packages/frontend-internal/src/wiring/InternalComponentRef.ts +++ b/packages/frontend-internal/src/wiring/InternalComponentRef.ts @@ -21,19 +21,12 @@ export const OpaqueComponentRef = OpaqueType.create<{ public: ComponentRef; versions: { readonly version: 'v1'; - readonly options: - | { - mode: 'sync'; - transformProps?: (props: object) => object; - defaultComponent?: (props: object) => JSX.Element | null; - } - | { - mode: 'async'; - transformProps?: (props: object) => object; - defaultComponent?: () => Promise< - (props: object) => JSX.Element | null - >; - }; + readonly options: { + transformProps?: (props: object) => object; + loader?: + | (() => (props: object) => JSX.Element | null) + | (() => Promise<(props: object) => JSX.Element | null>); + }; }; }>({ versions: ['v1'], diff --git a/packages/frontend-plugin-api/src/blueprints/ComponentImplementationBlueprint.test.tsx b/packages/frontend-plugin-api/src/blueprints/ComponentImplementationBlueprint.test.tsx index b0095e4350..0943659fed 100644 --- a/packages/frontend-plugin-api/src/blueprints/ComponentImplementationBlueprint.test.tsx +++ b/packages/frontend-plugin-api/src/blueprints/ComponentImplementationBlueprint.test.tsx @@ -20,15 +20,14 @@ describe('ComponentImplementationBlueprint', () => { it('should allow defining a component override for sync component ref', () => { const componentRef = createComponentRef({ id: 'test.component', - mode: 'sync', - defaultComponent: (props: { hello: string }) =>
{props.hello}
, + loader: () => (props: { hello: string }) =>
{props.hello}
, }); const extension = ComponentImplementationBlueprint.make({ params: define => define({ ref: componentRef, - component: props => { + loader: () => props => { // @ts-expect-error const t: number = props.hello; diff --git a/packages/frontend-plugin-api/src/blueprints/ComponentImplementationBlueprint.ts b/packages/frontend-plugin-api/src/blueprints/ComponentImplementationBlueprint.ts index d903d63f5e..0386c90f5e 100644 --- a/packages/frontend-plugin-api/src/blueprints/ComponentImplementationBlueprint.ts +++ b/packages/frontend-plugin-api/src/blueprints/ComponentImplementationBlueprint.ts @@ -22,10 +22,9 @@ import { export const componentDataRef = createExtensionDataRef<{ ref: ComponentRef; - component: - | ((props: {}) => JSX.Element | null) + loader: + | (() => (props: {}) => JSX.Element | null) | (() => Promise<(props: {}) => JSX.Element | null>); - type: 'sync' | 'async'; }>().with({ id: 'core.component.component' }); export const ComponentImplementationBlueprint = createExtensionBlueprint({ @@ -37,16 +36,10 @@ export const ComponentImplementationBlueprint = createExtensionBlueprint({ }, defineParams>(params: { ref: Ref; - component: Ref extends ComponentRef< - infer IInnerComponentProps, - any, - infer IMode - > - ? IMode extends 'sync' - ? (props: IInnerComponentProps) => JSX.Element - : IMode extends 'async' - ? () => Promise<(props: IInnerComponentProps) => JSX.Element> - : never + loader: Ref extends ComponentRef + ? + | (() => (props: IInnerComponentProps) => JSX.Element | null) + | (() => Promise<(props: IInnerComponentProps) => JSX.Element | null>) : never; }) { return createExtensionBlueprintParams(params); @@ -54,8 +47,7 @@ export const ComponentImplementationBlueprint = createExtensionBlueprint({ *factory(params) { yield componentDataRef({ ref: params.ref, - component: params.component, - type: params.ref.mode, + loader: params.loader, }); }, }); diff --git a/packages/frontend-plugin-api/src/components/createComponentRef.test.tsx b/packages/frontend-plugin-api/src/components/createComponentRef.test.tsx index 751daa0c3f..b1bcd66c17 100644 --- a/packages/frontend-plugin-api/src/components/createComponentRef.test.tsx +++ b/packages/frontend-plugin-api/src/components/createComponentRef.test.tsx @@ -18,7 +18,7 @@ import { createComponentRef } from './createComponentRef'; describe('createComponentRef', () => { it('can be created and read', () => { - const ref = createComponentRef({ id: 'foo', mode: 'sync' }); + const ref = createComponentRef({ id: 'foo' }); expect(ref.id).toBe('foo'); expect(String(ref)).toBe('ComponentRef{id=foo}'); }); @@ -28,14 +28,15 @@ describe('createComponentRef', () => { createComponentRef<{ foo: string }, { bar: string }>({ id: 'foo', - mode: 'sync', - defaultComponent: ({ foo }) => , + loader: + () => + ({ foo }) => + , }); createComponentRef<{ foo: string }, { bar: string }>({ id: 'foo', - mode: 'async', - defaultComponent: + loader: async () => ({ foo }) => , @@ -43,21 +44,6 @@ describe('createComponentRef', () => { createComponentRef<{ foo: string }, { bar: string }>({ id: 'foo', - mode: 'sync', - // @ts-expect-error - this should be an error as mode is sync - defaultComponent: async ({ foo }) => , - }); - - createComponentRef<{ foo: string }, { bar: string }>({ - id: 'foo', - mode: 'async', - // @ts-expect-error - this should be an error as mode is async - defaultComponent: ({ bar }) => , - }); - - createComponentRef<{ foo: string }, { bar: string }>({ - id: 'foo', - mode: 'sync', }); expect(Test).toBeDefined(); @@ -66,23 +52,15 @@ describe('createComponentRef', () => { it('should allow transformings props', () => { createComponentRef<{ foo: string }, { bar: string }>({ id: 'foo', - mode: 'sync', transformProps: props => ({ foo: props.bar }), }); createComponentRef<{ foo: string }, { bar: string }>({ id: 'foo', - mode: 'sync', // @ts-expect-error - this should be an error as foo is not a string transformProps: props => ({ foo: 1 }), }); - createComponentRef<{ foo: string }, { bar: string }>({ - id: 'foo', - mode: 'sync', - transformProps: props => ({ foo: props.bar }), - }); - expect(true).toBe(true); }); }); diff --git a/packages/frontend-plugin-api/src/components/createComponentRef.tsx b/packages/frontend-plugin-api/src/components/createComponentRef.tsx index f0a1e36371..7c4f027fa6 100644 --- a/packages/frontend-plugin-api/src/components/createComponentRef.tsx +++ b/packages/frontend-plugin-api/src/components/createComponentRef.tsx @@ -20,12 +20,10 @@ import { OpaqueComponentRef } from '@internal/frontend'; export type ComponentRef< TInnerComponentProps extends {} = {}, TExternalComponentProps extends {} = TInnerComponentProps, - TMode extends 'sync' | 'async' = 'sync' | 'async', > = { id: string; TProps: TInnerComponentProps; TExternalProps: TExternalComponentProps; - TMode: TMode; $$type: '@backstage/ComponentRef'; }; @@ -34,66 +32,27 @@ export type ComponentRefOptions< TExternalComponentProps extends {} = TInnerComponentProps, > = { id: string; - componentAsync: TMode extends 'async' - ? () => Promise<(props: TInnerComponentProps) => JSX.Element | null> - : TMode extends 'sync' - ? (props: TInnerComponentProps) => JSX.Element | null - : never; + loader?: + | (() => (props: TInnerComponentProps) => JSX.Element | null) + | (() => Promise<(props: TInnerComponentProps) => JSX.Element | null>); transformProps?: (props: TExternalComponentProps) => TInnerComponentProps; }; -/** - * Creates a new component ref that is synchronous. - * @public - */ export function createComponentRef< TInnerComponentProps extends {}, TExternalComponentProps extends {} = TInnerComponentProps, >( - options: ComponentRefOptions< - TInnerComponentProps, - TExternalComponentProps, - 'sync' - >, -): ComponentRef; - -/** - * Creates a new component ref that is asynchronous. - * @public - */ -export function createComponentRef< - TInnerComponentProps extends {}, - TExternalComponentProps extends {} = TInnerComponentProps, ->( - options: ComponentRefOptions< - TInnerComponentProps, - TExternalComponentProps, - 'async' - >, -): ComponentRef; - -export function createComponentRef< - TInnerComponentProps extends {}, - TExternalComponentProps extends {} = TInnerComponentProps, - TMode extends 'sync' | 'async' = 'sync' | 'async', ->( - options: ComponentRefOptions< - TInnerComponentProps, - TExternalComponentProps, - TMode - >, + options: ComponentRefOptions, ): ComponentRef { return OpaqueComponentRef.createInstance('v1', { id: options.id, TProps: null as unknown as TInnerComponentProps, TExternalProps: null as unknown as TExternalComponentProps, - TMode: null as unknown as TMode, toString() { return `ComponentRef{id=${options.id}}`; }, options: { - mode: options.mode, - defaultComponent: options.defaultComponent, + loader: options.loader, transformProps: options.transformProps, } as (typeof OpaqueComponentRef.TInternal)['options'], }); diff --git a/packages/frontend-plugin-api/src/components/makeComponentFromRef.test.tsx b/packages/frontend-plugin-api/src/components/makeComponentFromRef.test.tsx index ffaa9d5ccf..a35ede3e09 100644 --- a/packages/frontend-plugin-api/src/components/makeComponentFromRef.test.tsx +++ b/packages/frontend-plugin-api/src/components/makeComponentFromRef.test.tsx @@ -22,15 +22,16 @@ describe('makeComponentFromRef', () => { it('should create a component from a ref for sync component', () => { const ref = createComponentRef({ id: 'random', - mode: 'sync', - defaultComponent: (props: { name: string }) => { + loader: () => (props: { name: string }) => { return
{props.name}
; }, + transformProps: (props: { id: string }) => ({ + name: props.id, + }), }); const Component = makeComponentFromRef({ ref }); - - render(); + render(); expect(screen.getByTestId('test')).toHaveTextContent('test'); }); @@ -38,7 +39,6 @@ describe('makeComponentFromRef', () => { it('should render a fallback when theres no default implementation provided', () => { const ref = createComponentRef({ id: 'random', - mode: 'sync', }); const Component = makeComponentFromRef({ ref }); @@ -51,11 +51,10 @@ describe('makeComponentFromRef', () => { it('should map props from external to internal', () => { const ref = createComponentRef({ id: 'random', - mode: 'sync', transformProps: (props: { name: string }) => ({ uppercase: props.name.toUpperCase(), }), - defaultComponent: props => { + loader: () => props => { // @ts-expect-error as uppercase is types as a string const test: number = props.uppercase; @@ -75,8 +74,7 @@ describe('makeComponentFromRef', () => { it('should create a component from a ref for async component', async () => { const ref = createComponentRef({ id: 'random', - mode: 'async', - defaultComponent: async () => (props: { name: string }) => { + loader: async () => (props: { name: string }) => { return
{props.name}
; }, }); @@ -91,7 +89,6 @@ describe('makeComponentFromRef', () => { it('should render a fallback when theres no default implementation provided', async () => { const ref = createComponentRef({ id: 'random', - mode: 'async', }); const Component = makeComponentFromRef({ ref }); @@ -104,11 +101,10 @@ describe('makeComponentFromRef', () => { it('should map props from external to internal', async () => { const ref = createComponentRef({ id: 'random', - mode: 'async', transformProps: (props: { name: string }) => ({ uppercase: props.name.toUpperCase(), }), - defaultComponent: async () => props => { + loader: async () => props => { // @ts-expect-error as uppercase is types as a string const test: number = props.uppercase; diff --git a/packages/frontend-plugin-api/src/components/makeComponentFromRef.tsx b/packages/frontend-plugin-api/src/components/makeComponentFromRef.tsx index 3bf7cf69ad..2f0155fbec 100644 --- a/packages/frontend-plugin-api/src/components/makeComponentFromRef.tsx +++ b/packages/frontend-plugin-api/src/components/makeComponentFromRef.tsx @@ -33,29 +33,24 @@ export function makeComponentFromRef< const ComponentRefImpl = (props: ExternalComponentProps) => { const innerProps = options.transformProps?.(props) ?? props; - if (options.mode === 'sync') { - const DefaultImplementation = - options.defaultComponent ?? FallbackComponent; + const ComponentOrPromise = options.loader?.() ?? FallbackComponent; - return ; + if ('then' in ComponentOrPromise) { + const DefaultImplementation = lazy(() => + ComponentOrPromise.then(c => { + return { default: c }; + }), + ); + + return ( + // todo: is this necessary? can we remove this? + + + + ); } - const DefaultImplementation = lazy( - () => - options.defaultComponent?.().then(c => { - return { default: c }; - }) ?? - Promise.resolve({ - default: FallbackComponent, - }), - ); - - return ( - // todo: is this necessary? can we remove this? - - - - ); + return ; }; return ComponentRefImpl;