fix(createIcon): Fix broken API in createIcon.tsx

tlabaj · tlabaj · commit c6580bea0037 · 2026-04-17T10:51:13.000-04:00
diff --git a/packages/react-icons/src/__tests__/createIcon.test.tsx b/packages/react-icons/src/__tests__/createIcon.test.tsx
@@ -1,5 +1,7 @@
+// eslint-disable-next-line no-restricted-imports -- test file excluded from package tsconfig; default import satisfies TS/JSX
+import React from 'react';
 import { render, screen } from '@testing-library/react';
-import { IconDefinition, CreateIconProps, createIcon, SVGPathObject } from '../createIcon';
+import { IconDefinition, CreateIconProps, createIcon, LegacyFlatIconDefinition, SVGPathObject } from '../createIcon';
 
 const multiPathIcon: IconDefinition = {
   name: 'IconName',
@@ -57,7 +59,37 @@ test('sets correct viewBox', () => {
 
 test('sets correct svgPath if string', () => {
   render(<SVGIcon />);
-  expect(screen.getByRole('img', { hidden: true }).querySelector('path')).toHaveAttribute('d', iconDef.svgPath);
+  expect(screen.getByRole('img', { hidden: true }).querySelector('path')).toHaveAttribute(
+    'd',
+    singlePathIcon.svgPathData
+  );
+});
+
+test('accepts legacy flat createIcon({ svgPath }) shape', () => {
+  const legacyDef: LegacyFlatIconDefinition = {
+    name: 'LegacyIcon',
+    width: 10,
+    height: 20,
+    svgPath: 'legacy-path',
+    svgClassName: 'legacy-svg'
+  };
+  const LegacySVGIcon = createIcon(legacyDef);
+  render(<LegacySVGIcon />);
+  expect(screen.getByRole('img', { hidden: true }).querySelector('path')).toHaveAttribute('d', 'legacy-path');
+});
+
+test('accepts CreateIconProps with nested icon using deprecated svgPath field', () => {
+  const nestedLegacyPath: CreateIconProps = {
+    name: 'NestedLegacyPathIcon',
+    icon: {
+      width: 8,
+      height: 8,
+      svgPath: 'nested-legacy-d'
+    }
+  };
+  const NestedIcon = createIcon(nestedLegacyPath);
+  render(<NestedIcon />);
+  expect(screen.getByRole('img', { hidden: true }).querySelector('path')).toHaveAttribute('d', 'nested-legacy-d');
 });
 
 test('sets correct svgClassName by default', () => {
@@ -127,3 +159,77 @@ test('additional props should be spread to the root svg element', () => {
   render(<SVGIcon data-testid="icon" />);
   expect(screen.getByTestId('icon')).toBeInTheDocument();
 });
+
+describe('rh-ui mapping: nested SVGs, set prop, and warnings', () => {
+  const defaultPath = 'M0 0-default';
+  const rhUiPath = 'M0 0-rh-ui';
+
+  const defaultIconDef: IconDefinition = {
+    name: 'DefaultVariant',
+    width: 16,
+    height: 16,
+    svgPathData: defaultPath
+  };
+
+  const rhUiIconDef: IconDefinition = {
+    name: 'RhUiVariant',
+    width: 16,
+    height: 16,
+    svgPathData: rhUiPath
+  };
+
+  const dualConfig: CreateIconProps = {
+    name: 'DualMappedIcon',
+    icon: defaultIconDef,
+    rhUiIcon: rhUiIconDef
+  };
+
+  const DualMappedIcon = createIcon(dualConfig);
+
+  test('renders two nested inner svgs when rhUiIcon is set and `set` is omitted (swap layout)', () => {
+    render(<DualMappedIcon />);
+    const root = screen.getByRole('img', { hidden: true });
+    expect(root).toHaveClass('pf-v6-svg');
+    const innerSvgs = root.querySelectorAll(':scope > svg');
+    expect(innerSvgs).toHaveLength(2);
+    expect(root?.querySelector('.pf-v6-icon-default path')).toHaveAttribute('d', defaultPath);
+    expect(root?.querySelector('.pf-v6-icon-rh-ui path')).toHaveAttribute('d', rhUiPath);
+  });
+
+  test('set="default" renders a single flat svg using the default icon paths', () => {
+    render(<DualMappedIcon set="default" />);
+    const root = screen.getByRole('img', { hidden: true });
+    expect(root.querySelectorAll(':scope > svg')).toHaveLength(0);
+    expect(root).toHaveAttribute('viewBox', '0 0 16 16');
+    expect(root.querySelector('path')).toHaveAttribute('d', defaultPath);
+    expect(root.querySelectorAll('svg')).toHaveLength(0);
+  });
+
+  test('set="rh-ui" renders a single flat svg using the rh-ui icon paths', () => {
+    render(<DualMappedIcon set="rh-ui" />);
+    const root = screen.getByRole('img', { hidden: true });
+    expect(root.querySelectorAll(':scope > svg')).toHaveLength(0);
+    expect(root.querySelector('path')).toHaveAttribute('d', rhUiPath);
+    expect(root.querySelectorAll('svg')).toHaveLength(0);
+  });
+
+  test('set="rh-ui" with no rhUiIcon mapping falls back to default and warns', () => {
+    const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {});
+    const IconNoRhMapping = createIcon({
+      name: 'NoRhMappingIcon',
+      icon: defaultIconDef,
+      rhUiIcon: null
+    });
+
+    render(<IconNoRhMapping set="rh-ui" />);
+
+    expect(warnSpy).toHaveBeenCalledWith(
+      'Set "rh-ui" was provided for NoRhMappingIcon, but no rh-ui icon data exists for this icon. The default icon will be rendered.'
+    );
+    const root = screen.getByRole('img', { hidden: true });
+    expect(root.querySelector('path')).toHaveAttribute('d', defaultPath);
+    expect(root.querySelectorAll('svg')).toHaveLength(0);
+
+    warnSpy.mockRestore();
+  });
+});
diff --git a/packages/react-icons/src/createIcon.tsx b/packages/react-icons/src/createIcon.tsx
@@ -5,22 +5,49 @@ export interface SVGPathObject {
   className?: string;
 }
 
-export interface IconDefinition {
+export interface IconDefinitionBase {
   name?: string;
   width: number;
   height: number;
-  svgPathData: string | SVGPathObject[];
   xOffset?: number;
   yOffset?: number;
   svgClassName?: string;
 }
 
+/**
+ * SVG path content for one icon variant (default or rh-ui). At runtime at least one of
+ * `svgPathData` or `svgPath` must be set; if both are present, `svgPathData` is used.
+ */
+export interface IconDefinition extends IconDefinitionBase {
+  svgPathData?: string | SVGPathObject[];
+  /**
+   * @deprecated Use {@link IconDefinition.svgPathData} instead.
+   */
+  svgPath?: string | SVGPathObject[];
+}
+
+/** Narrows {@link IconDefinition} to the preferred shape with required `svgPathData`. */
+export type IconDefinitionWithSvgPathData = Required<Pick<IconDefinition, 'svgPathData'>> & IconDefinition;
+
+/**
+ * @deprecated Use {@link IconDefinition} with `svgPathData` instead.
+ * Narrows {@link IconDefinition} to the legacy shape with required `svgPath`.
+ */
+export type IconDefinitionWithSvgPath = Required<Pick<IconDefinition, 'svgPath'>> & IconDefinition;
+
+/** When passing `icon` or `rhUiIcon` keys (nested form), `icon` is required at runtime. */
 export interface CreateIconProps {
   name?: string;
   icon?: IconDefinition;
   rhUiIcon?: IconDefinition | null;
 }
 
+/**
+ * @deprecated The previous `createIcon` accepted a flat {@link IconDefinition} with top-level
+ * `svgPath`. Pass {@link CreateIconProps} with a nested `icon` field instead.
+ */
+export type LegacyFlatIconDefinition = IconDefinition;
+
 export interface SVGIconProps extends Omit<React.HTMLProps<SVGElement>, 'ref'> {
   title?: string;
   className?: string;
@@ -32,7 +59,70 @@ export interface SVGIconProps extends Omit<React.HTMLProps<SVGElement>, 'ref'> {
 
 let currentId = 0;
 
-const createSvg = (icon: IconDefinition, iconClassName: string) => {
+/** Returns path data from `svgPathData` or deprecated `svgPath` (prefers `svgPathData` when both exist). */
+function resolveSvgPathData(icon: IconDefinition): string | SVGPathObject[] {
+  if ('svgPathData' in icon && icon.svgPathData !== undefined) {
+    return icon.svgPathData;
+  }
+  if ('svgPath' in icon && icon.svgPath !== undefined) {
+    return icon.svgPath;
+  }
+  throw new Error('@patternfly/react-icons: IconDefinition must define svgPathData or svgPath');
+}
+
+/** Produces a single {@link IconDefinitionWithSvgPathData} for internal rendering. */
+function normalizeIconDefinition(icon: IconDefinition): IconDefinitionWithSvgPathData {
+  return {
+    name: icon.name,
+    width: icon.width,
+    height: icon.height,
+    svgPathData: resolveSvgPathData(icon),
+    xOffset: icon.xOffset,
+    yOffset: icon.yOffset,
+    svgClassName: icon.svgClassName
+  };
+}
+
+/** True when the argument uses the nested `CreateIconProps` shape (`icon` and/or `rhUiIcon` keys). */
+function isNestedCreateIconProps(arg: object): arg is CreateIconProps {
+  return 'icon' in arg || 'rhUiIcon' in arg;
+}
+
+/** Props after resolving legacy `svgPath` and flat `createIcon` arguments. */
+interface NormalizedCreateIconProps {
+  name?: string;
+  icon?: IconDefinitionWithSvgPathData;
+  rhUiIcon: IconDefinitionWithSvgPathData | null;
+}
+
+/**
+ * Coerces legacy flat or nested props into normalized {@link NormalizedCreateIconProps}.
+ * Nested input must include a non-null `icon` or throws.
+ */
+function normalizeCreateIconArg(arg: CreateIconProps | LegacyFlatIconDefinition): NormalizedCreateIconProps {
+  if (isNestedCreateIconProps(arg)) {
+    const p = arg as CreateIconProps;
+    if (p.icon == null) {
+      const label = p.name != null ? ` (name: ${String(p.name)})` : '';
+      throw new Error(
+        `@patternfly/react-icons: createIcon requires an \`icon\` definition when using nested CreateIconProps${label}.`
+      );
+    }
+    return {
+      name: p.name,
+      icon: normalizeIconDefinition(p.icon),
+      rhUiIcon: p.rhUiIcon != null ? normalizeIconDefinition(p.rhUiIcon) : null
+    };
+  }
+  return {
+    name: (arg as LegacyFlatIconDefinition).name,
+    icon: normalizeIconDefinition(arg as IconDefinition),
+    rhUiIcon: null
+  };
+}
+
+/** Renders an inner `<svg>` with viewBox and path(s) for the dual-SVG (CSS swap) layout. */
+const createSvg = (icon: IconDefinitionWithSvgPathData, iconClassName: string) => {
   const { xOffset, yOffset, width, height, svgPathData, svgClassName } = icon ?? {};
   const _xOffset = xOffset ?? 0;
   const _yOffset = yOffset ?? 0;
@@ -64,9 +154,33 @@ const createSvg = (icon: IconDefinition, iconClassName: string) => {
 };
 
 /**
- * Factory to create Icon class components for consumers
+ * Builds a React **class** component that renders a PatternFly SVG icon (`role="img"`, optional `<title>` for a11y).
+ *
+ * **Argument shape — pick one:**
+ *
+ * 1. **`CreateIconProps` (preferred)** — `{ name?, icon?, rhUiIcon? }`. Dimensions and path data sit on `icon`
+ *    (and optionally on `rhUiIcon` for Red Hat UI–mapped icons). If the object **has an `icon` or `rhUiIcon` key**
+ *    (including `rhUiIcon: null`), this shape is assumed.
+ *
+ * 2. **Legacy flat `IconDefinition`** — the same fields as `icon`, but at the **top level** (no nested `icon`).
+ *    Still accepted so existing callers are not broken. Prefer migrating to `CreateIconProps`.
+ *
+ * **Path data on each `IconDefinition`:** use `svgPathData` (string or {@link SVGPathObject}[]). The old name
+ * `svgPath` is deprecated but still read; `svgPathData` wins if both are present.
+ *
+ * **Default vs RH UI rendering:** If `rhUiIcon` is set and the consumer does **not** pass `set` on the component,
+ *    the output is an outer `<svg.pf-v6-svg>` containing **two** inner `<svg>`s (default + rh-ui) so CSS can swap
+ *    which variant is visible. If `set` is `"default"` or `"rh-ui"`, a **single** flat `<svg>` is rendered for that
+ *    variant. Requesting `set="rh-ui"` when there is no `rhUiIcon` falls back to the default glyph and logs a
+ *    `console.warn` (see implementation).
+ *
+ * @param arg Icon configuration: either {@link CreateIconProps} (nested `icon` / `rhUiIcon`) or a legacy flat
+ *    {@link LegacyFlatIconDefinition}. Runtime detection follows the rules in **Argument shape** above.
+ * @returns A `ComponentClass<SVGIconProps>` — render it as `<YourIcon />` or with `title`, `className`, `set`, etc.
  */
-export function createIcon({ name, icon, rhUiIcon = null }: CreateIconProps): React.ComponentClass<SVGIconProps> {
+export function createIcon(arg: CreateIconProps | LegacyFlatIconDefinition): React.ComponentClass<SVGIconProps> {
+  const { name, icon, rhUiIcon = null } = normalizeCreateIconArg(arg);
+
   return class SVGIcon extends Component<SVGIconProps> {
     static displayName = name;
 
@@ -76,10 +190,7 @@ export function createIcon({ name, icon, rhUiIcon = null }: CreateIconProps): Re
       noDefaultStyle: false
     };
 
-    constructor(props: SVGIconProps) {
-      super(props);
-    }
-
+    /** Renders one root `<svg>`; either a single variant or nested inner SVGs for RH UI swap. */
     render() {
       const { title, className: propsClassName, set, noDefaultStyle, ...props } = this.props;
 
@@ -98,8 +209,10 @@ export function createIcon({ name, icon, rhUiIcon = null }: CreateIconProps): Re
       }
 
       if ((set === undefined && rhUiIcon === null) || set !== undefined) {
-        const iconData = set !== undefined && set === 'rh-ui' && rhUiIcon !== null ? rhUiIcon : icon;
-        const { xOffset, yOffset, width, height, svgPathData, svgClassName } = iconData ?? {};
+        const iconData: IconDefinitionWithSvgPathData | undefined =
+          set !== undefined && set === 'rh-ui' && rhUiIcon !== null ? rhUiIcon : icon;
+        const { xOffset, yOffset, width, height, svgPathData, svgClassName } =
+          iconData ?? ({} as Partial<IconDefinitionWithSvgPathData>);
         const _xOffset = xOffset ?? 0;
         const _yOffset = yOffset ?? 0;
         const viewBox = [_xOffset, _yOffset, width, height].join(' ');
