Skip to content

feat(icons): investigating pf-rh icon hotswapping #12210

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kmcfaul wants to merge 2 commits into
base: main
Choose a base branch
from
+496 −59
Open

feat(icons): investigating pf-rh icon hotswapping #12210

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
faf45e4
feat(icons): investigating pf-rh icon hotswapping
kmcfaul Jan 16, 2026
c85b84e
update createIcon tests for new structure
kmcfaul Jan 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 15 additions & 25 deletions packages/react-icons/scripts/writeIcons.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { outputFileSync, ensureDirSync } from 'fs-extra/esm';
import { generateIcons } from './generateIcons.mjs';
import { createElement } from 'react';
import { renderToString } from 'react-dom/server';

import { pfToRhIcons } from '../src/pfToRhIcons.js';
import * as url from 'url';
const __dirname = url.fileURLToPath(new URL('.', import.meta.url));

Expand All @@ -17,39 +17,31 @@ const staticDir = join(outDir, 'static');
const removeSnake = (s) => s.toUpperCase().replace('-', '').replace('_', '');
const toCamel = (s) => `${s[0].toUpperCase()}${s.substr(1).replace(/([-_][\w])/gi, removeSnake)}`;

const writeCJSExport = (fname, jsName, icon) => {
const writeCJSExport = (fname, jsName, icon, rhUiIcon = null) => {
outputFileSync(
join(outDir, 'js/icons', `${fname}.js`),
`"use strict"
exports.__esModule = true;
exports.${jsName}Config = {
name: '${jsName}',
height: ${icon.height},
width: ${icon.width},
svgPath: ${JSON.stringify(icon.svgPathData)},
yOffset: ${icon.yOffset || 0},
xOffset: ${icon.xOffset || 0},
svgClassName: ${JSON.stringify(icon.svgClassName)},
icon: ${JSON.stringify(icon)},
rhUiIcon: ${rhUiIcon ? JSON.stringify(rhUiIcon) : 'null'},
};
exports.${jsName} = require('../createIcon').createIcon(exports.${jsName}Config);
exports["default"] = exports.${jsName};
`.trim()
);
};

const writeESMExport = (fname, jsName, icon) => {
const writeESMExport = (fname, jsName, icon, rhUiIcon = null) => {
outputFileSync(
join(outDir, 'esm/icons', `${fname}.js`),
`import { createIcon } from '../createIcon';

export const ${jsName}Config = {
name: '${jsName}',
height: ${icon.height},
width: ${icon.width},
svgPath: ${JSON.stringify(icon.svgPathData)},
yOffset: ${icon.yOffset || 0},
xOffset: ${icon.xOffset || 0},
svgClassName: ${JSON.stringify(icon.svgClassName)},
icon: ${JSON.stringify(icon)},
rhUiIcon: ${rhUiIcon ? JSON.stringify(rhUiIcon) : 'null'},
};

export const ${jsName} = createIcon(${jsName}Config);
Expand All @@ -59,17 +51,13 @@ export default ${jsName};
);
};

const writeDTSExport = (fname, jsName, icon) => {
const writeDTSExport = (fname, jsName, icon, rhUiIcon = null) => {
const text = `import { ComponentClass } from 'react';
import { SVGIconProps } from '../createIcon';
export declare const ${jsName}Config: {
name: '${jsName}',
height: ${icon.height},
width: ${icon.width},
svgPath: ${JSON.stringify(icon.svgPathData)},
yOffset: ${icon.yOffset || 0},
xOffset: ${icon.xOffset || 0},
svgClassName: ${JSON.stringify(icon.svgClassName)},
icon: ${JSON.stringify(icon)},
rhUiIcon: ${rhUiIcon ? JSON.stringify(rhUiIcon) : 'null'},
};
export declare const ${jsName}: ComponentClass<SVGIconProps>;
export default ${jsName};
Expand Down Expand Up @@ -133,9 +121,11 @@ function writeIcons(icons) {
Object.entries(icons).forEach(([iconName, icon]) => {
const fname = `${iconName}-icon`;
const jsName = `${toCamel(iconName)}Icon`;
writeESMExport(fname, jsName, icon);
writeCJSExport(fname, jsName, icon);
writeDTSExport(fname, jsName, icon);

const altIcon = pfToRhIcons[jsName] ? pfToRhIcons[jsName].icon : null;
writeESMExport(fname, jsName, icon, altIcon);
writeCJSExport(fname, jsName, icon, altIcon);
writeDTSExport(fname, jsName, icon, altIcon);

index.push({ fname, jsName });
});
Expand Down
39 changes: 25 additions & 14 deletions packages/react-icons/src/__tests__/createIcon.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,32 +1,43 @@
import { render, screen } from '@testing-library/react';
import { createIcon } from '../createIcon';
import { IconDefinition, CreateIconProps, createIcon, SVGPathObject } from '../createIcon';

const iconDef = {
const multiPathIcon: IconDefinition = {
name: 'IconName',
width: 10,
height: 20,
svgPath: 'svgPath'
svgPathData: [
{ path: 'svgPath1', className: 'class1' },
{ path: 'svgPath2', className: 'class2' }
],
svgClassName: 'test'
};

const iconDefWithArrayPath = {
const singlePathIcon: IconDefinition = {
name: 'IconName',
width: 10,
height: 20,
svgPath: [
{ path: 'svgPath1', className: 'class1' },
{ path: 'svgPath2', className: 'class2' }
],
svgPathData: 'svgPath',
svgClassName: 'test'
};

const iconDef: CreateIconProps = {
name: 'SinglePathIconName',
icon: singlePathIcon
};

const iconDefWithArrayPath: CreateIconProps = {
name: 'MultiPathIconName',
icon: multiPathIcon
};

const SVGIcon = createIcon(iconDef);
const SVGArrayIcon = createIcon(iconDefWithArrayPath);

test('sets correct viewBox', () => {
render(<SVGIcon />);
expect(screen.getByRole('img', { hidden: true })).toHaveAttribute(
'viewBox',
`0 0 ${iconDef.width} ${iconDef.height}`
`0 0 ${singlePathIcon.width} ${singlePathIcon.height}`
);
});

Expand All @@ -39,16 +50,16 @@ test('sets correct svgPath if array', () => {
render(<SVGArrayIcon />);
const paths = screen.getByRole('img', { hidden: true }).querySelectorAll('path');
expect(paths).toHaveLength(2);
expect(paths[0]).toHaveAttribute('d', iconDefWithArrayPath.svgPath[0].path);
expect(paths[1]).toHaveAttribute('d', iconDefWithArrayPath.svgPath[1].path);
expect(paths[0]).toHaveClass(iconDefWithArrayPath.svgPath[0].className);
expect(paths[1]).toHaveClass(iconDefWithArrayPath.svgPath[1].className);
expect(paths[0]).toHaveAttribute('d', (multiPathIcon.svgPathData as SVGPathObject[])[0].path);
expect(paths[1]).toHaveAttribute('d', (multiPathIcon.svgPathData as SVGPathObject[])[1].path);
expect(paths[0]).toHaveClass((multiPathIcon.svgPathData as SVGPathObject[])[0].className ?? '');
expect(paths[1]).toHaveClass((multiPathIcon.svgPathData as SVGPathObject[])[1].className ?? '');
});

test('sets correct svgClassName', () => {
render(<SVGArrayIcon />);
const paths = screen.getByRole('img', { hidden: true });
expect(paths).toHaveClass(iconDefWithArrayPath.svgClassName);
expect(paths).toHaveClass(multiPathIcon.svgClassName ?? '');
});

test('aria-hidden is true if no title is specified', () => {
Expand Down
160 changes: 140 additions & 20 deletions packages/react-icons/src/createIcon.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,45 +4,96 @@ export interface SVGPathObject {
path: string;
className?: string;
}

export interface IconDefinition {
name?: string;
width: number;
height: number;
svgPath: string | SVGPathObject[];
svgPathData: string | SVGPathObject[];
xOffset?: number;
yOffset?: number;
svgClassName?: string;
}

export interface CreateIconProps {
name?: string;
icon?: IconDefinition;
rhUiIcon?: IconDefinition | null;
}

export interface SVGIconProps extends Omit<React.HTMLProps<SVGElement>, 'ref'> {
title?: string;
className?: string;
/* Indicates the icon should render using alternate svg data for unified theme */
set?: 'default' | 'unified';
}

let currentId = 0;
const canUseDOM = !!(typeof window !== 'undefined' && window.document && window.document.createElement);

/**
* Factory to create Icon class components for consumers
*/
export function createIcon({
name,
xOffset = 0,
yOffset = 0,
width,
height,
svgPath,
svgClassName
}: IconDefinition): React.ComponentClass<SVGIconProps> {
return class SVGIcon extends Component<SVGIconProps> {
export function createIcon({ name, icon, rhUiIcon = null }: CreateIconProps): React.ComponentClass<SVGIconProps> {
return class SVGIcon extends Component<SVGIconProps, { themeClassVersion: number }> {
static displayName = name;

id = `icon-title-${currentId++}`;
private observer: MutationObserver | null = null;

constructor(props: SVGIconProps) {
super(props);
this.state = { themeClassVersion: 0 };
}

componentDidMount() {
if (rhUiIcon !== null && canUseDOM) {
this.observer = new MutationObserver((mutations) => {
for (const mutation of mutations) {
if (mutation.type === 'attributes' && mutation.attributeName === 'class') {
const target = mutation.target as HTMLElement;
const hadClass = (mutation.oldValue || '').includes('pf-v6-theme-unified');
const hasClass = target.classList.contains('pf-v6-theme-unified');

if (hadClass !== hasClass && this.props.set === undefined) {
this.setState((prevState) => ({
themeClassVersion: prevState.themeClassVersion + 1
}));
}
}
}
});

this.observer.observe(document.documentElement, {
attributes: true,
attributeFilter: ['class'],
attributeOldValue: true
});
}
Comment on lines +49 to +72
Copy link

@coderabbitai coderabbitai bot Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

MutationObserver per icon instance may degrade performance at scale.

Each icon instance creates its own MutationObserver watching document.documentElement. With hundreds of icons on a page, this creates hundreds of observers that all fire on every class change, each triggering a setState and potential re-render.

Consider a shared singleton observer pattern:

Singleton observer approach 
// Module-level singleton
let themeObserver: MutationObserver | null = null;
const listeners = new Set<() => void>();

function subscribeToThemeChanges(callback: () => void): () => void {
  if (canUseDOM && !themeObserver) {
    themeObserver = new MutationObserver((mutations) => {
      for (const mutation of mutations) {
        if (mutation.type === 'attributes' && mutation.attributeName === 'class') {
          const hadClass = (mutation.oldValue || '').includes('pf-v6-theme-unified');
          const hasClass = document.documentElement.classList.contains('pf-v6-theme-unified');
          if (hadClass !== hasClass) {
            listeners.forEach((cb) => cb());
          }
        }
      }
    });
    themeObserver.observe(document.documentElement, {
      attributes: true,
      attributeFilter: ['class'],
      attributeOldValue: true
    });
  }
  listeners.add(callback);
  return () => listeners.delete(callback);
}

Then in the component, subscribe and unsubscribe via componentDidMount/componentWillUnmount.

🤖 Prompt for AI Agents 
In `@packages/react-icons/src/createIcon.tsx` around lines 49 - 72, Multiple
MutationObserver instances are created per icon in componentDidMount, which
scales poorly; replace the per-instance observer with a module-level singleton
observer (themeObserver) and a subscribeToThemeChanges(callback) API that
maintains a Set listeners; in componentDidMount subscribe a callback that checks
rhUiIcon/canUseDOM and the this.props.set condition and calls this.setState(prev
=> ({ themeClassVersion: prev.themeClassVersion + 1 })), and in
componentWillUnmount unsubscribe the callback to remove it from listeners so the
singleton observer only notifies registered icons and cleans up when none
remain.

}

componentWillUnmount() {
if (this.observer) {
this.observer.disconnect();
this.observer = null;
}
}

render() {
const { title, className: propsClassName, ...props } = this.props;
const { title, className: propsClassName, set, ...props } = this.props;

const shouldUseAltData =
rhUiIcon !== null &&
(set === 'unified' ||
(set === undefined && canUseDOM && document.documentElement.classList.contains('pf-v6-theme-unified')));

const iconData = shouldUseAltData ? rhUiIcon : icon;
const { xOffset, yOffset, width, height, svgClassName, svgPathData } = iconData ?? {};
const _xOffset = xOffset ?? 0;
const _yOffset = yOffset ?? 0;

const hasTitle = Boolean(title);
const viewBox = [xOffset, yOffset, width, height].join(' ');
const viewBox = [_xOffset, _yOffset, width, height].join(' ');

const classNames = ['pf-v6-svg'];
if (svgClassName) {
Expand All @@ -52,6 +103,14 @@ export function createIcon({
classNames.push(propsClassName);
}

const svgPaths = Array.isArray(svgPathData) ? (
svgPathData.map((pathObject, index) => (
<path className={pathObject.className} key={`${pathObject.path}-${index}`} d={pathObject.path} />
))
) : (
<path d={svgPathData as string} />
);
Comment on lines +106 to +112
Copy link

@coderabbitai coderabbitai bot Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Handle undefined svgPathData gracefully.

If iconData.svgPathData is undefined, the ternary falls through to render <path d={undefined} />, which produces invalid SVG markup.

Add fallback 
-      const svgPaths = Array.isArray(svgPathData) ? (
+      const svgPaths = !svgPathData ? null : Array.isArray(svgPathData) ? (
         svgPathData.map((pathObject, index) => (
           <path className={pathObject.className} key={`${pathObject.path}-${index}`} d={pathObject.path} />
         ))
       ) : (
         <path d={svgPathData as string} />
       );
🤖 Prompt for AI Agents 
In `@packages/react-icons/src/createIcon.tsx` around lines 106 - 112, The code in
createIcon.tsx can render <path d={undefined}> when svgPathData is undefined;
update the svgPaths construction to first check for svgPathData being
null/undefined and return null (or an empty fragment) in that case, otherwise
keep the existing Array.isArray(svgPathData) branch (mapping
pathObject.className and pathObject.path with stable keys) and the string
branch; ensure the check references the svgPathData variable so no invalid <path
d={undefined}> is emitted.


return (
<svg
className={classNames.join(' ')}
Expand All @@ -65,15 +124,76 @@ export function createIcon({
{...(props as Omit<React.SVGProps<SVGElement>, 'ref'>)} // Lie.
>
{hasTitle && <title id={this.id}>{title}</title>}
{Array.isArray(svgPath) ? (
svgPath.map((pathObject, index) => (
<path className={pathObject.className} key={`${pathObject.path}-${index}`} d={pathObject.path} />
))
) : (
<path d={svgPath} />
)}
{svgPaths}
</svg>
);

// Alternate CSS method tinkering
// TODO: remove or refactor to use this method
// Below works for paths, but not viewbox without needing the MutationObserver which would be nice to not have when using CSS method
// May be able to use two separate svgs instead of paths instead if going this route

// const defaultSvgPathData = Array.isArray(icon?.svgPathData) ? (
// icon?.svgPathData.map((pathObject, index) => (
// <path className={pathObject.className} key={`${pathObject.path}-${index}`} d={pathObject.path} />
// ))
// ) : (
// <path d={icon?.svgPathData as string} />
// );

// const rhUiSvgPathData =
// rhUiIcon?.svgPathData && Array.isArray(rhUiIcon?.svgPathData) ? (
// rhUiIcon?.svgPathData.map((pathObject, index) => (
// <path className={pathObject.className} key={`${pathObject.path}-${index}`} d={pathObject.path} />
// ))
// ) : (
// <path d={rhUiIcon?.svgPathData as string} />
// );

// const finalSvgPath =
// rhUiIcon !== null ? (
// <>
// <g className="pf-icon">{defaultSvgPathData}</g>
// <g className="rh-icon">{rhUiSvgPathData}</g>
// </>
// ) : (
// pfSvgPathData
// );

// return (
// <svg
// className={classNames.join(' ')}
// viewBox={viewBox}
// fill="currentColor"
// aria-labelledby={hasTitle ? this.id : null}
// aria-hidden={hasTitle ? null : true}
// role="img"
// width="1em"
// height="1em"
// {...(props as Omit<React.SVGProps<SVGElement>, 'ref'>)} // Lie.
// >
// {hasTitle && <title id={this.id}>{title}</title>}
// <style type="text/css">
// {/* Testing CSS visibility switching */}
// {/* TODO: move to css file, add to global styles */}
// {`
// .pf-v6-theme-unified {
// .pf-icon {
// display: none;
// }
// .rh-icon {
// display: block;
// }
// }
// .rh-icon {
// display: none;
// }

// `}
// </style>
// {finalSvgPath}
// </svg>
// );
}
};
}
Loading
Loading