Skip to content

Commit 93a5207

Browse files
HotellHotell
authored
fix(react-icons): replace dangerouslySetInnerHTML with structured SvgNode rendering for color icons (#1078)
1 parent 97e9d16 commit 93a5207

10 files changed

Lines changed: 392 additions & 45 deletions

‎packages/react-icons/build-color-render.test.js‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
// @ts-check
2+
/**
3+
* @vitest-environment jsdom
4+
*/
5+
6+
/**
7+
* Integration test: renders every color icon variant from the built output
8+
* to verify that the SvgNode[] → React.createElement pipeline produces
9+
* valid, renderable SVG without dangerouslySetInnerHTML.
10+
*
11+
* This test runs as part of build-verify (after `nx run react-icons:build`).
12+
*
13+
* Strategy: Load all atom modules from lib/atoms/svg/ in parallel, then filter
14+
* for Color exports and render each one.
15+
*/
16+
17+
import { describe, it, expect } from 'vitest';
18+
import { render } from '@testing-library/react';
19+
import { createElement } from 'react';
20+
import { readdir } from 'node:fs/promises';
21+
import path from 'node:path';
22+
import { fileURLToPath } from 'node:url';
23+
24+
const __dirname = path.dirname(fileURLToPath(import.meta.url));
25+
const atomsDir = path.join(__dirname, 'lib', 'atoms', 'svg');
26+
27+
/**
28+
* Load all atom modules in parallel and collect Color icon exports.
29+
* @returns {Promise<Array<{ name: string; file: string; Component: any }>>}
30+
*/
31+
async function loadColorIcons() {
32+
const files = await readdir(atomsDir);
33+
const jsFiles = files.filter((f) => f.endsWith('.js'));
34+
35+
const modules = await Promise.all(
36+
jsFiles.map((file) => import(path.join(atomsDir, file)).then((mod) => ({ file, mod }))),
37+
);
38+
39+
/** @type {Array<{ name: string; file: string; Component: any }>} */
40+
const colorIcons = [];
41+
42+
for (const { file, mod } of modules) {
43+
for (const [name, Component] of Object.entries(mod)) {
44+
if (!name.includes('Color')) continue;
45+
if (typeof Component !== 'function' && typeof (/** @type {any} */ (Component)?.render) !== 'function') continue;
46+
colorIcons.push({ name, file, Component });
47+
}
48+
}
49+
50+
return colorIcons;
51+
}
52+
53+
describe('Color Icon Rendering', () => {
54+
it('all color icon exports from lib/atoms/svg should render valid SVG', async () => {
55+
const colorIcons = await loadColorIcons();
56+
57+
expect(colorIcons.length).toMatchInlineSnapshot(`1165`);
58+
59+
/** @type {Array<{ name: string; file: string; error: unknown }>} */
60+
const failures = [];
61+
62+
for (const { name, file, Component } of colorIcons) {
63+
try {
64+
const { container } = render(createElement(Component));
65+
66+
const svg = container.querySelector('svg');
67+
expect(svg, `${name}: should render an <svg> element`).toBeTruthy();
68+
69+
// Color icons must have child elements (from SvgNode[] rendering, not dangerouslySetInnerHTML)
70+
expect(svg?.children.length, `${name}: should have child elements inside <svg>`).toBeGreaterThan(0);
71+
72+
// Should have standard SVG attributes
73+
expect(svg, `${name}: should have xmlns`).toHaveAttribute('xmlns', 'http://www.w3.org/2000/svg');
74+
expect(svg, `${name}: should have viewBox`).toHaveAttribute('viewBox');
75+
} catch (error) {
76+
failures.push({ name, file, error });
77+
}
78+
}
79+
80+
// Report all failures at once for easier debugging
81+
if (failures.length > 0) {
82+
const summary = failures
83+
.map((f) => ` ${f.name} (${f.file}): ${f.error instanceof Error ? f.error.message : String(f.error)}`)
84+
.join('\n');
85+
throw new Error(`${failures.length} out of ${colorIcons.length} color icons failed to render:\n${summary}`);
86+
}
87+
}, 10_000);
88+
});

‎packages/react-icons/build-verify.test.js‎

Lines changed: 2 additions & 2 deletions
Large diffs are not rendered by default.

‎packages/react-icons/package.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,10 @@
3030
"build:js": "node scripts/build.js",
3131
"prebuild": "npm run clean",
3232
"build": "npm run build:fonts-and-svg && npm run build:generate-chunks-and-atoms && npm run build:js",
33-
"build-verify": "vitest run build-verify.test.js build-transforms.test.js",
33+
"build-verify": "vitest run build-verify.test.js build-transforms.test.js build-color-render.test.js",
3434
"bundle-size": "monosize measure",
3535
"lint": "eslint src",
36-
"test": "vitest --exclude build-verify.test.js --exclude build-transforms.test.js",
36+
"test": "vitest --exclude build-verify.test.js --exclude build-transforms.test.js --exclude build-color-render.test.js",
3737
"type-check:infra": "../../node_modules/.bin/tsc -p tsconfig.utils.json"
3838
},
3939
"devDependencies": {

‎packages/react-icons/scripts/convert.utils.js‎

Lines changed: 98 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,97 @@ const { writeSpriteFiles } = require('./sprite.writer');
1111

1212
/** @typedef {{ [key: string]: 'mirror' | 'unique' }} RtlMetadata */
1313

14+
/**
15+
* Convert a hyphenated SVG attribute name to React-compatible camelCase.
16+
* e.g. 'stop-color' → 'stopColor', 'fill-opacity' → 'fillOpacity'
17+
*
18+
* @param {string} attr
19+
* @returns {string}
20+
*/
21+
function svgAttrToReactProp(attr) {
22+
return attr.replace(/-([a-z])/g, (_, c) => c.toUpperCase());
23+
}
24+
25+
/**
26+
* Parse a CSS style string into a React-compatible style object.
27+
*
28+
* React requires `style` to be an object (e.g. `{ maskType: "alpha" }`),
29+
* not a CSS string (e.g. `"mask-type:alpha"`). Passing a string throws:
30+
* "The `style` prop expects a mapping from style properties to values, not a string."
31+
*
32+
* @param {string} cssText - inline CSS text (e.g. "mask-type:alpha;mix-blend-mode:multiply")
33+
* @returns {Record<string, string>}
34+
*/
35+
function parseCssStyleToObject(cssText) {
36+
/** @type {Record<string, string>} */
37+
const styleObj = {};
38+
for (const decl of cssText.split(';')) {
39+
const colon = decl.indexOf(':');
40+
if (colon === -1) continue;
41+
const prop = svgAttrToReactProp(decl.slice(0, colon).trim());
42+
styleObj[prop] = decl.slice(colon + 1).trim();
43+
}
44+
return styleObj;
45+
}
46+
47+
/**
48+
* @typedef {[tag: string, attrs: Record<string, string | Record<string, string>> | null, ...children: SvgNode[]]} SvgNode
49+
*/
50+
51+
/**
52+
* Parse an SVG inner-HTML string into a structured SvgNode[] tree.
53+
* Converts hyphenated SVG attribute names to React-compatible camelCase.
54+
*
55+
* Uses a single regex to tokenize tags and a stack to track nesting.
56+
* Icon SVGs contain no text/CDATA/comments — only elements with attributes.
57+
*
58+
* @param {string} svgString - inner SVG content (without outer `<svg>` tag)
59+
* @returns {SvgNode[]}
60+
*/
61+
function parseSvgToNodes(svgString) {
62+
/** @type {SvgNode[]} */
63+
const root = [];
64+
/** @type {SvgNode[][]} */
65+
const stack = [root];
66+
const tagRe = /<(\/?)(\w+)((?:\s+[\w:-]+="[^"]*")*)\s*(\/?)>/g;
67+
const attrRe = /([\w:-]+)="([^"]*)"/g;
68+
69+
let tagMatch;
70+
while ((tagMatch = tagRe.exec(svgString)) !== null) {
71+
const [, isClose, tag, attrStr, isSelfClose] = tagMatch;
72+
73+
if (isClose) {
74+
stack.pop();
75+
continue;
76+
}
77+
78+
/** @type {Record<string, string | Record<string, string>>} */
79+
const attrs = {};
80+
let attrMatch;
81+
attrRe.lastIndex = 0;
82+
while ((attrMatch = attrRe.exec(attrStr)) !== null) {
83+
const name = svgAttrToReactProp(attrMatch[1]);
84+
if (name === 'style') {
85+
attrs[name] = parseCssStyleToObject(attrMatch[2]);
86+
} else {
87+
attrs[name] = attrMatch[2];
88+
}
89+
}
90+
91+
/** @type {SvgNode} */
92+
const node = [tag, Object.keys(attrs).length > 0 ? attrs : null];
93+
stack[stack.length - 1].push(node);
94+
95+
if (!isSelfClose) {
96+
// For non-self-closing tags, push node itself as the new container.
97+
// Children will be appended starting at index 2 (after tag and attrs).
98+
stack.push(/** @type {any} */ (node));
99+
}
100+
}
101+
102+
return root;
103+
}
104+
14105
/**
15106
* Returns the standard header lines used in generated icon files.
16107
* @param {string} relImport - relative import path to createFluentIcon
@@ -28,7 +119,7 @@ function getCreateFluentIconHeader(relImport) {
28119
* @typedef {Object} ParsedIconSource
29120
* @property {string} exportName - PascalCase export name (e.g. 'AccessTime20Filled')
30121
* @property {string} fileName - kebab-case file name with .tsx extension
31-
* @property {{ paths: string[] } | { rawSvg: string }} iconData - extracted SVG data
122+
* @property {{ paths: string[] } | { nodes: SvgNode[], rawSvg: string }} iconData - extracted SVG data
32123
* @property {string} width - numeric width string (e.g. '20') or '1em' for resizable
33124
* @property {boolean} isColor - true if this is a color icon variant
34125
* @property {boolean} flipInRtl - true if the icon should be mirrored in RTL contexts
@@ -66,14 +157,14 @@ function parseIconSource(opts) {
66157
const rawWidth = resizable ? '"1em"' : getAttr('width')[0];
67158
const width = rawWidth.replace(/"/g, '');
68159

69-
/** @type {{ paths: string[] } | { rawSvg: string }} */
160+
/** @type {{ paths: string[] } | { nodes: SvgNode[], rawSvg: string }} */
70161
let iconData;
71162
if (isColor) {
72163
const innerSvg = svgContent
73164
.replace(/^[\s\S]*?<svg[^>]*>/, '')
74165
.replace(/<\/svg>[\s\S]*$/, '')
75166
.trim();
76-
iconData = { rawSvg: innerSvg };
167+
iconData = { nodes: parseSvgToNodes(innerSvg), rawSvg: innerSvg };
77168
} else {
78169
const pathValues = getAttr('d').map((v) => v.slice(1, -1)); // strip surrounding quotes
79170
iconData = { paths: pathValues };
@@ -106,8 +197,9 @@ function buildIconExportCode(parsed) {
106197
const deprecatedPrefix =
107198
'/** @deprecated Color icons are deprecated. [See User Guidance](https://microsoft.github.io/fluentui-system-icons/?path=/docs/icons-user-guidance--docs#color-variants-deprecated) */\n';
108199

109-
if ('rawSvg' in iconData) {
110-
return `${isColor ? deprecatedPrefix : ''}export const ${exportName}: FluentIcon = (/*#__PURE__*/createFluentIcon('${exportName}', ${widthStr}, \`${iconData.rawSvg}\`${options}));`;
200+
if ('nodes' in iconData) {
201+
const nodesStr = JSON.stringify(iconData.nodes);
202+
return `${isColor ? deprecatedPrefix : ''}export const ${exportName}: FluentIcon = (/*#__PURE__*/createFluentIcon('${exportName}', ${widthStr}, ${nodesStr}${options}));`;
111203
}
112204

113205
const paths = iconData.paths.map((p) => `"${p}"`).join(',');
@@ -212,4 +304,5 @@ module.exports = {
212304
getCreateFluentIconHeader,
213305
loadRtlMetadata,
214306
generatePerIconFiles,
307+
parseSvgToNodes,
215308
};

‎packages/react-icons/scripts/convert.utils.test.js‎

Lines changed: 73 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,13 @@ import fs from 'fs';
33
import path from 'path';
44
import { describe, it, expect, afterAll } from 'vitest';
55

6-
import { parseIconSource, buildIconExportCode, getCreateFluentIconHeader, generatePerIconFiles } from './convert.utils';
6+
import {
7+
parseIconSource,
8+
buildIconExportCode,
9+
getCreateFluentIconHeader,
10+
generatePerIconFiles,
11+
parseSvgToNodes,
12+
} from './convert.utils';
713

814
describe(`convert utils`, () => {
915
describe(`getCreateFluentIconHeader`, () => {
@@ -13,7 +19,7 @@ describe(`convert utils`, () => {
1319
expect(header).toHaveLength(3);
1420
expect(header).toMatchInlineSnapshot(`
1521
[
16-
""use client";",
22+
"\"use client\";",
1723
"import type { FluentIcon } from '../utils/createFluentIcon';",
1824
"import { createFluentIcon } from '../utils/createFluentIcon';",
1925
]
@@ -66,7 +72,7 @@ describe(`convert utils`, () => {
6672
expect(res?.iconData).toHaveProperty('paths');
6773
});
6874

69-
it('parses color icon and extracts rawSvg', () => {
75+
it('parses color icon and extracts rawSvg, nodes', () => {
7076
writeFile('ic_fluent_color_test_20_color.svg', '<svg width="20"><g fill="#000"/><path d="M1 2"/></svg>');
7177
const res = parseIconSource({
7278
file: 'ic_fluent_color_test_20_color.svg',
@@ -79,6 +85,7 @@ describe(`convert utils`, () => {
7985
expect(res?.exportName).toBe('ColorTestColor');
8086
expect(res?.isColor).toBe(true);
8187
expect(res?.iconData).toHaveProperty('rawSvg');
88+
expect(res?.iconData).toHaveProperty('nodes');
8289
});
8390

8491
it('sets flipInRtl when metadata indicates mirror', () => {
@@ -150,22 +157,28 @@ describe(`convert utils`, () => {
150157
expect(code).not.toContain('color: true');
151158
});
152159

153-
it('generates export code for color icon with rawSvg', () => {
160+
it('generates export code for color icon with rawSvg and nodes', () => {
154161
const code = buildIconExportCode({
155162
exportName: 'PatientColor',
156163
fileName: 'patient-color.tsx',
157-
iconData: { rawSvg: '<g fill="#000"/><path d="M1 2" fill="#ff0000"/>' },
164+
iconData: {
165+
rawSvg: '<g fill="#000"/><path d="M1 2" fill="#ff0000"/>',
166+
nodes: [
167+
['g', { fill: '#000' }],
168+
['path', { d: 'M1 2', fill: '#ff0000' }],
169+
],
170+
},
158171
width: '20',
159172
isColor: true,
160173
flipInRtl: false,
161174
});
162175
expect(code).toMatchInlineSnapshot(
163176
`
164177
"/** @deprecated Color icons are deprecated. [See User Guidance](https://microsoft.github.io/fluentui-system-icons/?path=/docs/icons-user-guidance--docs#color-variants-deprecated) */
165-
export const PatientColor: FluentIcon = (/*#__PURE__*/createFluentIcon('PatientColor', "20", \`<g fill="#000"/><path d="M1 2" fill="#ff0000"/>\`, { color: true }));"
178+
export const PatientColor: FluentIcon = (/*#__PURE__*/createFluentIcon('PatientColor', \"20\", [[\"g\",{\"fill\":\"#000\"}],[\"path\",{\"d\":\"M1 2\",\"fill\":\"#ff0000\"}]], { color: true }));"
166179
`,
167180
);
168-
expect(code).toContain('fill=');
181+
expect(code).toContain('fill');
169182
expect(code).toContain('color: true');
170183
});
171184

@@ -324,4 +337,57 @@ describe(`convert utils`, () => {
324337
});
325338
});
326339
});
340+
341+
describe('parseSvgToNodes', () => {
342+
it('parses self-closing elements', () => {
343+
const nodes = parseSvgToNodes('<path d="M1 2" fill="#000"/>');
344+
expect(nodes).toEqual([['path', { d: 'M1 2', fill: '#000' }]]);
345+
});
346+
347+
it('parses nested elements with children', () => {
348+
const nodes = parseSvgToNodes('<defs><linearGradient id="a"><stop stop-color="#fff"/></linearGradient></defs>');
349+
expect(nodes).toEqual([['defs', null, ['linearGradient', { id: 'a' }, ['stop', { stopColor: '#fff' }]]]]);
350+
});
351+
352+
it('converts hyphenated SVG attributes to camelCase', () => {
353+
const nodes = parseSvgToNodes('<path fill-opacity="0.5" fill-rule="evenodd" clip-rule="evenodd"/>');
354+
expect(nodes).toEqual([['path', { fillOpacity: '0.5', fillRule: 'evenodd', clipRule: 'evenodd' }]]);
355+
});
356+
357+
it('parses multiple top-level elements', () => {
358+
const nodes = parseSvgToNodes('<path d="M1"/><path d="M2"/>');
359+
expect(nodes).toEqual([
360+
['path', { d: 'M1' }],
361+
['path', { d: 'M2' }],
362+
]);
363+
});
364+
365+
it('parses inline style attributes as React-compatible style objects', () => {
366+
const nodes = parseSvgToNodes(
367+
'<g style="mask-type:alpha"><rect style="mix-blend-mode:multiply;opacity:0.5"/></g>',
368+
);
369+
expect(nodes).toEqual([
370+
['g', { style: { maskType: 'alpha' } }, ['rect', { style: { mixBlendMode: 'multiply', opacity: '0.5' } }]],
371+
]);
372+
});
373+
374+
it('parses a real color icon SVG fragment', () => {
375+
const svg =
376+
'<path d="M2 8" fill="url(#a)"/><defs><linearGradient id="a" x1="1" gradientUnits="userSpaceOnUse"><stop stop-color="#52D17C"/><stop offset="1" stop-color="#22918B"/></linearGradient></defs>';
377+
const nodes = parseSvgToNodes(svg);
378+
expect(nodes).toEqual([
379+
['path', { d: 'M2 8', fill: 'url(#a)' }],
380+
[
381+
'defs',
382+
null,
383+
[
384+
'linearGradient',
385+
{ id: 'a', x1: '1', gradientUnits: 'userSpaceOnUse' },
386+
['stop', { stopColor: '#52D17C' }],
387+
['stop', { offset: '1', stopColor: '#22918B' }],
388+
],
389+
],
390+
]);
391+
});
392+
});
327393
});

0 commit comments

Comments
 (0)