[Bug]: Using @carbon/icons-react/next in carbon 10 includes ALL icons instead of only the ones used

[Mikadv]

Package

carbon-components, carbon-components-react, @carbon/icons-react, @carbon/motion, @carbon/themes

Browser

Chrome

Package version

^10.49.0

React version

^16.14.0

Description

When importing icons from @carbon/icons-react, only a bunch of icons are later on bundled into chunks by the bundler. However when someone imports from @carbon/icons-react/next an icon in the same way suddenly ALL icons are loaded in the bundle.

How do we load icons?
import { Add16 } from "@carbon/icons-react"; or import { View } from "@carbon/icons-react/next";

With the first one we only get a small number of icons bundled, with the second ALL.

Steps to reproduce

• Create a small application that uses webpack for bundling.
• Render an carbon 10 icon with the first approach: import { Add16 } from "@carbon/icons-react";
• Use the Webpack Bundle Analyzer

Notice: Only a few icons are bundled.

Now:

• Render an carbon 11 icon with the first approach: import { View } from "@carbon/icons-react/next";
• Use the Webpack Bundle Analyzer again

Notice: All C11 icons are bundled.

Suggested Severity

Severity 3 = User can complete task, and/or has a workaround within the user experience of a given component.

Application/PAL

IKC | WatsonX

Code of Conduct

• I agree to follow this project's Code of Conduct
• I checked the current issues for duplicate problems

[Mikadv]

When looking at the TypeScript definition files it looks like this for next

And for the carbon10 one:

Not sure if that is it also in the code.

[wkeese]

The problem is not limited to Carbon V10 or to the @carbon/icons-react/next code. In our product, it's pulling in 2M from @carbon/icons-react/next but almost 9M from @carbon/icons-react. And, I tested that I can almost completely eliminate the @carbon/icons-react/next 2M overhead by simply changing lines like

import { CheckmarkFilled, Rule, WarningAltFilled } from "@carbon/icons-react/next";

to

import CheckmarkFilled from "@carbon/icons-react/next/CheckmarkFilled";
import Rule from "@carbon/icons-react/next/Rule";
import WarningAltFilled from "@carbon/icons-react/next/WarningAltFilled";

I looked at @carbon/icons-react's index.esm.js and didn't see any bugs. However, the problem is that Webpack's tree shaking works really poorly, specifically, when it includes a file, it's not trimming unused functions. Therefore, bundling multiple icons into a single file is problematic.

See also https://www.npmjs.com/package/babel-plugin-lodash which addresses the identical problem with lodash.

I see from https://github.com/carbon-design-system/carbon/blob/main/packages/icon-build-helpers/src/builders/react/builder.js#L75 that you are intentionally combining icons into bundles because

// In the past, we tried re-exporting from the source
// but this ended up causing slowdowns due to module resolution...
// a re-export strategy ... takes too
// long due to the number of modules along with the time it takes tooling to
// process them.

However, since you no longer have a separate component for each size (Add16, Add32, etc.) there are only about 2000 components now, rather than 5000. And hopefully, tooling has improved since you decided you needed to do the bundle thing.

So, I suggest you switch back to just exporting each icon individually, as a separate file, where the index.js just references all those files.

[wkeese]

cc @joshblack @tw15egan ^^^ since they committed that bundling code

[Mikadv]

So, I suggest you switch back to just exporting each icon individually, as a separate file, where the index.js just references all those files.

This!

cc @tay1orjones

[tay1orjones]

@wkeese @Mikadv I'm not opposed to someone looking at all of this again, but this was discussed and explored at length in the run up to the v11 release,

• feat(icon-build-helpers): add module-info extension and next react builder #8189

At the time it was only ~1650 modules and the same issues cropped up. It backed up the findings of the earlier exploration in #6390 (comment)

To my knowledge these blockers still exist and are why new solutions like unplugin-icons are popping up that take the alternative approach of configuring your bundler to dynamically import only the icons you use, rather than relying on native tree-shaking.

As you found, right now the best way to ensure only the icons you use are included in your bundle is to import them with a fully qualified path, which bypasses the buckets.

import CheckmarkFilled from "@carbon/icons-react/es/CheckmarkFilled";

---

Some additional concerns/considerations with regard to icons and tree shaking that I found while researching past attempts at this:

• Terser is very picky when it comes to actually tree shaking and dead code eliminating unused modules: A flat carbon-components-react entrypoint is not tree-shakeable #5442
• We may eventually want to explore using a component generator function which would return a valid component: [Bug]: tree shaking no longer works for icon imports #11543 (comment)
• propTypes and defaultProps are considered side effects: fix(icon-build-helpers): wrap propTypes in __DEV__ check #11569

[wkeese]

Wow, thanks for all the information.

At the time it was only ~1650 modules and the same issues cropped up. It backed up the findings of the earlier exploration in #6390 (comment)

Oh, that's disappointing. So you still hit problems due to bugs/limitations in Webpack, Terser, Babel, etc., both with:

• the standard re-export design for libraries (every icon in a separate file, with one index.js that re-exports all of them)
• concatenating all the icons into one index.js file

I wonder if you tried something like this:

index.js:

export * from "./index_a";
export * from "./index_b";
export * from "./index_c";
...

index_a.js:

export * from "./AAA";
export * from "./AAB";
...

propTypes and defaultProps are considered side effects: #11569

I don't quite understand why PropTypes is considered a side-effect (and hence this is a Terser "limitation" rather than a "bug"). I guess it doesn't matter.

I don't know why #11569 didn't solve the problem for us, but it looks like we can workaround the issue by using babel-plugin-transform-react-remove-prop-types. Everyone should be using that babel plugin anyway because it removes code bloat in general.

Anyway, it sounds like there's nothing Carbon can do here, except possibly my suggestion above to have index_a.js, index_b.js etc. index files, without any actual concation. In any case, I have no objection to closing the ticket, but @Mikadv's is the one that originally opened it. As for me, I've never claimed or thought this was a bug in Carbon.

[tay1orjones]

Yeah, one solution for the problem described at the top of the issue is to import icons with a fully qualified path to bypass the buckets.

I'm happy to reopen this if someone wants to look into that approach you describe. It's likely to suffer from the same performance issue with Node's module resolution. Although maybe the performance has (or will) improved in newer versions of Node.

[wkeese]

For the record, here's what I'm doing for Carbon 10, to automatically convert import { ... } from "@carbon/icons-react" statements to import file-by-file.

npm install -D glob babel-plugin-transform-imports

and then at the top of .babelrc.js:

const glob = require("glob");

// Map from @carbon/icons-react V10 icon name to the file for that icon.  Keys are lowercase, with underscores removed.
// Complicated because @carbon/icons-react's directory naming is somewhat random, especially w.r.t. using one or two
// dashes, and also because there's an extra level of nesting under the watson-health/ and Q/ directories.
// Ex: WarningAltInvertedFilled16 —> warning--alt-inverted--filled/16.js
// Ex: WatsonHealthLaunchStudy_132 --> watson-health/launch-study--1/32.js
const carbon10IconNameToFile = Object.fromEntries(
  glob
    .sync("**/{16,20,24,32}.js", {
      cwd: "node_modules/@carbon/icons-react/lib",
      posix: true,
    })
    .map((path) => [
      path.replace(/[-/]/g, "").replace(/\.js$/, "").toLowerCase(),
      `@carbon/icons-react/lib/${path}`,
    ])
);

// Map from @carbon/icons-react/next icon name to the file for that icon.  Keys are lowercase, with underscores removed.
// Complicated because there's an extra level of nesting under the watson-health/ and Q/ directories.
// Ex: WatsonHealthLaunchStudy_1 --> watson-health/LaunchStudy_1.js
const carbon11IconNameToFile = Object.fromEntries(
  glob
    .sync("**/*.js", {
      cwd: "node_modules/@carbon/icons-react/next",
      posix: true,
    })
    .map((path) => [
      path.replace(/[-/]/g, "").replace(/\.js$/, "").toLowerCase(),
      `@carbon/icons-react/next/${path}`,
    ])
);

And finally, in the plugins section, add:

plugins: [
    [
    "transform-imports",
    // Workaround tree-shaking problem where unused icons are included in bundles.
    {
      "@carbon/icons-react": {
        transform: (iconName) => {
          const file =
            carbon10IconNameToFile[iconName.replace(/_/g, "").toLowerCase()];
          if (!file) throw new Error(`Can't find file for icon ${iconName}`);
          return file;
        },
        preventFullImport: true,
      },
      "@carbon/icons-react/next": {
        transform: (iconName) => {
          const file =
            carbon11IconNameToFile[iconName.replace(/_/g, "").toLowerCase()];
          if (!file) throw new Error(`Can't find file for icon ${iconName}`);
          return file;
        },
        preventFullImport: true,
      },
    },
  ],
  ...
]

I think the babel-plugin-transform-react-remove-prop-types thing is a red herring because there are no PropTypes in Carbon 10's @carbon/icons-react, except for in @carbon/icons-react-next, and yet @carbon/icons-react has the same problem.