Skip to content

Commit 764d78b

Browse files
tlabajtlabaj
committed
fix(createIcon): Fix broken API in createIcon.tsx
1 parent 911223a commit 764d78b

File tree

2 files changed

+210
-13
lines changed

2 files changed

+210
-13
lines changed

‎packages/react-icons/src/__tests__/createIcon.test.tsx‎

Lines changed: 108 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
1+
// eslint-disable-next-line no-restricted-imports -- test file excluded from package tsconfig; default import satisfies TS/JSX
2+
import React from 'react';
13
import { render, screen } from '@testing-library/react';
2-
import { IconDefinition, CreateIconProps, createIcon, SVGPathObject } from '../createIcon';
4+
import { IconDefinition, CreateIconProps, createIcon, LegacyFlatIconDefinition, SVGPathObject } from '../createIcon';
35

46
const multiPathIcon: IconDefinition = {
57
name: 'IconName',
@@ -43,7 +45,37 @@ test('sets correct viewBox', () => {
4345

4446
test('sets correct svgPath if string', () => {
4547
render(<SVGIcon />);
46-
expect(screen.getByRole('img', { hidden: true }).querySelector('path')).toHaveAttribute('d', iconDef.svgPath);
48+
expect(screen.getByRole('img', { hidden: true }).querySelector('path')).toHaveAttribute(
49+
'd',
50+
singlePathIcon.svgPathData
51+
);
52+
});
53+
54+
test('accepts legacy flat createIcon({ svgPath }) shape', () => {
55+
const legacyDef: LegacyFlatIconDefinition = {
56+
name: 'LegacyIcon',
57+
width: 10,
58+
height: 20,
59+
svgPath: 'legacy-path',
60+
svgClassName: 'legacy-svg'
61+
};
62+
const LegacySVGIcon = createIcon(legacyDef);
63+
render(<LegacySVGIcon />);
64+
expect(screen.getByRole('img', { hidden: true }).querySelector('path')).toHaveAttribute('d', 'legacy-path');
65+
});
66+
67+
test('accepts CreateIconProps with nested icon using deprecated svgPath field', () => {
68+
const nestedLegacyPath: CreateIconProps = {
69+
name: 'NestedLegacyPathIcon',
70+
icon: {
71+
width: 8,
72+
height: 8,
73+
svgPath: 'nested-legacy-d'
74+
}
75+
};
76+
const NestedIcon = createIcon(nestedLegacyPath);
77+
render(<NestedIcon />);
78+
expect(screen.getByRole('img', { hidden: true }).querySelector('path')).toHaveAttribute('d', 'nested-legacy-d');
4779
});
4880

4981
test('sets correct svgPath if array', () => {
@@ -98,3 +130,77 @@ test('additional props should be spread to the root svg element', () => {
98130
render(<SVGIcon data-testid="icon" />);
99131
expect(screen.getByTestId('icon')).toBeInTheDocument();
100132
});
133+
134+
describe('rh-ui mapping: nested SVGs, set prop, and warnings', () => {
135+
const defaultPath = 'M0 0-default';
136+
const rhUiPath = 'M0 0-rh-ui';
137+
138+
const defaultIconDef: IconDefinition = {
139+
name: 'DefaultVariant',
140+
width: 16,
141+
height: 16,
142+
svgPathData: defaultPath
143+
};
144+
145+
const rhUiIconDef: IconDefinition = {
146+
name: 'RhUiVariant',
147+
width: 16,
148+
height: 16,
149+
svgPathData: rhUiPath
150+
};
151+
152+
const dualConfig: CreateIconProps = {
153+
name: 'DualMappedIcon',
154+
icon: defaultIconDef,
155+
rhUiIcon: rhUiIconDef
156+
};
157+
158+
const DualMappedIcon = createIcon(dualConfig);
159+
160+
test('renders two nested inner svgs when rhUiIcon is set and `set` is omitted (swap layout)', () => {
161+
render(<DualMappedIcon />);
162+
const root = screen.getByRole('img', { hidden: true });
163+
expect(root).toHaveClass('pf-v6-svg');
164+
const innerSvgs = root.querySelectorAll(':scope > svg');
165+
expect(innerSvgs).toHaveLength(2);
166+
expect(root?.querySelector('.pf-v6-icon-default path')).toHaveAttribute('d', defaultPath);
167+
expect(root?.querySelector('.pf-v6-icon-rh-ui path')).toHaveAttribute('d', rhUiPath);
168+
});
169+
170+
test('set="default" renders a single flat svg using the default icon paths', () => {
171+
render(<DualMappedIcon set="default" />);
172+
const root = screen.getByRole('img', { hidden: true });
173+
expect(root.querySelectorAll(':scope > svg')).toHaveLength(0);
174+
expect(root).toHaveAttribute('viewBox', '0 0 16 16');
175+
expect(root.querySelector('path')).toHaveAttribute('d', defaultPath);
176+
expect(root.querySelectorAll('svg')).toHaveLength(0);
177+
});
178+
179+
test('set="rh-ui" renders a single flat svg using the rh-ui icon paths', () => {
180+
render(<DualMappedIcon set="rh-ui" />);
181+
const root = screen.getByRole('img', { hidden: true });
182+
expect(root.querySelectorAll(':scope > svg')).toHaveLength(0);
183+
expect(root.querySelector('path')).toHaveAttribute('d', rhUiPath);
184+
expect(root.querySelectorAll('svg')).toHaveLength(0);
185+
});
186+
187+
test('set="rh-ui" with no rhUiIcon mapping falls back to default and warns', () => {
188+
const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {});
189+
const IconNoRhMapping = createIcon({
190+
name: 'NoRhMappingIcon',
191+
icon: defaultIconDef,
192+
rhUiIcon: null
193+
});
194+
195+
render(<IconNoRhMapping set="rh-ui" />);
196+
197+
expect(warnSpy).toHaveBeenCalledWith(
198+
'Set "rh-ui" was provided for NoRhMappingIcon, but no rh-ui icon data exists for this icon. The default icon will be rendered.'
199+
);
200+
const root = screen.getByRole('img', { hidden: true });
201+
expect(root.querySelector('path')).toHaveAttribute('d', defaultPath);
202+
expect(root.querySelectorAll('svg')).toHaveLength(0);
203+
204+
warnSpy.mockRestore();
205+
});
206+
});

‎packages/react-icons/src/createIcon.tsx‎

Lines changed: 102 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,22 +5,42 @@ export interface SVGPathObject {
55
className?: string;
66
}
77

8-
export interface IconDefinition {
8+
export interface IconDefinitionBase {
99
name?: string;
1010
width: number;
1111
height: number;
12-
svgPathData: string | SVGPathObject[];
1312
xOffset?: number;
1413
yOffset?: number;
1514
svgClassName?: string;
1615
}
1716

17+
/** Icon metadata using the current `svgPathData` field name. */
18+
export interface IconDefinitionWithSvgPathData extends IconDefinitionBase {
19+
svgPathData: string | SVGPathObject[];
20+
}
21+
22+
/**
23+
* @deprecated Use {@link IconDefinitionWithSvgPathData} with `svgPathData` instead.
24+
*/
25+
export interface IconDefinitionWithSvgPath extends IconDefinitionBase {
26+
svgPath: string | SVGPathObject[];
27+
}
28+
29+
/** Describes SVG path content for one icon variant (default or rh-ui). */
30+
export type IconDefinition = IconDefinitionWithSvgPathData | IconDefinitionWithSvgPath;
31+
1832
export interface CreateIconProps {
1933
name?: string;
2034
icon?: IconDefinition;
2135
rhUiIcon?: IconDefinition | null;
2236
}
2337

38+
/**
39+
* @deprecated The previous `createIcon` accepted a flat {@link IconDefinition} with top-level
40+
* `svgPath`. Pass {@link CreateIconProps} with a nested `icon` field instead.
41+
*/
42+
export type LegacyFlatIconDefinition = IconDefinition;
43+
2444
export interface SVGIconProps extends Omit<React.HTMLProps<SVGElement>, 'ref'> {
2545
title?: string;
2646
className?: string;
@@ -30,7 +50,56 @@ export interface SVGIconProps extends Omit<React.HTMLProps<SVGElement>, 'ref'> {
3050

3151
let currentId = 0;
3252

33-
const createSvg = (icon: IconDefinition, iconClassName: string) => {
53+
function resolveSvgPathData(icon: IconDefinition): string | SVGPathObject[] {
54+
if ('svgPathData' in icon && icon.svgPathData !== undefined) {
55+
return icon.svgPathData;
56+
}
57+
if ('svgPath' in icon && icon.svgPath !== undefined) {
58+
return icon.svgPath;
59+
}
60+
throw new Error('@patternfly/react-icons: IconDefinition must define svgPathData or svgPath');
61+
}
62+
63+
function normalizeIconDefinition(icon: IconDefinition): IconDefinitionWithSvgPathData {
64+
return {
65+
name: icon.name,
66+
width: icon.width,
67+
height: icon.height,
68+
svgPathData: resolveSvgPathData(icon),
69+
xOffset: icon.xOffset,
70+
yOffset: icon.yOffset,
71+
svgClassName: icon.svgClassName
72+
};
73+
}
74+
75+
function isNestedCreateIconProps(arg: object): arg is CreateIconProps {
76+
return 'icon' in arg || 'rhUiIcon' in arg;
77+
}
78+
79+
/** Props after resolving legacy `svgPath` and flat `createIcon` arguments. */
80+
interface NormalizedCreateIconProps {
81+
name?: string;
82+
icon?: IconDefinitionWithSvgPathData;
83+
rhUiIcon: IconDefinitionWithSvgPathData | null;
84+
}
85+
86+
function normalizeCreateIconArg(arg: CreateIconProps | LegacyFlatIconDefinition): NormalizedCreateIconProps {
87+
if (isNestedCreateIconProps(arg)) {
88+
const p = arg as CreateIconProps;
89+
return {
90+
name: p.name,
91+
icon: p.icon !== undefined && p.icon !== null ? normalizeIconDefinition(p.icon) : undefined,
92+
rhUiIcon: p.rhUiIcon != null ? normalizeIconDefinition(p.rhUiIcon) : null
93+
};
94+
}
95+
return {
96+
name: (arg as LegacyFlatIconDefinition).name,
97+
icon: normalizeIconDefinition(arg as IconDefinition),
98+
rhUiIcon: null
99+
};
100+
}
101+
102+
const createSvg = (icon: IconDefinitionWithSvgPathData, iconClassName: string) => {
34103
const { xOffset, yOffset, width, height, svgPathData, svgClassName } = icon ?? {};
35104
const _xOffset = xOffset ?? 0;
36105
const _yOffset = yOffset ?? 0;
@@ -62,18 +131,38 @@ const createSvg = (icon: IconDefinition, iconClassName: string) => {
62131
};
63132

64133
/**
65-
* Factory to create Icon class components for consumers
134+
* Builds a React **class** component that renders a PatternFly SVG icon (`role="img"`, optional `<title>` for a11y).
135+
*
136+
* **Argument shape — pick one:**
137+
*
138+
* 1. **`CreateIconProps` (preferred)** — `{ name?, icon?, rhUiIcon? }`. Dimensions and path data sit on `icon`
139+
* (and optionally on `rhUiIcon` for Red Hat UI–mapped icons). If the object **has an `icon` or `rhUiIcon` key**
140+
* (including `rhUiIcon: null`), this shape is assumed.
141+
*
142+
* 2. **Legacy flat `IconDefinition`** — the same fields as `icon`, but at the **top level** (no nested `icon`).
143+
* Still accepted so existing callers are not broken. Prefer migrating to `CreateIconProps`.
144+
*
145+
* **Path data on each `IconDefinition`:** use `svgPathData` (string or {@link SVGPathObject}[]). The old name
146+
* `svgPath` is deprecated but still read; `svgPathData` wins if both are present.
147+
*
148+
* **Default vs RH UI rendering:** If `rhUiIcon` is set and the consumer does **not** pass `set` on the component,
149+
* the output is an outer `<svg.pf-v6-svg>` containing **two** inner `<svg>`s (default + rh-ui) so CSS can swap
150+
* which variant is visible. If `set` is `"default"` or `"rh-ui"`, a **single** flat `<svg>` is rendered for that
151+
* variant. Requesting `set="rh-ui"` when there is no `rhUiIcon` falls back to the default glyph and logs a
152+
* `console.warn` (see implementation).
153+
*
154+
* @param arg Icon configuration: either {@link CreateIconProps} (nested `icon` / `rhUiIcon`) or a legacy flat
155+
* {@link LegacyFlatIconDefinition}. Runtime detection follows the rules in **Argument shape** above.
156+
* @returns A `ComponentClass<SVGIconProps>` — render it as `<YourIcon />` or with `title`, `className`, `set`, etc.
66157
*/
67-
export function createIcon({ name, icon, rhUiIcon = null }: CreateIconProps): React.ComponentClass<SVGIconProps> {
158+
export function createIcon(arg: CreateIconProps | LegacyFlatIconDefinition): React.ComponentClass<SVGIconProps> {
159+
const { name, icon, rhUiIcon = null } = normalizeCreateIconArg(arg);
160+
68161
return class SVGIcon extends Component<SVGIconProps> {
69162
static displayName = name;
70163

71164
id = `icon-title-${currentId++}`;
72165

73-
constructor(props: SVGIconProps) {
74-
super(props);
75-
}
76-
77166
render() {
78167
const { title, className: propsClassName, set, ...props } = this.props;
79168

@@ -92,8 +181,10 @@ export function createIcon({ name, icon, rhUiIcon = null }: CreateIconProps): Re
92181
}
93182

94183
if ((set === undefined && rhUiIcon === null) || set !== undefined) {
95-
const iconData = set !== undefined && set === 'rh-ui' && rhUiIcon !== null ? rhUiIcon : icon;
96-
const { xOffset, yOffset, width, height, svgPathData, svgClassName } = iconData ?? {};
184+
const iconData: IconDefinitionWithSvgPathData | undefined =
185+
set !== undefined && set === 'rh-ui' && rhUiIcon !== null ? rhUiIcon : icon;
186+
const { xOffset, yOffset, width, height, svgPathData, svgClassName } =
187+
iconData ?? ({} as Partial<IconDefinitionWithSvgPathData>);
97188
const _xOffset = xOffset ?? 0;
98189
const _yOffset = yOffset ?? 0;
99190
const viewBox = [_xOffset, _yOffset, width, height].join(' ');

0 commit comments

Comments
 (0)