Skip to content
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.

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

@fluentui/react-icons: build performance & package bloat #25989

Open
layershifter opened this issue Dec 14, 2022 · 23 comments
Open

@fluentui/react-icons: build performance & package bloat #25989

layershifter opened this issue Dec 14, 2022 · 23 comments
Assignees

Comments

@layershifter
Copy link
Member

layershifter commented Dec 14, 2022

Library

React Components / v9 (@fluentui/react-components) & @fluentui/react-icons.

Current state with tooling

Webpack

In development

To be done

In production

Tree-shaking works ✅
Bundles reasonably fast (webpack 5.75.0 on Apple M1 in 1857 ms, 2048 ms) ✅

CodeSandbox

non cloud

Works surprisingly well (even with 4x CPU slowdown):
https://codesandbox.io/s/naughty-paper-14wujx?file=/src/App.js

Uses CJS and loads all chunks for every icon ⚠️

image

cloud beta

Works even better
https://codesandbox.io/p/sandbox/priceless-knuth-m8e6iz

Loads bundled ESM by Vite (around 2.1mb)

image

TypeScript

Used TS 4.9.4.
Builds a fixture reasonably fast with skipLibCheck: true & skipLibCheck: false

Next.js

Used Next.js 13.0.6.
Rebuilds in dev are reasonably fast. Builds, too (on Apple M1 12.37s with icons, 8.67s without).

create-react-app

In development

To be done

In production

It's still Webpack under hood, but they process node_modules with Babel loader 💥

  • Cold build 17.07s with icons
  • Cold build 3.30s without icons
  • Warm build 4.51s with icons
  • Warm build 3.33s without icons

Jest

Does not seem that icons affect it in any way.

Package size

It's where things are getting worse. react-icons-2.0.190.tgz downloadable size is 15.1mb, unpacked size is 71mb 💥

Fonts

It's a big problem actually as it double package twice.

Action 1 (mandatory): glyphs should be present once

Currently glyphs are present twice for CJS & ESM 💣 However, fonts are not modules and a JSON file next them is also not a module.

https://www.jsdelivr.com/package/npm/@fluentui/react-icons?path=lib%2Futils%2Ffonts
https://www.jsdelivr.com/package/npm/@fluentui/react-icons?path=lib-cjs%2Futils%2Ffonts

If we will have a single instance we go down to 63mb (-8mb!) unpacked.

Action 2: remove fonts completely

  • There are not a part of public API
  • Some customers don't need these fonts

Move fonts and related JS to a separate package:

  • Partners who needs it - will install it
  • Partners who don't need it - will not download more than needed

While the plugin looks on a relative path - I don't think that it's a good enough reason to keep ghyphs and slow down installs for everyone.

Deleting fonts completely moves us down to 41mb.

Types

Duplicate types

Currently we have separate types for ESM and CJS:

image

It means that we duplicate definitions for a no reason as the package.json references only lib/index.d.ts and we have export maps that prevent deep imports.

Duplicate types cost 5mb.
Ideally, it would be great to rollup them.

Expanded definitions ✅

image

We have this for every icon, types are inlined, not great at all.

Killing a generic there makes types more accurate:

export declare const HeartRegular: {
    (props: FluentIconsProps): JSX.Element;
    displayName: string | undefined;
};

Ideally, it should be just:

export declare const HeartRegular: FluentIcon;

Usage of a type without displayName probably will lead to better results. Just the change above saves 3mb.

Minify output

To be done.

Requested priority

High

@layershifter layershifter changed the title @fluentui/react-icons: build performance @fluentui/react-icons: build performance & package bloat Dec 14, 2022
@layershifter
Copy link
Member Author

The problem with types affects also consumer side. In the project we have the whole set of icons bundled:

import {
  bundleIcon,
  AccessTimeFilled,
  AccessibilityFilled
} from "@fluentui/react-icons";

export const AccessTime: FluentIcon = /*#__PURE__*/ bundleIcon(
  AccessTimeFilled,
  AccessTimeRegular
);

/* all other icons */
/* ... */

The problem what we have is build performance: we use TS compiler to compile TS code and generate type definitions. tsc --extendedDiagnostics gives following:

  ✖ Files:                         755
  ✖ Lines of Library:            32995
  ✖ Lines of Definitions:       384954
  ✖ Lines of TypeScript:         12437
  ✖ Lines of JavaScript:             0
  ✖ Lines of JSON:                   0
  ✖ Lines of Other:                  0
  ✖ Nodes of Library:           149996
  ✖ Nodes of Definitions:      1566104
  ✖ Nodes of TypeScript:         31750
  ✖ Nodes of JavaScript:             0
  ✖ Nodes of JSON:                   0
  ✖ Nodes of Other:                  0
  ✖ Identifiers:                509267
  ✖ Symbols:                   1071185
  ✖ Types:                       31575
  ✖ Instantiations:            3317977
  ✖ Memory used:               928504K
  ✖ Assignability cache size:    41352
  ✖ Identity cache size:           309
  ✖ Subtype cache size:            430
  ✖ Strict subtype cache size:       0
  ✖ I/O Read time:               0.30s
  ✖ Parse time:                  1.21s
  ✖ ResolveModule time:          0.19s
  ✖ ResolveTypeReference time:   0.07s
  ✖ Program time:                1.84s
  ✖ Bind time:                   0.53s
  ✖ Check time:                  6.67s
  ✖ printTime time:             65.60s
  ✖ Emit time:                  65.60s
  ✖ transformTime time:         65.31s
  ✖ Source Map time:             0.01s
  ✖ commentTime time:            0.03s
  ✖ I/O Write time:              0.05s
  ✖ Total time:                 74.64s

Time printTime, Emit time & transformTime are insane 💥

Running with emitDeclarationOnly if printTime is high.
https://github.com/microsoft/TypeScript/wiki/Performance#performance-considerations

I run the same build with declaration: false (which we can't really do):

  ✖ Check time:                  7.33s
  ✖ printTime time:              0.30s
  ✖ Emit time:                   0.31s
  ✖ transformTime time:          0.04s
  ✖ Total time:                 12.18s

At the same time I tried to collect CPU profile to understand what explodes:

Current state

image

declaration: false

image

getAccessibleSymbolChainFromSymbolTable points to microsoft/TypeScript#34119 where the similar issue is discussed: performance slowdown on huge types. Considering that I tried to simplify types to avoid complex d.ts like this:

image

Basically did explicit typing to FluentIcon:

+import * as React from "react";

+type FluentIcon = React.FC<
+  React.SVGAttributes<SVGElement> & {
+    primaryFill?: string | undefined;
+    className?: string | undefined;
+    filled?: boolean | undefined;
+    title?: string | undefined;
+  }
+>;

-export const AccessTime = /*#__PURE__*/ bundleIcon(
+export const AccessTime: FluentIcon = /*#__PURE__*/ bundleIcon(
  AccessTimeFilled,
  AccessTimeRegular
);

The change results in:

image

image

  ✖ printTime time:              0.30s
  ✖ Emit time:                   0.31s
  ✖ transformTime time:          0.10s
  ✖ Source Map time:             0.01s
  ✖ commentTime time:            0.02s
  ✖ I/O Write time:              0.03s
  ✖ Total time:                  8.49s

I am not sure if better typings for icons on lib side will change things, but it would be great to test.

@layershifter layershifter added Type: Bug 🐛 Type: Epic Partner Ask Shield: P1 Shield developers rate the issue as a high priority (good issue) and removed Needs: Triage 🔍 Type: Epic labels Jan 24, 2023
@layershifter
Copy link
Member Author

layershifter commented Mar 17, 2023

Other possible actions:

Compress output

Apply minifications to distributed files to compress JS output i.e. run Terser/SWC on lib/**/*.js and lib-cjs/**/*.js.

Do not ship ESM

Another risky attempt might be to ship only CommonJS, here is a sandbox: https://stackblitz.com/edit/node-e7ki6x?file=index.js

  • Use type: module + export maps & "moduleResolution": "node16" in TS
  • icons-*.cjs files contain only CJS output
  • index.cjs exports all icons
  • index.mjs re-exports icons from CJS entrypoint

While it works with Node:

~/projects/node-e7ki6x
❯ node index.mjs
[Module: null prototype] { FooIcon: 'Foo-Icon', BarIcon: 'Bar-Icon' }

~/projects/node-e7ki6x
❯ node index.cjs
{ FooIcon: 'Foo-Icon', BarIcon: 'Bar-Icon' }

I am not sure what will be an effect on bundlers 👀

Why not CJS entrypoint instead?

// index.mjs

export const IconX = ''
export const IconY = ''
// index.cjs

module.exports = await import('./index.mjs')

This does not work as top level await works only in modules.

Why not ESM only?

Jest does not handle native ESM well, transpilation on consumer side will be required.

@spmonahan
Copy link
Contributor

Another risky attempt might be to ship only CommonJS...
... I am not sure what will be an effect on bundlers 👀

I think this prevents bundlers from tree-shaking code.

@layershifter
Copy link
Member Author

layershifter commented Mar 21, 2023

@spmonahan I updated the sandbox. Treeshaking works with Rollup at least, I believe that it could work with Webpack after some tweaks.

@spmonahan
Copy link
Contributor

Cool! I'd love to see how that works.

@kkocdko
Copy link

kkocdko commented Mar 30, 2023

I agree with layershifter's idea about Do not ship ESM, the React itself also do this. But Compress output may caused much longer build time but only reduce a few kb.

@MLoughry
Copy link
Contributor

No, don't unbundle the fonts. The entire purpose of the font work I did was so that Outlook (or any other bundling app concerned about performance) could convert all icons to the font versions, including those in our dependencies (who could be agnostic about SVG vs font).

This was a big performance win for Outlook. Please set up time with us before undoing it.

@MLoughry
Copy link
Contributor

Also, if you're only going to ship one of ESM or CJS, why would you ever pick CJS? It's not statically analyzable, and webpack won't tree-shake it.

@layershifter
Copy link
Member Author

layershifter commented Jun 20, 2023

No, don't unbundle the fonts. The entire purpose of the font work I did was so that Outlook (or any other bundling app concerned about performance) could convert all icons to the font versions, including those in our dependencies (who could be agnostic about SVG vs font).

This was a big performance win for Outlook. Please set up time with us before undoing it.

Your work will not be removed, fonts should be just moved to a separate package (for example, @fluentui/react-icons-fonts), so consumers that don't use that feature will not have bloat in their node_modules. Pay-as-you-use.

Also, if you're only going to ship one of ESM or CJS, why would you ever pick CJS?

There is no agreement about that, it's one of proposals. Why pick CJS? Because it works in any environment, ESM code does not have good support in underlying tooling (hello Jest 🙃).

It's not statically analyzable, and webpack won't tree-shake it.

That's not true, Webpack supports tree shaking in CJS, Stackblitz demo

@MLoughry
Copy link
Contributor

Your work will not be removed, fonts should be just moved to a separate package (for example, @fluentui/react-icons-fonts), so consumers that don't use that feature will not have bloat in their node_modules. Pay-as-you-use.

No, I don't think you understand. If 1JS or Fluent or whatever are using the SVG package, and you unbundle the fonts from the package, there's no way for the bundler to coerce those SVG assets to fonts. Currently, that can be done with conditional exports: https://github.com/microsoft/fluentui-system-icons/tree/main/packages/react-icons#using-the-icon-font

// webpack.config.js
module.exports = {
  //...
  resolve: {
    conditionNames: ['fluentIconFont', 'require', 'node'],
  },
};

@MLoughry

This comment was marked as off-topic.

@layershifter

This comment was marked as off-topic.

@layershifter
Copy link
Member Author

layershifter commented Jun 21, 2023

No, I don't think you understand. If 1JS or Fluent or whatever are using the SVG package, and you unbundle the fonts from the package, there's no way for the bundler to coerce those SVG assets to fonts. Currently, that can be done with conditional exports: https://github.com/microsoft/fluentui-system-icons/tree/main/packages/react-icons#using-the-icon-font

Sorry, it's still unclear what do you mean. Can you please elaborate?

For example, imports to a package could be replaced by a custom loader:

-import { IconX } from '@fluentui/react-icons'
+import { IconX } from '@fluentui/react-font-icons'

Or even use a Webpack alias.

@svdoever

This comment was marked as off-topic.

@layershifter
Copy link
Member Author

layershifter commented Jun 22, 2023

Is there a way to solve this issue using Webpack? We don't even use icons (but use @fluentui/react-components) and end up with this mess, almost 14MB to use a few compnents:

547a530c-64a2-49d9-8d95-eaab03a0df18

Making the use use of fluentui a nightmare, because we use it to build widgets for Azure DevOps, and each widget is hosted in its own iframe. Some every widget JavaScript file ends up to be 14MB+ in size. This completely blows up the browser...

@svdoever This is not related to the problem described this issue. From the screenshot it's visible that you are using CommonJS output (should be ESM) in Webpack that is not expected and indicates a potential problem with your Webpack configuration. Please open a new issue and provide a minimal reproduction there, so we could take a look.

@carmezini
Copy link

Is there a way to solve this issue using Webpack? We don't even use icons (but use @fluentui/react-components) and end up with this mess, almost 14MB to use a few compnents:
547a530c-64a2-49d9-8d95-eaab03a0df18
Making the use use of fluentui a nightmare, because we use it to build widgets for Azure DevOps, and each widget is hosted in its own iframe. Some every widget JavaScript file ends up to be 14MB+ in size. This completely blows up the browser...

@svdoever This is not related to the problem described this issue. From the screenshot it's visible that you are using CommonJS output (should be ESM) in Webpack that is not expected and indicates a potential problem with your Webpack configuration. Please open a new issue and provide a minimal reproduction there, so we could take a look.

Hey guys, I am facing the same problem. The new issue was created, after all?

@svdoever
Copy link

svdoever commented Oct 12, 2023 via email

@microsoft-github-policy-service microsoft-github-policy-service bot added the Resolution: Soft Close Soft closing inactive issues over a certain period label Mar 10, 2024
@layershifter layershifter reopened this Mar 11, 2024
@layershifter layershifter removed the Resolution: Soft Close Soft closing inactive issues over a certain period label Mar 11, 2024
@lainRosensteel
Copy link

This is currently causing a blocking issue when using Fluent UI v9 with the Power Apps Component Framework (PAC). The size of the bundle with only a button and a label is reaching close to 14MB, almost entirely from the icons and fonts. The size is so large that the PAC CLI tools and PCF scripts are unable to upload the code component bundle to Power Apps. In other words, it is currently not possible to use Fluent UI v9 with the PAC CLI tools due to the bundle size being too large. The workaround until this is resolved is to use Fluent UI v8 (or maybe there is some hack with webpack since the PCF scripts use it to bundle).

Technical issues aside, the icons and fonts are covered under a different license. Most of the repository is MIT, except the icons and fonts. This creates a lack of consistency and some legal ambiguity about the use of the icons and fonts as well as the mandatory inclusion of the SVG icons with the bundle. According to the Microsoft Fabric Assets License Agreement, download of the icons and fonts constitutes agreement to the license terms which restricts the use of the icons and fonts:

In connection with the use of a Microsoft API within the development of a software
application, website, or product you create or a service you offer designed to provide access
or interact with a Microsoft service or application (“Application”)

Although I doubt Microsoft will enforce use of the Segoe UI font for only Microsoft services or applications, it muddies the water.

Having the icons and fonts in a separate repository or package would resolve both the issues; The icon and font coupling issue creating massive bundles which is in turn prevents PAC code components from using Fluent UI v9 without hacks, and the ambiguous licensing issue.

@MLoughry
Copy link
Contributor

The size of the bundle with only a button and a label is reaching close to 14MB, almost entirely from the icons and fonts.

How are you bundling your app? It sounds like you do no tree-shaking at all, if using a single button pulls in the entire package, which is a much bigger issue for you than just this package.

@lainRosensteel
Copy link

lainRosensteel commented Mar 13, 2024

@MLoughry using the pcf-scripts with a project initialized via pcf init.

There is an official Microsoft guide that uses Fluent UI v8.
That all works fine. Once it is setup and tested, migrate from v8 to v9 and you should run into the same issue.

Component Framework: Tutorial Create Canvas Dataset Component

Unfortunately, the official pcf-scripts package is not an open-source project or hosted publicly on GitHub.
However, it is viewable on npmjs.com.

Code for the pcf-scripts package

@thelok
Copy link

thelok commented Mar 14, 2024

I followed the comment from microsoft/fluentui-system-icons#619 (comment)

I was using "module": "commonjs" in my tsconfig.json.

I changed it to:

"module": "esnext",
"moduleResolution": "node",

and it changed my webpack bundle size from 14MB down to 1.7MB.

@lainRosensteel
Copy link

I followed the comment from microsoft/fluentui-system-icons#619 (comment)

I was using "module": "commonjs" in my tsconfig.json.

I changed it to:

"module": "esnext",
"moduleResolution": "node",

and it changed my webpack bundle size from 14MB down to 1.7MB.

That worked! Although ironically topical that the answer was switching from cjs to ESM for the module output. I also tried using "moduleResolution": "node" with cjs (default from the pfc-script tsconfig) and it still generated a 14 MB bundle. So maybe there is still something there in relation to cjs module output with webpack, but the issue I experienced now looks more like a pcf-scripts issue with the init script and their base tsconfig than a fluentui issue. Thanks for looking into that @thelok !

@tudorpopams tudorpopams assigned mainframev and unassigned tomi-msft Jun 4, 2024
@Hotell
Copy link
Contributor

Hotell commented Jul 1, 2024

Jest

Does not seem that icons affect it in any way.

jest/unit-tests are indeed affected #31714 (comment)

Webpack

#24952 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests