Skip to content

Commit fe6db88

Browse files
committed
updates from pr review
1 parent 43f8d0b commit fe6db88

3 files changed

Lines changed: 88 additions & 101 deletions

File tree

packages/react-icons/scripts/writeIcons.mjs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@ import { pfToRhIcons } from './icons/pfToRhIcons.mjs';
77
import * as url from 'url';
88
const __dirname = url.fileURLToPath(new URL('.', import.meta.url));
99

10-
// Import createIcon from compiled dist (build:esm must run first)
10+
// Import createIconBase from compiled dist (build:esm must run first)
1111
const createIconModule = await import('../dist/esm/createIcon.js');
12-
const createIcon = createIconModule.createIcon;
12+
const createIconBase = createIconModule.createIconBase;
1313

1414
const outDir = join(__dirname, '../dist');
1515
const staticDir = join(outDir, 'static');
@@ -27,7 +27,7 @@ exports.${jsName}Config = {
2727
icon: ${JSON.stringify(icon)},
2828
rhUiIcon: ${rhUiIcon ? JSON.stringify(rhUiIcon) : 'null'},
2929
};
30-
exports.${jsName} = require('../createIcon').createIcon(exports.${jsName}Config);
30+
exports.${jsName} = require('../createIcon').createIconBase(exports.${jsName}Config);
3131
exports["default"] = exports.${jsName};
3232
`.trim()
3333
);
@@ -36,15 +36,15 @@ exports["default"] = exports.${jsName};
3636
const writeESMExport = (fname, jsName, icon, rhUiIcon = null) => {
3737
outputFileSync(
3838
join(outDir, 'esm/icons', `${fname}.js`),
39-
`import { createIcon } from '../createIcon.js';
39+
`import { createIconBase } from '../createIcon.js';
4040
4141
export const ${jsName}Config = {
4242
name: '${jsName}',
4343
icon: ${JSON.stringify(icon)},
4444
rhUiIcon: ${rhUiIcon ? JSON.stringify(rhUiIcon) : 'null'},
4545
};
4646
47-
export const ${jsName} = createIcon(${jsName}Config);
47+
export const ${jsName} = createIconBase(${jsName}Config);
4848
4949
export default ${jsName};
5050
`.trim()
@@ -68,16 +68,16 @@ export default ${jsName};
6868
};
6969

7070
/**
71-
* Generates a static SVG string from icon data using createIcon
71+
* Generates a static SVG string from icon data using createIconBase
7272
* @param {string} iconName The name of the icon
7373
* @param {object} icon The icon data object
7474
* @returns {string} Static SVG markup
7575
*/
7676
function generateStaticSVG(iconName, icon) {
7777
const jsName = `${toCamel(iconName)}Icon`;
7878

79-
// Create icon component using createIcon
80-
const IconComponent = createIcon({
79+
// Create icon component using createIconBase
80+
const IconComponent = createIconBase({
8181
name: jsName,
8282
icon
8383
});

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

Lines changed: 24 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,12 @@
11
import { render, screen } from '@testing-library/react';
2-
import { IconDefinition, CreateIconProps, createIcon, LegacyFlatIconDefinition, SVGPathObject } from '../createIcon';
2+
import {
3+
IconDefinition,
4+
CreateIconBaseProps,
5+
createIcon,
6+
createIconBase,
7+
LegacyFlatIconDefinition,
8+
SVGPathObject
9+
} from '../createIcon';
310

411
const multiPathIcon: IconDefinition = {
512
name: 'IconName',
@@ -28,24 +35,24 @@ const rhStandardIcon: IconDefinition = {
2835
svgClassName: 'pf-v6-icon-rh-standard'
2936
};
3037

31-
const iconDef: CreateIconProps = {
38+
const iconDef: CreateIconBaseProps = {
3239
name: 'SinglePathIconName',
3340
icon: singlePathIcon
3441
};
3542

36-
const iconDefWithArrayPath: CreateIconProps = {
43+
const iconDefWithArrayPath: CreateIconBaseProps = {
3744
name: 'MultiPathIconName',
3845
icon: multiPathIcon
3946
};
4047

41-
const iconDefWithRhStandard: CreateIconProps = {
48+
const iconDefWithRhStandard: CreateIconBaseProps = {
4249
name: 'RhStandardIconName',
4350
icon: rhStandardIcon
4451
};
4552

46-
const SVGIcon = createIcon(iconDef);
47-
const SVGArrayIcon = createIcon(iconDefWithArrayPath);
48-
const RhStandardIcon = createIcon(iconDefWithRhStandard);
53+
const SVGIcon = createIconBase(iconDef);
54+
const SVGArrayIcon = createIconBase(iconDefWithArrayPath);
55+
const RhStandardIcon = createIconBase(iconDefWithRhStandard);
4956

5057
test('sets correct viewBox', () => {
5158
render(<SVGIcon />);
@@ -76,16 +83,16 @@ test('accepts legacy flat createIcon({ svgPath }) shape', () => {
7683
expect(screen.getByRole('img', { hidden: true }).querySelector('path')).toHaveAttribute('d', 'legacy-path');
7784
});
7885

79-
test('accepts CreateIconProps with nested icon using deprecated svgPath field', () => {
80-
const nestedLegacyPath: CreateIconProps = {
86+
test('createIconBase accepts nested icon with deprecated svgPath field', () => {
87+
const nestedLegacyPath: CreateIconBaseProps = {
8188
name: 'NestedLegacyPathIcon',
8289
icon: {
8390
width: 8,
8491
height: 8,
8592
svgPath: 'nested-legacy-d'
8693
}
8794
};
88-
const NestedIcon = createIcon(nestedLegacyPath);
95+
const NestedIcon = createIconBase(nestedLegacyPath);
8996
render(<NestedIcon />);
9097
expect(screen.getByRole('img', { hidden: true }).querySelector('path')).toHaveAttribute('d', 'nested-legacy-d');
9198
});
@@ -105,15 +112,13 @@ test('does not set svgClassName when noDefaultStyle is true', () => {
105112
expect(screen.getByRole('img', { hidden: true })).not.toHaveClass('pf-v6-icon-rh-standard');
106113
});
107114

108-
test('throws when nested CreateIconProps omits icon', () => {
115+
test('throws when createIconBase omits icon', () => {
109116
expect(() =>
110-
createIcon({
117+
createIconBase({
111118
name: 'MissingDefaultIcon',
112119
rhUiIcon: null
113-
})
114-
).toThrow(
115-
'@patternfly/react-icons: createIcon requires an `icon` definition when using nested CreateIconProps (name: MissingDefaultIcon).'
116-
);
120+
} as any)
121+
).toThrow('@patternfly/react-icons: createIconBase requires an `icon` definition (name: MissingDefaultIcon).');
117122
});
118123

119124
test('sets correct svgPath if array', () => {
@@ -187,13 +192,13 @@ describe('rh-ui mapping: nested SVGs, set prop, and warnings', () => {
187192
svgPathData: rhUiPath
188193
};
189194

190-
const dualConfig: CreateIconProps = {
195+
const dualConfig: CreateIconBaseProps = {
191196
name: 'DualMappedIcon',
192197
icon: defaultIconDef,
193198
rhUiIcon: rhUiIconDef
194199
};
195200

196-
const DualMappedIcon = createIcon(dualConfig);
201+
const DualMappedIcon = createIconBase(dualConfig);
197202

198203
test('renders two nested inner svgs when rhUiIcon is set and `set` is omitted (swap layout)', () => {
199204
render(<DualMappedIcon />);
@@ -225,7 +230,7 @@ describe('rh-ui mapping: nested SVGs, set prop, and warnings', () => {
225230
test('set="rh-ui" with no rhUiIcon mapping falls back to default and warns', () => {
226231
const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {});
227232
try {
228-
const IconNoRhMapping = createIcon({
233+
const IconNoRhMapping = createIconBase({
229234
name: 'NoRhMappingIcon',
230235
icon: defaultIconDef,
231236
rhUiIcon: null

packages/react-icons/src/createIcon.tsx

Lines changed: 56 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -35,16 +35,32 @@ export type IconDefinitionWithSvgPathData = Required<Pick<IconDefinition, 'svgPa
3535
*/
3636
export type IconDefinitionWithSvgPath = Required<Pick<IconDefinition, 'svgPath'>> & IconDefinition;
3737

38-
/** When passing `icon` or `rhUiIcon` keys (nested form), `icon` is required at runtime. */
39-
export interface CreateIconProps {
38+
/**
39+
* Props for {@link createIconBase} — nested icon definition(s). Used by generated icons and callers
40+
* that already structure data as `{ icon, rhUiIcon? }`.
41+
*/
42+
export interface CreateIconBaseProps {
4043
name?: string;
41-
icon?: IconDefinition;
44+
icon: IconDefinition;
4245
rhUiIcon?: IconDefinition | null;
4346
}
4447

4548
/**
46-
* @deprecated The previous `createIcon` accepted a flat {@link IconDefinition} with top-level
47-
* `svgPath`. Pass {@link CreateIconProps} with a nested `icon` field instead.
49+
* @deprecated Use {@link CreateIconBaseProps} instead.
50+
*/
51+
export type CreateIconProps = CreateIconBaseProps;
52+
53+
/**
54+
* Props for {@link createIcon} — flat {@link IconDefinition} fields at the top level, optionally with
55+
* `rhUiIcon`, matching the pre–nested-config API.
56+
*/
57+
export type CreateIconLegacyProps = IconDefinition & {
58+
rhUiIcon?: IconDefinition | null;
59+
};
60+
61+
/**
62+
* @deprecated The previous `createIcon` accepted only a flat {@link IconDefinition}. Use {@link createIcon}
63+
* for that shape, or {@link createIconBase} with nested `icon` / `rhUiIcon`.
4864
*/
4965
export type LegacyFlatIconDefinition = IconDefinition;
5066

@@ -83,44 +99,6 @@ function normalizeIconDefinition(icon: IconDefinition): IconDefinitionWithSvgPat
8399
};
84100
}
85101

86-
/** True when the argument uses the nested `CreateIconProps` shape (`icon` and/or `rhUiIcon` keys). */
87-
function isNestedCreateIconProps(arg: object): arg is CreateIconProps {
88-
return 'icon' in arg || 'rhUiIcon' in arg;
89-
}
90-
91-
/** Props after resolving legacy `svgPath` and flat `createIcon` arguments. */
92-
interface NormalizedCreateIconProps {
93-
name?: string;
94-
icon?: IconDefinitionWithSvgPathData;
95-
rhUiIcon: IconDefinitionWithSvgPathData | null;
96-
}
97-
98-
/**
99-
* Coerces legacy flat or nested props into normalized {@link NormalizedCreateIconProps}.
100-
* Nested input must include a non-null `icon` or throws.
101-
*/
102-
function normalizeCreateIconArg(arg: CreateIconProps | LegacyFlatIconDefinition): NormalizedCreateIconProps {
103-
if (isNestedCreateIconProps(arg)) {
104-
const p = arg as CreateIconProps;
105-
if (p.icon == null) {
106-
const label = p.name != null ? ` (name: ${String(p.name)})` : '';
107-
throw new Error(
108-
`@patternfly/react-icons: createIcon requires an \`icon\` definition when using nested CreateIconProps${label}.`
109-
);
110-
}
111-
return {
112-
name: p.name,
113-
icon: normalizeIconDefinition(p.icon),
114-
rhUiIcon: p.rhUiIcon != null ? normalizeIconDefinition(p.rhUiIcon) : null
115-
};
116-
}
117-
return {
118-
name: (arg as LegacyFlatIconDefinition).name,
119-
icon: normalizeIconDefinition(arg as IconDefinition),
120-
rhUiIcon: null
121-
};
122-
}
123-
124102
/** Renders an inner `<svg>` with viewBox and path(s) for the dual-SVG (CSS swap) layout. */
125103
const createSvg = (icon: IconDefinitionWithSvgPathData, iconClassName: string) => {
126104
const { xOffset, yOffset, width, height, svgPathData, svgClassName } = icon ?? {};
@@ -154,35 +132,26 @@ const createSvg = (icon: IconDefinitionWithSvgPathData, iconClassName: string) =
154132
};
155133

156134
/**
157-
* Builds a React **class** component that renders a PatternFly SVG icon (`role="img"`, optional `<title>` for a11y).
158-
*
159-
* **Argument shape — pick one:**
160-
*
161-
* 1. **`CreateIconProps` (preferred)** — `{ name?, icon?, rhUiIcon? }`. Dimensions and path data sit on `icon`
162-
* (and optionally on `rhUiIcon` for Red Hat UI–mapped icons). If the object **has an `icon` or `rhUiIcon` key**
163-
* (including `rhUiIcon: null`), this shape is assumed.
164-
*
165-
* 2. **Legacy flat `IconDefinition`** — the same fields as `icon`, but at the **top level** (no nested `icon`).
166-
* Still accepted so existing callers are not broken. Prefer migrating to `CreateIconProps`.
135+
* Preferred factory for **nested** icon config (`icon` and optional `rhUiIcon`). Package-generated icons use this.
167136
*
168-
* **Path data on each `IconDefinition`:** use `svgPathData` (string or {@link SVGPathObject}[]). The old name
169-
* `svgPath` is deprecated but still read; `svgPathData` wins if both are present.
170-
*
171-
* **Default vs RH UI rendering:** If `rhUiIcon` is set and the consumer does **not** pass `set` on the component,
172-
* the output is an outer `<svg.pf-v6-svg>` containing **two** inner `<svg>`s (default + rh-ui) so CSS can swap
173-
* which variant is visible. If `set` is `"default"` or `"rh-ui"`, a **single** flat `<svg>` is rendered for that
174-
* variant. Requesting `set="rh-ui"` when there is no `rhUiIcon` falls back to the default glyph and logs a
175-
* `console.warn` (see implementation).
176-
*
177-
* @param arg Icon configuration: either {@link CreateIconProps} (nested `icon` / `rhUiIcon`) or a legacy flat
178-
* {@link LegacyFlatIconDefinition}. Runtime detection follows the rules in **Argument shape** above.
179-
* @returns A `ComponentClass<SVGIconProps>` — render it as `<YourIcon />` or with `title`, `className`, `set`, etc.
137+
* @param name Optional display name for the component; falls back to `icon.name` when not set.
138+
* @see {@link createIcon} for the legacy **flat** argument shape.
180139
*/
181-
export function createIcon(arg: CreateIconProps | LegacyFlatIconDefinition): React.ComponentClass<SVGIconProps> {
182-
const { name, icon, rhUiIcon = null } = normalizeCreateIconArg(arg);
140+
export function createIconBase({
141+
name,
142+
icon,
143+
rhUiIcon = null
144+
}: CreateIconBaseProps): React.ComponentClass<SVGIconProps> {
145+
if (icon == null) {
146+
const label = name != null ? ` (name: ${String(name)})` : '';
147+
throw new Error(`@patternfly/react-icons: createIconBase requires an \`icon\` definition${label}.`);
148+
}
149+
const normalizedIcon = normalizeIconDefinition(icon);
150+
const normalizedRhUiIcon = rhUiIcon != null ? normalizeIconDefinition(rhUiIcon) : null;
151+
const displayName = name ?? icon.name;
183152

184153
return class SVGIcon extends Component<SVGIconProps> {
185-
static displayName = name;
154+
static displayName = displayName;
186155

187156
id = `icon-title-${currentId++}`;
188157

@@ -201,16 +170,16 @@ export function createIcon(arg: CreateIconProps | LegacyFlatIconDefinition): Rea
201170
classNames.push(propsClassName);
202171
}
203172

204-
if (set === 'rh-ui' && rhUiIcon === null) {
173+
if (set === 'rh-ui' && normalizedRhUiIcon === null) {
205174
// eslint-disable-next-line no-console
206175
console.warn(
207-
`Set "rh-ui" was provided for ${name}, but no rh-ui icon data exists for this icon. The default icon will be rendered.`
176+
`Set "rh-ui" was provided for ${displayName}, but no rh-ui icon data exists for this icon. The default icon will be rendered.`
208177
);
209178
}
210179

211-
if ((set === undefined && rhUiIcon === null) || set !== undefined) {
180+
if ((set === undefined && normalizedRhUiIcon === null) || set !== undefined) {
212181
const iconData: IconDefinitionWithSvgPathData | undefined =
213-
set !== undefined && set === 'rh-ui' && rhUiIcon !== null ? rhUiIcon : icon;
182+
set !== undefined && set === 'rh-ui' && normalizedRhUiIcon !== null ? normalizedRhUiIcon : normalizedIcon;
214183
const { xOffset, yOffset, width, height, svgPathData, svgClassName } =
215184
iconData ?? ({} as Partial<IconDefinitionWithSvgPathData>);
216185
const _xOffset = xOffset ?? 0;
@@ -259,11 +228,24 @@ export function createIcon(arg: CreateIconProps | LegacyFlatIconDefinition): Rea
259228
{...(props as Omit<React.SVGProps<SVGElement>, 'ref'>)} // Lie.
260229
>
261230
{hasTitle && <title id={this.id}>{title}</title>}
262-
{icon && createSvg(icon, 'pf-v6-icon-default')}
263-
{rhUiIcon && createSvg(rhUiIcon, 'pf-v6-icon-rh-ui')}
231+
{normalizedIcon && createSvg(normalizedIcon, 'pf-v6-icon-default')}
232+
{normalizedRhUiIcon && createSvg(normalizedRhUiIcon, 'pf-v6-icon-rh-ui')}
264233
</svg>
265234
);
266235
}
267236
}
268237
};
269238
}
239+
240+
/**
241+
* Legacy-friendly factory: **flat** {@link IconDefinition} fields (plus optional `rhUiIcon`) and delegates to
242+
* {@link createIconBase}. For nested configs, use {@link createIconBase} directly.
243+
*/
244+
export function createIcon(props: CreateIconLegacyProps): React.ComponentClass<SVGIconProps> {
245+
const { rhUiIcon, ...icon } = props;
246+
return createIconBase({
247+
name: icon.name,
248+
icon,
249+
rhUiIcon: rhUiIcon ?? null
250+
});
251+
}

0 commit comments

Comments
 (0)