Skip to content

fix(react-icons): properly resolve atoms grouppings if icon contains style variant as part of its name #955

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

Merged
Hotell merged 3 commits into from
Feb 3, 2026
Merged

fix(react-icons): properly resolve atoms grouppings if icon contains style variant as part of its name #955

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
181f866
fix(react-icons): properly resolve atoms grouppings if icon contains …
Hotell Jan 22, 2026
f98e0ed
refactor: make assertCompoundStyleVariantIssues async and dry
Hotell Jan 23, 2026
5a26eef
test: update assertion to toContain
Hotell Jan 30, 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
4 changes: 4 additions & 0 deletions packages/react-icons/__snapshots__/build-verify.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2462,6 +2462,7 @@ exports[`Build Verification > Atoms > should have atoms/fonts directory with ico
"text-clear-formatting.js",
"text-collapse.js",
"text-color-accent.js",
"text-color.js",
"text-column-one-narrow.js",
"text-column-one-semi-narrow.js",
"text-column-one-wide-lightning.js",
Expand Down Expand Up @@ -5265,6 +5266,7 @@ exports[`Build Verification > Atoms > should have atoms/fonts directory with ico
"text-clear-formatting.js",
"text-collapse.js",
"text-color-accent.js",
"text-color.js",
"text-column-one-narrow.js",
"text-column-one-semi-narrow.js",
"text-column-one-wide-lightning.js",
Expand Down Expand Up @@ -8075,6 +8077,7 @@ exports[`Build Verification > Atoms > should have atoms/svg directory with icon
"text-clear-formatting.js",
"text-collapse.js",
"text-color-accent.js",
"text-color.js",
"text-column-one-narrow.js",
"text-column-one-semi-narrow.js",
"text-column-one-wide-lightning.js",
Expand Down Expand Up @@ -10885,6 +10888,7 @@ exports[`Build Verification > Atoms > should have atoms/svg directory with icon
"text-clear-formatting.js",
"text-collapse.js",
"text-color-accent.js",
"text-color.js",
"text-column-one-narrow.js",
"text-column-one-semi-narrow.js",
"text-column-one-wide-lightning.js",
Expand Down
46 changes: 42 additions & 4 deletions packages/react-icons/build-verify.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1830,15 +1830,15 @@ describe('Build Verification', () => {
const { svgPathCjs, svgPathEsm } = getAssetPaths();
const esmStats = await getStats(svgPathEsm);
const cjsStats = await getStats(svgPathCjs);
expect(esmStats.jsFiles.length).toMatchInlineSnapshot(`2805`);
expect(cjsStats.jsFiles.length).toMatchInlineSnapshot(`2805`);
expect(esmStats.jsFiles.length).toMatchInlineSnapshot(`2806`);
expect(cjsStats.jsFiles.length).toMatchInlineSnapshot(`2806`);
});
it(`should have same number of atoms/fonts icon files in lib and lib-cjs`, async () => {
const { fontsPathCjs, fontsPathEsm } = getAssetPaths();
const esmStats = await getStats(fontsPathEsm);
const cjsStats = await getStats(fontsPathCjs);
expect(esmStats.jsFiles.length).toMatchInlineSnapshot(`2798`);
expect(cjsStats.jsFiles.length).toMatchInlineSnapshot(`2798`);
expect(esmStats.jsFiles.length).toMatchInlineSnapshot(`2799`);
expect(cjsStats.jsFiles.length).toMatchInlineSnapshot(`2799`);
});
it.each(['lib', 'lib-cjs'])('should have atoms/svg directory with icon files in %s', async (libDir) => {
const atomsSvgPath = path.join(__dirname, libDir, 'atoms', 'svg');
Expand Down Expand Up @@ -1950,6 +1950,44 @@ describe('Build Verification', () => {
expect(packageJson.exports['./fonts/*'].import).toBe('./lib/atoms/fonts/*.js');
expect(packageJson.exports['./fonts/*'].require).toBe('./lib-cjs/atoms/fonts/*.js');
});

it.each(['svg', 'fonts'])(
'text-color atoms should be properly separated from text atoms in lib/atoms/%s',
async (exportKindDir) => {
const textFile = path.join(__dirname, 'lib', 'atoms', exportKindDir, 'text.js');
const textColorFile = path.join(__dirname, 'lib', 'atoms', exportKindDir, 'text-color.js');
// Both files should exist
expect(fs.existsSync(textFile)).toBe(true);
expect(fs.existsSync(textColorFile)).toBe(true);

const textContent = await readFile(textFile, 'utf-8');
const textColorContent = await readFile(textColorFile, 'utf-8');

// text-color.js should only contain TextColor* exports
expect(textColorContent).toContain(`export const TextColorFilled`);
expect(textColorContent).toContain(`export const TextColorRegular`);
expect(textColorContent).toContain(`export const TextColor16Regular`);
expect(textColorContent).toContain(`export const TextColor20Regular`);
expect(textColorContent).toContain(`export const TextColor24Regular`);

// text-color.js should NOT contain Text* exports (without Color in the name)
expect(textColorContent).not.toContain('export const Text12Regular');
expect(textColorContent).not.toContain('export const Text16Regular');

// text.js should contain Text* exports
expect(textContent).toContain('export const Text12Regular');
expect(textContent).toContain('export const Text16Regular');
expect(textContent).toContain('export const Text32Regular');

// text.js should have backward-compatible re-exports for TextColor* with deprecation notice
expect(textContent).toContain(`@deprecated use \`@fluentui/${exportKindDir}/text-color\` import`);
expect(textContent).toContain(`export const TextColorFilled`);
expect(textContent).toContain(`export const TextColorRegular`);
expect(textContent).toContain(`export const TextColor16Regular`);
expect(textContent).toContain(`export const TextColor20Regular`);
expect(textContent).toContain(`export const TextColor24Regular`);
},
);
});

describe('Metadata Validation', () => {
Expand Down
10 changes: 8 additions & 2 deletions packages/react-icons/convert-font.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,11 @@ const mkdirp = require('mkdirp');

const yargs = require('yargs');
const _ = require('lodash');
const { handleDeprecatedColorAtoms } = require('./deprecated-atoms');
const {
assertCompoundStyleVariantIssues,
handleDeprecatedColorAtoms,
handleDeprecatedTextColorAtoms,
} = require('./deprecated-atoms');
const { createFormatMetadata, writeMetadata } = require('./metadata.utils');
const { createStableChunks } = require('./chunking-utils');
const {
Expand Down Expand Up @@ -281,7 +285,9 @@ async function processPerIcon(destPath, iconEntries, rtlMetadata, options = { gr
const sized = await generatePerIconFiles(destPath, iconEntries.sized, rtlMetadata, false, options.groupByBase);
Object.assign(fontMetadata, createFormatMetadata(sized.iconNames, 'font', 'sized'));

await handleDeprecatedColorAtoms(destPath, 'font');
handleDeprecatedColorAtoms(destPath, 'font');
handleDeprecatedTextColorAtoms(destPath, 'font');
await assertCompoundStyleVariantIssues(destPath);

console.log(`[font per-icon] Wrote ${resizable.fileCount + sized.fileCount} files to ${destPath}`);

Expand Down
10 changes: 8 additions & 2 deletions packages/react-icons/convert.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,11 @@ const { readdir } = require('fs/promises');
const path = require('path');
const yargs = require('yargs');
const { makeIconExport, getCreateFluentIconHeader, loadRtlMetadata, generatePerIconFiles } = require('./convert.utils');
const { handleDeprecatedColorAtoms } = require('./deprecated-atoms');
const {
assertCompoundStyleVariantIssues,
handleDeprecatedColorAtoms,
handleDeprecatedTextColorAtoms,
} = require('./deprecated-atoms');
const { createStableChunks } = require('./chunking-utils');
const { createFormatMetadata, writeMetadata } = require('./metadata.utils');

Expand Down Expand Up @@ -180,7 +184,9 @@ async function processPerIcon(sourceFiles, destPath, rtlMetadata, options = { gr
const sized = await generatePerIconFiles(sourceFiles, destPath, rtlMetadata, false, options.groupByBase);
Object.assign(svgMetadata, createFormatMetadata(sized.iconNames, 'svg', 'sized'));

await handleDeprecatedColorAtoms(destPath, 'svg');
handleDeprecatedColorAtoms(destPath, 'svg');
handleDeprecatedTextColorAtoms(destPath, 'svg');
await assertCompoundStyleVariantIssues(destPath);

console.log(`[svg per-icon] Wrote ${resizable.fileCount + sized.fileCount} icon files to ${destPath}`);
return { svgMetadata };
Expand Down
14 changes: 7 additions & 7 deletions packages/react-icons/convert.utils.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,9 +151,9 @@ describe(`convert utils`, () => {
if (fs.existsSync(tmpDest)) fs.rmSync(tmpDest, { recursive: true, force: true });

// Two different filenames that will produce the same export name when processed
writeSvg('ic_fluent_dup_20.svg', '<svg width="20" d="M0"></svg>');
// this variant without underscore will camelCase to the same export name (Dup20)
writeSvg('ic_fluent_dup20.svg', '<svg width="20" d="M1"></svg>');
writeSvg('ic_fluent_dup_20_regular.svg', '<svg width="20" d="M0"></svg>');
Copy link
Copy Markdown
Collaborator Author

@Hotell Hotell Jan 23, 2026

Choose a reason for hiding this comment

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

updates test to reflect reality of /assets

// this variant without underscore will camelCase to the same export name (Dup20Regular)
writeSvg('ic_fluent_dup20_regular.svg', '<svg width="20" d="M1"></svg>');

const files = await import('fs/promises').then((m) => m.readdir(tmpSrc));
if (!fs.existsSync(tmpDest)) fs.mkdirSync(tmpDest, { recursive: true });
Expand Down Expand Up @@ -190,8 +190,8 @@ describe(`convert utils`, () => {

it('groups related icons into one file and deduplicates exports', async () => {
// prepare source svgs
writeSvg('ic_fluent_zoom_in_20.svg', 'M0 0');
writeSvg('ic_fluent_zoom_in_16.svg', 'M1 1');
writeSvg('ic_fluent_zoom_in_20_regular.svg', 'M0 0');
writeSvg('ic_fluent_zoom_in_16_regular.svg', 'M1 1');
writeSvg('ic_fluent_zoom_in_20_filled.svg', 'M2 2');

const files = await require('fs/promises').readdir(tmpSrc);
Expand All @@ -209,7 +209,7 @@ describe(`convert utils`, () => {
// should contain multiple exports
expect(content).toContain('export const ZoomIn20Filled');
// 16px variant should be present after processing sized icons
expect(content).toMatch(/export const\s+ZoomIn16/);
expect(content).toMatch(/export const\s+ZoomIn16Regular/);
// no duplicate export names
const names = [...content.matchAll(/export const\s+(\w+)\s*:/g)].map((m) => m[1]);
const unique = new Set(names);
Expand All @@ -222,7 +222,7 @@ describe(`convert utils`, () => {
const tmpDest2 = path.join(__dirname, 'tmp-gen-dest-2');
if (!fs.existsSync(tmpSrc2)) fs.mkdirSync(tmpSrc2, { recursive: true });
if (!fs.existsSync(tmpDest2)) fs.mkdirSync(tmpDest2, { recursive: true });
fs.writeFileSync(path.join(tmpSrc2, 'ic_fluent_test_20.svg'), '<svg width="20" d="M0"></svg>');
fs.writeFileSync(path.join(tmpSrc2, 'ic_fluent_test_20_regular.svg'), '<svg width="20" d="M0"></svg>');
fs.writeFileSync(path.join(tmpSrc2, 'ic_fluent_test_16_filled.svg'), '<svg width="16" d="M1"></svg>');

const files = await require('fs/promises').readdir(tmpSrc2);
Expand Down
Loading
Loading