diff --git a/packages/core-compat-api/src/compatWrapper/ForwardsCompatProvider.tsx b/packages/core-compat-api/src/compatWrapper/ForwardsCompatProvider.tsx index 6464032673..1ca118eb88 100644 --- a/packages/core-compat-api/src/compatWrapper/ForwardsCompatProvider.tsx +++ b/packages/core-compat-api/src/compatWrapper/ForwardsCompatProvider.tsx @@ -29,6 +29,7 @@ import { ProgressProps, ExternalRouteRef, IconComponent, + IconElement, IconsApi, RouteFunc, RouteRef, @@ -41,7 +42,7 @@ import { NotFoundErrorPage, ErrorDisplay, } from '@backstage/frontend-plugin-api'; -import { ComponentType, useMemo } from 'react'; +import { ComponentType, createElement, useMemo } from 'react'; import { ReactNode } from 'react'; import { toLegacyPlugin } from './BackwardsCompatProvider'; // eslint-disable-next-line @backstage/no-relative-monorepo-imports @@ -99,6 +100,11 @@ class CompatIconsApi implements IconsApi { this.#app = app; } + icon(key: string): IconElement | undefined { + const Icon = this.#app.getSystemIcon(key); + return Icon ? createElement(Icon) : undefined; + } + getIcon(key: string): IconComponent | undefined { return this.#app.getSystemIcon(key); } diff --git a/packages/core-plugin-api/src/app/useApp.test.tsx b/packages/core-plugin-api/src/app/useApp.test.tsx index 2cee7f6f3c..7888df0464 100644 --- a/packages/core-plugin-api/src/app/useApp.test.tsx +++ b/packages/core-plugin-api/src/app/useApp.test.tsx @@ -50,6 +50,9 @@ describe('useApp', () => { describe('new system', () => { const mockIcon = () => null; const mockIconsApi: IconsApi = { + icon: jest.fn((key: string) => + key === 'test-icon' ? mockIcon() : undefined, + ), getIcon: jest.fn((key: string) => key === 'test-icon' ? mockIcon : undefined, ), diff --git a/packages/frontend-app-api/src/apis/implementations/IconsApi/DefaultIconsApi.test.ts b/packages/frontend-app-api/src/apis/implementations/IconsApi/DefaultIconsApi.test.ts index e9bbfc328e..e8c5e0a8b9 100644 --- a/packages/frontend-app-api/src/apis/implementations/IconsApi/DefaultIconsApi.test.ts +++ b/packages/frontend-app-api/src/apis/implementations/IconsApi/DefaultIconsApi.test.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { createElement, type ReactNode } from 'react'; +import { createElement } from 'react'; import { DefaultIconsApi } from './DefaultIconsApi'; describe('DefaultIconsApi', () => { @@ -24,6 +24,7 @@ describe('DefaultIconsApi', () => { it('should return undefined for unknown keys', () => { const api = new DefaultIconsApi({}); + expect(api.icon('missing')).toBeUndefined(); expect(api.getIcon('missing')).toBeUndefined(); }); @@ -37,86 +38,73 @@ describe('DefaultIconsApi', () => { expect(api.listIconKeys()).toEqual(['a', 'b', 'c']); }); - it('should support IconElement values and wrap them in a component', () => { - const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); + it('should return IconElement values directly via icon()', () => { const element = createElement('span', null, 'test-icon'); const api = new DefaultIconsApi({ myIcon: element }); - const Icon = api.getIcon('myIcon'); - expect(Icon).toBeDefined(); - expect(typeof Icon).toBe('function'); - expect((Icon! as (props: object) => ReactNode)({})).toBe(element); - expect(warnSpy).not.toHaveBeenCalled(); + expect(api.icon('myIcon')).toBe(element); }); - it('should support null IconElement values', () => { - const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); + it('should return null IconElement values via icon()', () => { const api = new DefaultIconsApi({ empty: null }); - - const Icon = api.getIcon('empty'); - expect(Icon).toBeDefined(); - expect(typeof Icon).toBe('function'); - expect((Icon! as (props: object) => ReactNode)({})).toBeNull(); - expect(warnSpy).not.toHaveBeenCalled(); + expect(api.icon('empty')).toBeNull(); }); - it('should support IconComponent values and return them directly', () => { - const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); - const MyIcon = () => createElement('span', null, 'component-icon'); + it('should convert IconComponent values to elements for icon()', () => { + jest.spyOn(console, 'warn').mockImplementation(() => {}); + const MyIcon = () => createElement('span', null, 'rendered'); const api = new DefaultIconsApi({ myIcon: MyIcon }); - expect(api.getIcon('myIcon')).toBe(MyIcon); - expect(warnSpy).toHaveBeenCalledTimes(1); - expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('myIcon')); + const result = api.icon('myIcon'); + expect(result).toBeTruthy(); + // @ts-expect-error accessing internal React element structure + expect(result.type).toBe(MyIcon); + }); + + it('should wrap IconElement values in a component for getIcon()', () => { + const element = createElement('span', null, 'test-icon'); + const api = new DefaultIconsApi({ myIcon: element }); + + const icon = api.getIcon('myIcon'); + expect(icon).toBeDefined(); + expect(typeof icon).toBe('function'); + // @ts-expect-error testing runtime behavior + expect(icon({})).toBe(element); + expect(api.getIcon('myIcon')).toBe(icon); + }); + + it('should wrap null IconElement in a component for getIcon()', () => { + const api = new DefaultIconsApi({ empty: null }); + + const icon = api.getIcon('empty'); + expect(icon).toBeDefined(); + expect(typeof icon).toBe('function'); + // @ts-expect-error testing runtime behavior + expect(icon({})).toBeNull(); }); it('should log a single warning listing all IconComponent keys', () => { const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); - const A = () => createElement('span'); - const B = () => createElement('span'); - const api = new DefaultIconsApi({ - a: A, + void new DefaultIconsApi({ + a: () => createElement('span'), elem: createElement('span'), - b: B, + b: () => createElement('span'), empty: null, }); - expect(api.getIcon('a')).toBe(A); - expect(api.getIcon('b')).toBe(B); - expect(api.getIcon('elem')).toEqual(expect.any(Function)); - expect(api.getIcon('empty')).toEqual(expect.any(Function)); expect(warnSpy).toHaveBeenCalledTimes(1); expect(warnSpy).toHaveBeenCalledWith(expect.stringMatching(/a, b$/)); }); - it('should handle a mix of IconComponent and IconElement values', () => { + it('should not warn when only IconElement values are provided', () => { const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); - const element = createElement('span', null, 'elem'); - const Component = () => createElement('span', null, 'comp'); - const api = new DefaultIconsApi({ - elem: element, - comp: Component, + void new DefaultIconsApi({ + element: createElement('span'), empty: null, }); - // Element - wrapped in a component, returned via getIcon - const ElemIcon = api.getIcon('elem'); - expect(ElemIcon).toBeDefined(); - expect((ElemIcon! as (props: object) => ReactNode)({})).toBe(element); - - // Component - returned directly - expect(api.getIcon('comp')).toBe(Component); - - // Null element - wrapped in a component - const EmptyIcon = api.getIcon('empty'); - expect(EmptyIcon).toBeDefined(); - expect((EmptyIcon! as (props: object) => ReactNode)({})).toBeNull(); - - // Single warning mentioning only the component key - expect(warnSpy).toHaveBeenCalledTimes(1); - expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('comp')); - expect(warnSpy).toHaveBeenCalledWith(expect.not.stringContaining('elem')); + expect(warnSpy).not.toHaveBeenCalled(); }); }); diff --git a/packages/frontend-app-api/src/apis/implementations/IconsApi/DefaultIconsApi.ts b/packages/frontend-app-api/src/apis/implementations/IconsApi/DefaultIconsApi.ts index 43fef68cea..49fde51bf2 100644 --- a/packages/frontend-app-api/src/apis/implementations/IconsApi/DefaultIconsApi.ts +++ b/packages/frontend-app-api/src/apis/implementations/IconsApi/DefaultIconsApi.ts @@ -19,6 +19,7 @@ import { IconElement, IconsApi, } from '@backstage/frontend-plugin-api'; +import { createElement } from 'react'; /** * Implementation for the {@link IconsApi} @@ -26,18 +27,19 @@ import { * @internal */ export class DefaultIconsApi implements IconsApi { - #icons: Map; + #icons: Map; + #components = new Map(); constructor(icons: { [key in string]: IconComponent | IconElement }) { const deprecatedKeys: string[] = []; this.#icons = new Map( - Object.entries(icons).map(([key, icon]) => { - if (typeof icon === 'function') { + Object.entries(icons).map(([key, value]) => { + if (typeof value === 'function') { deprecatedKeys.push(key); - return [key, icon]; + return [key, createElement(value)]; } - return [key, () => icon]; + return [key, value]; }), ); @@ -45,16 +47,29 @@ export class DefaultIconsApi implements IconsApi { const keys = deprecatedKeys.join(', '); // eslint-disable-next-line no-console console.warn( - `The following icons were registered as IconComponent, which is deprecated. ` + - `Use IconElement instead by passing rather than MyIcon: ${keys}`, + `The following icons were registered as IconComponent, which is deprecated. Use IconElement instead by passing rather than MyIcon: ${keys}`, ); } } - getIcon(key: string): IconComponent | undefined { + icon(key: string): IconElement | undefined { return this.#icons.get(key); } + getIcon(key: string): IconComponent | undefined { + let component = this.#components.get(key); + if (component) { + return component; + } + const el = this.#icons.get(key); + if (el === undefined) { + return undefined; + } + component = () => el; + this.#components.set(key, component); + return component; + } + listIconKeys(): string[] { return Array.from(this.#icons.keys()); } diff --git a/packages/frontend-plugin-api/report.api.md b/packages/frontend-plugin-api/report.api.md index 1757994205..8f60926fe6 100644 --- a/packages/frontend-plugin-api/report.api.md +++ b/packages/frontend-plugin-api/report.api.md @@ -1447,8 +1447,9 @@ export type IconElement = JSX_2.Element | null; // @public export interface IconsApi { - // (undocumented) + // @deprecated (undocumented) getIcon(key: string): IconComponent | undefined; + icon(key: string): IconElement | undefined; // (undocumented) listIconKeys(): string[]; } diff --git a/packages/frontend-plugin-api/src/apis/definitions/IconsApi.ts b/packages/frontend-plugin-api/src/apis/definitions/IconsApi.ts index fbc9928dc1..d22ebcce4a 100644 --- a/packages/frontend-plugin-api/src/apis/definitions/IconsApi.ts +++ b/packages/frontend-plugin-api/src/apis/definitions/IconsApi.ts @@ -15,7 +15,7 @@ */ import { createApiRef } from '../system'; -import { IconComponent } from '../../icons'; +import { IconComponent, IconElement } from '../../icons'; /** * API for accessing app icons. @@ -23,6 +23,14 @@ import { IconComponent } from '../../icons'; * @public */ export interface IconsApi { + /** + * Look up an icon element by key. + */ + icon(key: string): IconElement | undefined; + + /** + * @deprecated Use {@link IconsApi.icon} instead. + */ getIcon(key: string): IconComponent | undefined; listIconKeys(): string[];