Skip to content

Commit b20432b

Browse files
authored
fix(react-icons): properly resolve atoms grouppings if icon contains style variant as part of its name (#955)
* fix(react-icons): properly resolve atoms grouppings if icon contains style variant as part of its name * refactor: make assertCompoundStyleVariantIssues async and dry * test: update assertion to toContain
1 parent 53e89c6 commit b20432b

File tree

9 files changed

+469
-56
lines changed

9 files changed

+469
-56
lines changed

packages/react-icons/__snapshots__/build-verify.test.js.snap

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2462,6 +2462,7 @@ exports[`Build Verification > Atoms > should have atoms/fonts directory with ico
24622462
"text-clear-formatting.js",
24632463
"text-collapse.js",
24642464
"text-color-accent.js",
2465+
"text-color.js",
24652466
"text-column-one-narrow.js",
24662467
"text-column-one-semi-narrow.js",
24672468
"text-column-one-wide-lightning.js",
@@ -5265,6 +5266,7 @@ exports[`Build Verification > Atoms > should have atoms/fonts directory with ico
52655266
"text-clear-formatting.js",
52665267
"text-collapse.js",
52675268
"text-color-accent.js",
5269+
"text-color.js",
52685270
"text-column-one-narrow.js",
52695271
"text-column-one-semi-narrow.js",
52705272
"text-column-one-wide-lightning.js",
@@ -8075,6 +8077,7 @@ exports[`Build Verification > Atoms > should have atoms/svg directory with icon
80758077
"text-clear-formatting.js",
80768078
"text-collapse.js",
80778079
"text-color-accent.js",
8080+
"text-color.js",
80788081
"text-column-one-narrow.js",
80798082
"text-column-one-semi-narrow.js",
80808083
"text-column-one-wide-lightning.js",
@@ -10885,6 +10888,7 @@ exports[`Build Verification > Atoms > should have atoms/svg directory with icon
1088510888
"text-clear-formatting.js",
1088610889
"text-collapse.js",
1088710890
"text-color-accent.js",
10891+
"text-color.js",
1088810892
"text-column-one-narrow.js",
1088910893
"text-column-one-semi-narrow.js",
1089010894
"text-column-one-wide-lightning.js",

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

Lines changed: 42 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1830,15 +1830,15 @@ describe('Build Verification', () => {
18301830
const { svgPathCjs, svgPathEsm } = getAssetPaths();
18311831
const esmStats = await getStats(svgPathEsm);
18321832
const cjsStats = await getStats(svgPathCjs);
1833-
expect(esmStats.jsFiles.length).toMatchInlineSnapshot(`2805`);
1834-
expect(cjsStats.jsFiles.length).toMatchInlineSnapshot(`2805`);
1833+
expect(esmStats.jsFiles.length).toMatchInlineSnapshot(`2806`);
1834+
expect(cjsStats.jsFiles.length).toMatchInlineSnapshot(`2806`);
18351835
});
18361836
it(`should have same number of atoms/fonts icon files in lib and lib-cjs`, async () => {
18371837
const { fontsPathCjs, fontsPathEsm } = getAssetPaths();
18381838
const esmStats = await getStats(fontsPathEsm);
18391839
const cjsStats = await getStats(fontsPathCjs);
1840-
expect(esmStats.jsFiles.length).toMatchInlineSnapshot(`2798`);
1841-
expect(cjsStats.jsFiles.length).toMatchInlineSnapshot(`2798`);
1840+
expect(esmStats.jsFiles.length).toMatchInlineSnapshot(`2799`);
1841+
expect(cjsStats.jsFiles.length).toMatchInlineSnapshot(`2799`);
18421842
});
18431843
it.each(['lib', 'lib-cjs'])('should have atoms/svg directory with icon files in %s', async (libDir) => {
18441844
const atomsSvgPath = path.join(__dirname, libDir, 'atoms', 'svg');
@@ -1950,6 +1950,44 @@ describe('Build Verification', () => {
19501950
expect(packageJson.exports['./fonts/*'].import).toBe('./lib/atoms/fonts/*.js');
19511951
expect(packageJson.exports['./fonts/*'].require).toBe('./lib-cjs/atoms/fonts/*.js');
19521952
});
1953+
1954+
it.each(['svg', 'fonts'])(
1955+
'text-color atoms should be properly separated from text atoms in lib/atoms/%s',
1956+
async (exportKindDir) => {
1957+
const textFile = path.join(__dirname, 'lib', 'atoms', exportKindDir, 'text.js');
1958+
const textColorFile = path.join(__dirname, 'lib', 'atoms', exportKindDir, 'text-color.js');
1959+
// Both files should exist
1960+
expect(fs.existsSync(textFile)).toBe(true);
1961+
expect(fs.existsSync(textColorFile)).toBe(true);
1962+
1963+
const textContent = await readFile(textFile, 'utf-8');
1964+
const textColorContent = await readFile(textColorFile, 'utf-8');
1965+
1966+
// text-color.js should only contain TextColor* exports
1967+
expect(textColorContent).toContain(`export const TextColorFilled`);
1968+
expect(textColorContent).toContain(`export const TextColorRegular`);
1969+
expect(textColorContent).toContain(`export const TextColor16Regular`);
1970+
expect(textColorContent).toContain(`export const TextColor20Regular`);
1971+
expect(textColorContent).toContain(`export const TextColor24Regular`);
1972+
1973+
// text-color.js should NOT contain Text* exports (without Color in the name)
1974+
expect(textColorContent).not.toContain('export const Text12Regular');
1975+
expect(textColorContent).not.toContain('export const Text16Regular');
1976+
1977+
// text.js should contain Text* exports
1978+
expect(textContent).toContain('export const Text12Regular');
1979+
expect(textContent).toContain('export const Text16Regular');
1980+
expect(textContent).toContain('export const Text32Regular');
1981+
1982+
// text.js should have backward-compatible re-exports for TextColor* with deprecation notice
1983+
expect(textContent).toContain(`@deprecated use \`@fluentui/${exportKindDir}/text-color\` import`);
1984+
expect(textContent).toContain(`export const TextColorFilled`);
1985+
expect(textContent).toContain(`export const TextColorRegular`);
1986+
expect(textContent).toContain(`export const TextColor16Regular`);
1987+
expect(textContent).toContain(`export const TextColor20Regular`);
1988+
expect(textContent).toContain(`export const TextColor24Regular`);
1989+
},
1990+
);
19531991
});
19541992

19551993
describe('Metadata Validation', () => {

packages/react-icons/convert-font.js

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,11 @@ const mkdirp = require('mkdirp');
99

1010
const yargs = require('yargs');
1111
const _ = require('lodash');
12-
const { handleDeprecatedColorAtoms } = require('./deprecated-atoms');
12+
const {
13+
assertCompoundStyleVariantIssues,
14+
handleDeprecatedColorAtoms,
15+
handleDeprecatedTextColorAtoms,
16+
} = require('./deprecated-atoms');
1317
const { createFormatMetadata, writeMetadata } = require('./metadata.utils');
1418
const { createStableChunks } = require('./chunking-utils');
1519
const {
@@ -281,7 +285,9 @@ async function processPerIcon(destPath, iconEntries, rtlMetadata, options = { gr
281285
const sized = await generatePerIconFiles(destPath, iconEntries.sized, rtlMetadata, false, options.groupByBase);
282286
Object.assign(fontMetadata, createFormatMetadata(sized.iconNames, 'font', 'sized'));
283287

284-
await handleDeprecatedColorAtoms(destPath, 'font');
288+
handleDeprecatedColorAtoms(destPath, 'font');
289+
handleDeprecatedTextColorAtoms(destPath, 'font');
290+
await assertCompoundStyleVariantIssues(destPath);
285291

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

packages/react-icons/convert.js

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,11 @@ const { readdir } = require('fs/promises');
88
const path = require('path');
99
const yargs = require('yargs');
1010
const { makeIconExport, getCreateFluentIconHeader, loadRtlMetadata, generatePerIconFiles } = require('./convert.utils');
11-
const { handleDeprecatedColorAtoms } = require('./deprecated-atoms');
11+
const {
12+
assertCompoundStyleVariantIssues,
13+
handleDeprecatedColorAtoms,
14+
handleDeprecatedTextColorAtoms,
15+
} = require('./deprecated-atoms');
1216
const { createStableChunks } = require('./chunking-utils');
1317
const { createFormatMetadata, writeMetadata } = require('./metadata.utils');
1418

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

183-
await handleDeprecatedColorAtoms(destPath, 'svg');
187+
handleDeprecatedColorAtoms(destPath, 'svg');
188+
handleDeprecatedTextColorAtoms(destPath, 'svg');
189+
await assertCompoundStyleVariantIssues(destPath);
184190

185191
console.log(`[svg per-icon] Wrote ${resizable.fileCount + sized.fileCount} icon files to ${destPath}`);
186192
return { svgMetadata };

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -151,9 +151,9 @@ describe(`convert utils`, () => {
151151
if (fs.existsSync(tmpDest)) fs.rmSync(tmpDest, { recursive: true, force: true });
152152

153153
// Two different filenames that will produce the same export name when processed
154-
writeSvg('ic_fluent_dup_20.svg', '<svg width="20" d="M0"></svg>');
155-
// this variant without underscore will camelCase to the same export name (Dup20)
156-
writeSvg('ic_fluent_dup20.svg', '<svg width="20" d="M1"></svg>');
154+
writeSvg('ic_fluent_dup_20_regular.svg', '<svg width="20" d="M0"></svg>');
155+
// this variant without underscore will camelCase to the same export name (Dup20Regular)
156+
writeSvg('ic_fluent_dup20_regular.svg', '<svg width="20" d="M1"></svg>');
157157

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

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

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

228228
const files = await require('fs/promises').readdir(tmpSrc2);

0 commit comments

Comments
 (0)