Skip to content

Commit d6a14f5

Browse files
committed
cleaned up IconDefinition vs IconData, reverted createIcon props interface rename, updated comments
1 parent 6ae0035 commit d6a14f5

2 files changed

Lines changed: 33 additions & 60 deletions

File tree

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

Lines changed: 11 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -79,20 +79,6 @@ test('accepts flat createIcon({ svgPath }) shape', () => {
7979
expect(screen.getByRole('img', { hidden: true }).querySelector('path')).toHaveAttribute('d', 'legacy-path');
8080
});
8181

82-
test('createIconBase accepts nested icon with deprecated svgPath field', () => {
83-
const nestedLegacyPath: CreateIconBaseProps = {
84-
name: 'NestedLegacyPathIcon',
85-
icon: {
86-
width: 8,
87-
height: 8,
88-
svgPath: 'nested-legacy-d'
89-
}
90-
};
91-
const NestedIcon = createIconBase(nestedLegacyPath);
92-
render(<NestedIcon />);
93-
expect(screen.getByRole('img', { hidden: true }).querySelector('path')).toHaveAttribute('d', 'nested-legacy-d');
94-
});
95-
9682
test('sets correct svgClassName by default', () => {
9783
render(<RhStandardIcon />);
9884
expect(screen.getByRole('img', { hidden: true })).toHaveClass('pf-v6-icon-rh-standard');
@@ -108,15 +94,6 @@ test('does not set svgClassName when noDefaultStyle is true', () => {
10894
expect(screen.getByRole('img', { hidden: true })).not.toHaveClass('pf-v6-icon-rh-standard');
10995
});
11096

111-
test('throws when createIconBase omits icon', () => {
112-
expect(() =>
113-
createIconBase({
114-
name: 'MissingDefaultIcon',
115-
rhUiIcon: null
116-
})
117-
).toThrow('@patternfly/react-icons: createIconBase requires an `icon` definition (name: MissingDefaultIcon).');
118-
});
119-
12097
test('sets correct svgPath if array', () => {
12198
render(<SVGArrayIcon />);
12299
const paths = screen.getByRole('img', { hidden: true }).querySelectorAll('path');
@@ -244,4 +221,15 @@ describe('rh-ui mapping: nested SVGs, set prop, and warnings', () => {
244221
warnSpy.mockRestore();
245222
}
246223
});
224+
225+
test('warns when createIconBase omits icon', () => {
226+
const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {});
227+
createIconBase({
228+
name: 'MissingDefaultIcon',
229+
rhUiIcon: null
230+
});
231+
expect(warnSpy).toHaveBeenCalledWith(
232+
'@patternfly/react-icons: createIconBase is missing an `icon` definition (name: MissingDefaultIcon).'
233+
);
234+
});
247235
});

packages/react-icons/src/createIcon.tsx

Lines changed: 22 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -6,40 +6,36 @@ export interface SVGPathObject {
66
}
77

88
/** Icon data format */
9-
export interface IconDefinition {
9+
export interface IconData {
1010
name?: string;
1111
width: number;
1212
height: number;
1313
xOffset?: number;
1414
yOffset?: number;
1515
svgClassName?: string;
1616
svgPathData?: string | SVGPathObject[];
17-
/** @deprecated Use `svgPathData` for nested definitions. */
18-
svgPath?: string | SVGPathObject[];
1917
}
2018

21-
/** Argument shape for {@link createIconBase} (module-private interface, not exported). */
19+
/** Internal API props */
2220
interface CreateIconBaseProps {
2321
name?: string;
24-
icon?: IconDefinition;
25-
rhUiIcon?: IconDefinition | null;
22+
icon?: IconData;
23+
rhUiIcon?: IconData | null;
2624
}
2725

28-
/**
29-
* Flat props shape for {@link createIcon}: layout fields plus `svgPath` (mapped to {@link IconDefinition.svgPathData}
30-
* on the nested `icon` before calling {@link createIconBase}).
31-
*/
32-
export interface CreateIconProps {
26+
/** Public API props */
27+
export interface IconDefinition {
3328
name?: string;
3429
width: number;
3530
height: number;
3631
xOffset?: number;
3732
yOffset?: number;
3833
svgPath?: string | SVGPathObject[];
3934
svgClassName?: string;
40-
rhUiIcon?: IconDefinition | null;
35+
rhUiIcon?: IconData | null;
4136
}
4237

38+
/** Additional svg props */
4339
export interface SVGIconProps extends Omit<React.HTMLProps<SVGElement>, 'ref'> {
4440
title?: string;
4541
className?: string;
@@ -51,7 +47,7 @@ export interface SVGIconProps extends Omit<React.HTMLProps<SVGElement>, 'ref'> {
5147

5248
let currentId = 0;
5349

54-
/** Renders `<path>` elements from `svgPathData` (string `d` or array of `{ path, className? }`). */
50+
/** Returns svg path(s) from a given svg data object. */
5551
function getSvgPaths(svgPathData: string | SVGPathObject[] | undefined): ReactNode {
5652
return svgPathData && Array.isArray(svgPathData) ? (
5753
svgPathData.map((pathObject, index) => (
@@ -62,9 +58,8 @@ function getSvgPaths(svgPathData: string | SVGPathObject[] | undefined): ReactNo
6258
);
6359
}
6460

65-
const createSvg = (icon: IconDefinition, iconClassName: string) => {
61+
const createInnerSvg = (icon: IconData, iconClassName: string) => {
6662
const { xOffset, yOffset, width, height, svgPathData, svgClassName } = icon ?? {};
67-
const resolvedPathData = svgPathData ?? icon?.svgPath;
6863
const _xOffset = xOffset ?? 0;
6964
const _yOffset = yOffset ?? 0;
7065
const viewBox = [_xOffset, _yOffset, width, height].join(' ');
@@ -80,21 +75,20 @@ const createSvg = (icon: IconDefinition, iconClassName: string) => {
8075

8176
return (
8277
<svg viewBox={viewBox} className={classNames.join(' ')}>
83-
{getSvgPaths(resolvedPathData)}
78+
{getSvgPaths(svgPathData)}
8479
</svg>
8580
);
8681
};
8782

8883
/**
89-
* Nested icon factory (used by generated icon modules under `dist`). Omitted from published typings when
90-
* `stripInternal` is enabled; use {@link createIcon} for the flat `svgPath` + layout props shape.
91-
*
84+
* Internal API for creating icons. Subject to change at any time. Please use `createIcon` instead.
9285
* @internal
9386
*/
9487
export function createIconBase({ name, icon, rhUiIcon = null }: CreateIconBaseProps): ComponentClass<SVGIconProps> {
9588
if (icon == null) {
96-
throw new Error(
97-
`@patternfly/react-icons: createIconBase requires an \`icon\` definition (name: ${name ?? 'unknown'}).`
89+
// eslint-disable-next-line no-console
90+
console.warn(
91+
`@patternfly/react-icons: createIconBase is missing an \`icon\` definition (name: ${name ?? 'unknown'}).`
9892
);
9993
}
10094
return class SVGIcon extends Component<SVGIconProps> {
@@ -106,10 +100,6 @@ export function createIconBase({ name, icon, rhUiIcon = null }: CreateIconBasePr
106100
noDefaultStyle: false
107101
};
108102

109-
/**
110-
* One root `<svg>`: a single flat variant when `set` is defined, or when `set` is omitted and `rhUiIcon` is
111-
* null; when `set` is omitted and `rhUiIcon` is set, nested inner `<svg>`s (default + RH UI swap layout).
112-
*/
113103
render() {
114104
const { title, className: propsClassName, set, noDefaultStyle, ...props } = this.props;
115105

@@ -121,7 +111,7 @@ export function createIconBase({ name, icon, rhUiIcon = null }: CreateIconBasePr
121111
}
122112

123113
if (set === 'rh-ui' && rhUiIcon === null) {
124-
// eslint-disable-next-line no-console -- same behavior as main branch createIcon
114+
// eslint-disable-next-line no-console
125115
console.warn(
126116
`Set "rh-ui" was provided for ${name}, but no rh-ui icon data exists for this icon. The default icon will be rendered.`
127117
);
@@ -130,7 +120,6 @@ export function createIconBase({ name, icon, rhUiIcon = null }: CreateIconBasePr
130120
if ((set === undefined && rhUiIcon === null) || set !== undefined) {
131121
const iconData = set !== undefined && set === 'rh-ui' && rhUiIcon !== null ? rhUiIcon : icon;
132122
const { xOffset, yOffset, width, height, svgPathData, svgClassName } = iconData ?? {};
133-
const resolvedPathData = svgPathData ?? iconData?.svgPath;
134123
const _xOffset = xOffset ?? 0;
135124
const _yOffset = yOffset ?? 0;
136125
const viewBox = [_xOffset, _yOffset, width, height].join(' ');
@@ -152,7 +141,7 @@ export function createIconBase({ name, icon, rhUiIcon = null }: CreateIconBasePr
152141
{...(props as Omit<React.SVGProps<SVGElement>, 'ref'>)} // Lie.
153142
>
154143
{hasTitle && <title id={this.id}>{title}</title>}
155-
{getSvgPaths(resolvedPathData)}
144+
{getSvgPaths(svgPathData)}
156145
</svg>
157146
);
158147
}
@@ -168,22 +157,18 @@ export function createIconBase({ name, icon, rhUiIcon = null }: CreateIconBasePr
168157
{...(props as Omit<React.SVGProps<SVGElement>, 'ref'>)} // Lie.
169158
>
170159
{hasTitle && <title id={this.id}>{title}</title>}
171-
{icon && createSvg(icon, 'pf-v6-icon-default')}
172-
{rhUiIcon && createSvg(rhUiIcon, 'pf-v6-icon-rh-ui')}
160+
{icon && createInnerSvg(icon, 'pf-v6-icon-default')}
161+
{rhUiIcon && createInnerSvg(rhUiIcon, 'pf-v6-icon-rh-ui')}
173162
</svg>
174163
);
175164
}
176165
};
177166
}
178167

179-
/**
180-
* Maps {@link CreateIconProps} (flat layout + `svgPath`) to a nested {@link IconDefinition} (`svgPath` →
181-
* `svgPathData`), then returns the component class from {@link createIconBase}. Generated icon modules call
182-
* {@link createIconBase} directly with nested data (see `@internal` on `createIconBase`).
183-
*/
184-
export function createIcon(props: CreateIconProps): ComponentClass<SVGIconProps> {
168+
/** Public API for creating icons. Will be maintained up to breaking releases. */
169+
export function createIcon(props: IconDefinition): ComponentClass<SVGIconProps> {
185170
const { rhUiIcon = null, ...rest } = props;
186-
const icon: IconDefinition = {
171+
const icon: IconData = {
187172
name: rest.name,
188173
width: rest.width,
189174
height: rest.height,

0 commit comments

Comments
 (0)