Reduce react-icons package size

[kkocdko]

Define icons by the new createFluentIcon helper function, to reduce the npm package size and bundle size.

The ./lib dir: 21.3 MiB -> 9.85 MiB.

Issue https://github.com/microsoft/fluentui-system-icons/issues/561

[kkocdko]

@microsoft-github-policy-service agree

[kkocdko]

Seems conflict with https://github.com/microsoft/fluentui-system-icons/pull/505 . Maybe this improvement should be taken after that pr was merged?

I hope the progress be faster.

[layershifter]

@kkocdko we have an issue on Fluent UI side related to this problem, https://github.com/microsoft/fluentui/issues/25989. The idea with modifying glyphs is pretty nice. However, it requires more checks to ensure that nothing else will regress.

Can you please split this PR to two separate ones? I.e. make types changes and code changes separate. With that we could progress faster.

[layershifter]

BTW, while I can agree about dist size improvements, improvements in bundle size are not so great 🐱

┌────────────────────────────────┬───────────────┬───────────┐
│ Fixture                        │ Minified size │ GZIP size │
├────────────────────────────────┼───────────────┼───────────┤
│ react-icons (new): single icon │ 5.844 kB      │ 2.659 kB  │
├────────────────────────────────┼───────────────┼───────────┤
│ react-icons (new): 5 icons     │ 7.765 kB      │ 3.041 kB  │
├────────────────────────────────┼───────────────┼───────────┤
│ react-icons (old): single icon │ 5.726 kB      │ 2.603 kB  │
├────────────────────────────────┼───────────────┼───────────┤
│ react-icons (old): 5 icons     │ 8.541 kB      │ 3.028 kB  │
└────────────────────────────────┴───────────────┴───────────┘

[kkocdko]

@layershifter

About .d.ts file

Update: Fixed. I just remove : FluentIcon and keep import type { FluentIcon }. But don't know why.

I also tried your suggesstion:

for(const chunk of iconChunks) {
    chunk.unshift(`import { createFluentIcon } from "../utils/createFluentIcon";`);
+   chunk.unshift(`import type { FluentIcon } from "../utils/createFluentIcon";`);
}

-      var jsCode = `export const ${destFilename} = /*#__PURE__*/createFluentIcon('${destFilename}', ${width}, [${paths}]);`
+      var jsCode = `export const ${destFilename}: FluentIcon = /*#__PURE__*/createFluentIcon('${destFilename}', ${width}, [${paths}]);`

But what I shocked is, if I import the type FluentIcon and write export const xxx: FluentIcon, the /*#__PURE__*/ mark will gone in bundle result! I don't know why and how to fix this. Now fixed and your suggesstion applied.

Split this PR to two separate ones

Maybe hard to do... Create a type FluentIcon in pr 1 and use it in pr 2 will cause build failed, looks unpleasant. And I think this pr with < 40 lines of code should not be split more.

About bundle size in less-icon project

Thanks for you mention, I optimized the createFluentIcon function, please test again.

[layershifter]

But what I shocked is, if I import the type FluentIcon and write export const xxx: FluentIcon, the /*#__PURE__*/ mark will gone in bundle result! I don't know why and how to fix this. Now fixed and your suggesstion applied.

Oh my gosh, this TS bug strikes again, it's fixed in newer versions of TS 🙈

@kkocdko you need to wrap it with parens:

var jsCode = `export const ${destFilename}: FluentIcon = (/*#__PURE__*/createFluentIcon('${destFilename}', ${width}, [${paths}]));`

Here is a repro.

[layershifter]

Maybe hard to do... Create a type FluentIcon in pr 1 and use it in pr 2 will cause build failed, looks unpleasant. And I think this pr with < 40 lines of code should not be split more.

Well, I personally looking for a bigger refactor 🐱

As the chain is strange now: createFluentIcon() => wrapIcon()=> useIconState(). For me, it looks like too much for an icon 🤔 However, it has nothing to do with your PR.

I don't have other comments to this PR, @kkocdko great job ⭐ If you would like to contribute more, let's continue on https://github.com/microsoft/fluentui/issues/25989.

[kkocdko]

@layershifter wrapIcon() is skipped in createFluentIcon() but the define is not removed to avoid breaking changes.

Feel free to merge, close or reuse the code in this PR.

[tomi-msft]

Thanks for your contribution! This was extremely helpful