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

A way to only bundle icons you actually use #2193

Closed
jrmyio opened this issue Mar 2, 2018 · 34 comments
Closed

A way to only bundle icons you actually use #2193

jrmyio opened this issue Mar 2, 2018 · 34 comments

Comments

@jrmyio
Copy link

jrmyio commented Mar 2, 2018

Is there currently no way to only bundle the icons you are actually using?

When importing the Icon Component:
import { Popover } from '@blueprintjs/core';

It adds a whopping ~350kb to your build because the entire iconSVGPaths.js is included. Tree shaking with Webpack 4 doesn't fix this.

Is this intended? Are there any workarounds to only bundle icons actually used?

@adidahiya
Copy link
Contributor

This is intended in 2.0, yes. We considered a few different architectures for SVG icons and decided on the current one for the best upgrade story (sorry we didn't share the RFCs for @blueprintjs/icons externally; we'll try to do so in the future).

I would be open to ideas for changing the architecture if you can get webpack 4 tree shaking to work in to eliminate unused icons from your bundle -- there is still some time for breaking changes because we haven't released 2.0.0 yet.

@reiv
Copy link
Contributor

reiv commented Apr 1, 2018

I've been thinking about this a bit, and I think this is an approach that could work:

The idea would be to split up the icons into separate files; each file would be named after the icon and export just the path data needed to render that specific icon, and the icon name, at the module level.

// asterisk.ts
export name = "asterisk";
export path16 = ["M14.54 11.18l.01-.02L9.8 ..."]
export path20 = ["M18.52 14.171.01-.02L11.89 ..."]

Such a module could be described by an interface, like:

export interface IconDefinition {
    /** Icon description for a11y */
    name: string;
    /** SVG paths */
    path16: string[];
    path20: string[];
}

These icons could live in the icons package. A separate all.ts would simply be a re-export of every one of these individual files.

The Icon component in @blueprintjs/core would need to be expanded to also accept IconDefinition:

export interface IIconProps extends IIntentProps, IProps {
    ...
    icon: IconName | IconDefinition | JSX.Element | false | null | undefined;
    ...
    

To use the icon, one would import the file directly, e.g. import * as Asterisk from "@blueprintjs/icons/asterisk"; for convenience, one could write a dedicated icons.ts which would just export all icons used in that project.

Finally, there would need to be a way to prevent the entirety of the icons package from being required by default (I believe this happens as soon as Icon is imported, either directly or as part of a component that uses it). One way might be to wrap the import in an if expression that checks for some variable being set, like so:

// in Icon.tsx

import * as _allIcons from "@blueprintjs/icons/all";
let allIcons: typeof _allIcons | undefined = undefined;

if (!BLUEPRINT_INDIVIDUAL_ICONS) {
    allIcons = require("@blueprintjs/icons/all");
}

I believe this is enough for Webpack's tree shaking to kick in. The variable could either be global or added by Webpack's DefinePlugin.

Of course, not pulling in all icons at once means that the IconName form for the icon prop can no longer be used, so a runtime error should be issued if icon is specified as a string.

For maximum convenience, a Babel plugin could automagically transform string-form icon props into the corresponding imports (this is how lodash does it, for example.)

Since all this is purely additive, there wouldn't be any breaking changes.

Would love to hear your thoughts on this.

@giladgray giladgray added this to the 3.0 milestone Apr 3, 2018
@reiv
Copy link
Contributor

reiv commented Apr 3, 2018

Perhaps splitting up the icons into individual files is a little overkill (it does seem to add a lot of overhead for Webpack too!) -- just providing the icons as individual exports rather than in continuous arrays would probably do the trick as well. Still trying things out on my end, but I'll PR as soon as I have something workable.

@giladgray
Copy link
Contributor

this does not require API breaks so it will not happen in 3.0.0

@parisholley
Copy link

@ConneXNL I worked around this in webpack by swapping the file with a custom version that only includes the icons I need:

new webpack.NormalModuleReplacementPlugin(
        /iconSvgPaths.js/,
        `${root}/iconSvgPaths.js`
)

@Ameobea
Copy link

Ameobea commented Aug 21, 2018

@parisholley I've tried this on my own project, but I wasn't able to get it working. I've tried tweaking the regex to /.*iconSvgPaths.[jt]s/ in an attempt to match correctly, but nothing seems to be working.

Did you make any other changes to your webpack conf to make this work?

@parisholley
Copy link

@Ameobea nope, just follow w/e instructions exist for the webpack version you are using

https://webpack.js.org/plugins/normal-module-replacement-plugin/

i'm on the latest everything atm

@crysehillmes
Copy link

@Ameobea Use /.*\/generated\/iconSvgPaths.*/.

@jasonbarry
Copy link

Here's how I got it working with a next.js project. Also had to install file-loader for the build to succeed. Main bundle is 211KB smaller now.

// /next.config.js
const path = require('path');
const { NormalModuleReplacementPlugin } = require('webpack');
// ...
config.plugins.push(
  new NormalModuleReplacementPlugin(
    /.*\/generated\/iconSvgPaths.*/,
    path.resolve(__dirname, 'test/emptyModule.js'),
  )
)
// /test/emptyModule.js
export default '';

@nucab
Copy link

nucab commented Dec 18, 2018

@jasonbarry I got this error. Did you encounter this error?

Attempted import error: 'IconSvgPaths16' is not exported from './generated/iconSvgPaths'.

@ernestofreyreg
Copy link

ernestofreyreg commented Jan 11, 2019

Solved for nextjs by:

// /next.config.js
const path = require('path')
const { NormalModuleReplacementPlugin } = require('webpack')

module.exports = {
  webpack: (config, { buildId, dev, isServer, defaultLoaders }) => {
    config.plugins.push(
      new NormalModuleReplacementPlugin(
        /.*\/generated\/iconSvgPaths.*/,
        path.resolve(__dirname, 'lib/icons.js'),
      )
    )

    return config
  }
}
// /lib/icons.js

export const IconSvgPaths16 = {
    "cross": ["M9.41 8l3.29-3.29c.19-.18.3-.43.3-.71a1.003 1.003 0 0 0-1.71-.71L8 6.59l-3.29-3.3a1.003 1.003 0 0 0-1.42 1.42L6.59 8 3.3 11.29c-.19.18-.3.43-.3.71a1.003 1.003 0 0 0 1.71.71L8 9.41l3.29 3.29c.18.19.43.3.71.3a1.003 1.003 0 0 0 .71-1.71L9.41 8z"],
    "lock": ["M13.96 7H12V3.95C12 1.77 10.21 0 8 0S4 1.77 4 3.95V7H1.96c-.55 0-.96.35-.96.9v6.91c0 .54.41 1.19.96 1.19h12c.55 0 1.04-.65 1.04-1.19V7.9c0-.55-.49-.9-1.04-.9zM6 7V3.95c0-1.09.9-1.97 2-1.97s2 .88 2 1.97V7H6z"],
    "user": ["M7.99-.01A7.998 7.998 0 0 0 .03 8.77c.01.09.03.18.04.28.02.15.04.31.07.47.02.11.05.22.08.34.03.13.06.26.1.38.04.12.08.25.12.37.04.11.08.21.12.32a6.583 6.583 0 0 0 .3.65c.07.14.14.27.22.4.04.07.08.13.12.2l.27.42.1.13a7.973 7.973 0 0 0 3.83 2.82c.03.01.05.02.07.03.37.12.75.22 1.14.29l.2.03c.39.06.79.1 1.2.1s.81-.04 1.2-.1l.2-.03c.39-.07.77-.16 1.14-.29.03-.01.05-.02.07-.03a8.037 8.037 0 0 0 3.83-2.82c.03-.04.06-.08.09-.13.1-.14.19-.28.28-.42.04-.07.08-.13.12-.2.08-.13.15-.27.22-.41.04-.08.08-.17.12-.26.06-.13.11-.26.17-.39.04-.1.08-.21.12-.32.04-.12.08-.24.12-.37.04-.13.07-.25.1-.38.03-.11.06-.22.08-.34.03-.16.05-.31.07-.47.01-.09.03-.18.04-.28.02-.26.04-.51.04-.78-.03-4.41-3.61-7.99-8.03-7.99zm0 14.4c-1.98 0-3.75-.9-4.92-2.31.67-.36 1.49-.66 2.14-.95 1.16-.52 1.04-.84 1.08-1.27.01-.06.01-.11.01-.17-.41-.36-.74-.86-.96-1.44v-.01c0-.01-.01-.02-.01-.02-.05-.13-.09-.26-.12-.39-.28-.05-.44-.35-.5-.63-.06-.11-.18-.38-.15-.69.04-.41.2-.59.38-.67v-.06c0-.51.05-1.24.14-1.72.02-.13.05-.26.09-.39.17-.59.53-1.12 1.01-1.49.49-.38 1.19-.59 1.82-.59.62 0 1.32.2 1.82.59.48.37.84.9 1.01 1.49.04.13.07.26.09.4.09.48.14 1.21.14 1.72v.07c.18.08.33.26.37.66.03.31-.1.58-.16.69-.06.27-.21.57-.48.62-.03.13-.07.26-.12.38 0 .01-.01.04-.01.04-.21.57-.54 1.06-.94 1.42 0 .06.01.13.01.19.04.43-.12.75 1.05 1.27.65.29 1.47.6 2.14.95a6.415 6.415 0 0 1-4.93 2.31z"],
    "warning-sign": ["M15.84 13.5l.01-.01-7-12-.01.01c-.17-.3-.48-.5-.85-.5s-.67.2-.85.5l-.01-.01-7 12 .01.01c-.09.15-.15.31-.15.5 0 .55.45 1 1 1h14c.55 0 1-.45 1-1 0-.19-.06-.35-.15-.5zm-6.85-.51h-2v-2h2v2zm0-3h-2v-5h2v5z"],
}

export const IconSvgPaths20 = {
    "cross": ["M11.41 10l4.29-4.29c.19-.18.3-.43.3-.71a1.003 1.003 0 0 0-1.71-.71L10 8.59l-4.29-4.3a1.003 1.003 0 0 0-1.42 1.42L8.59 10 4.3 14.29c-.19.18-.3.43-.3.71a1.003 1.003 0 0 0 1.71.71l4.29-4.3 4.29 4.29c.18.19.43.3.71.3a1.003 1.003 0 0 0 .71-1.71L11.41 10z"],
    "lock": ["M15.93 9H14V4.99c0-2.21-1.79-4-4-4s-4 1.79-4 4V9H3.93c-.55 0-.93.44-.93.99v8c0 .55.38 1.01.93 1.01h12c.55 0 1.07-.46 1.07-1.01v-8c0-.55-.52-.99-1.07-.99zM8 9V4.99c0-1.1.9-2 2-2s2 .9 2 2V9H8z"],
    "user": ["M10 0C4.48 0 0 4.48 0 10c0 .33.02.65.05.97.01.12.03.23.05.35.03.2.05.4.09.59.03.14.06.28.1.42l.12.48c.05.16.1.31.15.46.05.13.09.27.15.4.06.16.13.32.21.48.05.11.1.22.16.33.09.17.17.34.27.5.05.09.1.17.15.25.11.18.22.35.34.52.04.06.08.11.12.17 1.19 1.62 2.85 2.86 4.78 3.53l.09.03c.46.15.93.27 1.42.36.08.01.17.03.25.04.49.07.99.12 1.5.12s1.01-.05 1.5-.12c.08-.01.17-.02.25-.04.49-.09.96-.21 1.42-.36l.09-.03c1.93-.67 3.59-1.91 4.78-3.53.04-.05.08-.1.12-.16.12-.17.23-.35.34-.53.05-.08.1-.16.15-.25.1-.17.19-.34.27-.51.05-.11.1-.21.15-.32.07-.16.14-.32.21-.49.05-.13.1-.26.14-.39.05-.15.11-.31.15-.46.05-.16.08-.32.12-.48.03-.14.07-.28.1-.42.04-.19.06-.39.09-.59.02-.12.04-.23.05-.35.05-.32.07-.64.07-.97 0-5.52-4.48-10-10-10zm0 18a7.94 7.94 0 0 1-6.15-2.89c.84-.44 1.86-.82 2.67-1.19 1.45-.65 1.3-1.05 1.35-1.59.01-.07.01-.14.01-.21-.51-.45-.93-1.08-1.2-1.8l-.01-.01c0-.01-.01-.02-.01-.03a4.42 4.42 0 0 1-.15-.48c-.33-.07-.53-.44-.61-.79-.08-.14-.23-.48-.2-.87.05-.51.26-.74.49-.83v-.08c0-.63.06-1.55.17-2.15.02-.17.06-.33.11-.5.21-.73.66-1.4 1.26-1.86.62-.47 1.5-.72 2.28-.72.78 0 1.65.25 2.27.73.6.46 1.05 1.12 1.26 1.86.05.16.08.33.11.5.11.6.17 1.51.17 2.15v.09c.22.1.42.33.46.82.04.39-.12.73-.2.87-.07.34-.27.71-.6.78-.04.16-.09.33-.15.48 0 .01-.02.05-.02.05-.26.71-.67 1.33-1.17 1.78 0 .08.01.16.01.23.05.54-.15.94 1.31 1.59.81.36 1.84.74 2.68 1.19A7.958 7.958 0 0 1 10 18z"],
    "warning-sign": ["M19.86 17.52l.01-.01-9-16-.01.01C10.69 1.21 10.37 1 10 1s-.69.21-.86.52l-.01-.01-9 16 .01.01c-.08.14-.14.3-.14.48 0 .55.45 1 1 1h18c.55 0 1-.45 1-1 0-.18-.06-.34-.14-.48zM11 17H9v-2h2v2zm0-3H9V6h2v8z"],
}

This way you can pick and choose the icons you actually use from the generated file.

@01dr
Copy link

01dr commented Apr 1, 2019

Hey! Is there any temporary solution to completely exclude icons from the bundle in create-react-app without ejecting?

@Ameobea
Copy link

Ameobea commented Apr 1, 2019

@amazing-space-invader There are some hacky things you can do:

You can fork the blueprint repo, deleting all of the unused icons from the source code, and pull in the fork as a dependency instead.

Or you can just add a step to your build process that overwrites the icons file in your node_modules directory with a custom file that has all of the unused icons removed.

@silverwind
Copy link

silverwind commented Jul 1, 2022

Did anyone figure out how to exclude all icons via vite and/or rollup?

@justinbhopper
Copy link
Contributor

But I'd like also provide the option to leverage webpack/rollup tree shaking. A package global singleton approach seems like it could work:

import { Icons } from "@blueprintjs/icons";

// for access to all icons across BP packages
Icons.loadAll();

// for limited access to specific icons
Icons.load([
  "refresh",
  ...
]);

but that will be difficult to enforce safely in the type system.

One option to consider is requiring that consumers of BlueprintJS include a .d.ts reference (like Create React App or Vite.js do) to import the type definitions. This way, the definition of any control that has icon props can be extended to include the IconName type definition or not, depending on whether they plan to call loadAll() or their own custom set.

For example

@blueprintjs/icons index.d.ts (loaded by default)

export type IconReference = MaybeElement; // Only supports direct SVGs or elements by default

@blueprintjs/icons all.d.ts (opt-in only, see below)

declare type AllIconNames = "chevron-left" | "chevron-right" | etc;
declare type IconReference = MaybeElement | AllIconNames;

@blueprintjs/core InputGroup.ts

export interface InputGroupProps {
  icon?: IconReference; 
}

In a consumer app that wants all icons bundled (see all.d.ts above):
my-app/blueprintjs-icons.d.ts

/// <reference types="@blueprintjs/icons/all" />

In a consumer app that wants only specific icon:
my-app/blueprintjs-icons.d.ts

declare module "@blueprintjs/icons" {
  type IconReference = MaybeElement | "comment" | "person";
}

This has some key benefits:

  1. The breaking change is minimal. Users who want the old behavior only need to define a new .d.ts file that has a single line.
  2. Users who want fine control can have it.
  3. There's even an option for users who only want the import-style and don't ever want to use the IconName union type feature.

You could go further with this by creating a new package called @blueprintjs/icons-bundled (or @types/blueprintjs-all-icons?) which automatically defines the extension to IconReference automatically.

@ro-savage
Copy link

For anyone using Blueprint 4.x - this is the update code to ignore the icons.

Update your webpack config to

    new NormalModuleReplacementPlugin(
      /.*\/@blueprintjs\/icons\/lib\/esm\/iconSvgPaths.*/,
      resolve(__dirname, "../../app/javascript/blueprint-icons.js"),
    ),

Then inside your blueprint-icons.js add

Either this if you are going to keep using the old warning-sign naming

export function iconNameToPathsRecordKey(name) {
  return name
}

or this if you want to rename all your already imported icons to PascalCase

function toPascalCase(string) {
  return `${string}`
    .toLowerCase()
    .replace(new RegExp(/[-_]+/, 'g'), ' ')
    .replace(new RegExp(/[^\w\s]/, 'g'), '')
    .replace(
      new RegExp(/\s+(.)(\w*)/, 'g'),
      ($1, $2, $3) => `${$2.toUpperCase() + $3}`
    )
    .replace(new RegExp(/\w/), s => s.toUpperCase());
}

export function iconNameToPathsRecordKey(name) {
  return toPascalCase(name);
}

@switz
Copy link
Contributor

switz commented Aug 19, 2022

This ignores the icons, but it doesn't dynamically import/code split them, unless I'm misunderstanding how you're doing this.

dlech added a commit to pybricks/pybricks-code that referenced this issue Oct 27, 2022
This follows the suggestions from [1] to reduce the package size by
only including the icons we are actually using. This gets the main
module small enough to avoid the CRA "The bundle size is significantly
larger than recommended." warning (gzipped size is < 512KB).

[1]: palantir/blueprint#2193
dlech added a commit to pybricks/pybricks-code that referenced this issue Oct 27, 2022
This follows the suggestions from [1] to reduce the package size by
only including the icons we are actually using. This gets the main
module small enough to avoid the CRA "The bundle size is significantly
larger than recommended." warning (gzipped size is < 512KB).

[1]: palantir/blueprint#2193
@adidahiya
Copy link
Contributor

@blueprintjs/core & @blueprintjs/icons v5.0.0-alpha.1 have support for tree shaking and dynamically importing icons. Please try it out and let me know how it goes.

@justinbhopper's idea in #2193 (comment) looks interesting; I will try that out but no guarantees that I'll be able to include this additional feature of statically declaring a subset of available icons in type definitions in the v5.0 release. It might get punted to v6.0.

@justinbhopper
Copy link
Contributor

justinbhopper commented May 10, 2023

@adidahiya The new version generates components for each icon, but each of these components contains an entire SVG structure. Wouldn't it be easier if they only generated a React Fragment that contained the <path> element?

That way, most of the SVG structure (and all its custom behavior like attributes, classnames, title element, etc) can remain in the control of the <Icon> component.

In fact, I'm not entirely sure I understand why a component had to be pre-generated in the first place. Why not just dynamically import the path data?

@justinbhopper
Copy link
Contributor

@adidahiya I looked up your original PR #4513 and realize one of your goals was to have the icons be exportable as their own individual component, e.g. import { Download } from "@blueprintjs/icons", which explains why you chose to have a component be pre-generated. So please ignore the comment above.

I think it's likely there are consumers of the old IconSvgPaths16 and IconSvgPaths20 exports (our team is one of them). I can give details on why we used the paths directly, but that's probably not relevant here.

Could we make it possible to still retrieve the path data using the new Icons icons loader? e.g. Icons.getPath(icon: IconName).

@adidahiya
Copy link
Contributor

Statically loading all icon paths

You can currently access icon paths like this (in both v4.x and v5.0) where all icons paths will be imported into your bundle:

import { IconSvgPaths16, iconNameToPathsRecordKey } from "@blueprintjs/icons";

IconSvgPaths16[iconNameToPathsRecordKey("caret-down")];

Admittedly this is a little verbose, and it would be nicer to have an API like this:

import { getIconPath } from "@blueprintjs/icons";

getIconPath("caret-down", 16);

I'll add that new function in v5.0.

Dynamically loading icon paths

Could we make it possible to still retrieve the path data using the new Icons icons loader? e.g. Icons.getPath(icon: IconName).

Yes, it would be possible to add a new API to IconLoader which loads icon path modules dynamically. I will consider adding Icons.getPath(icon: IconName) in v5.0.

Note that this will require adding another customization option to IconLoader so that users can load the path modules with non-default webpack import options and in other bundlers which are not webpack (example: #6152).

@blueprintjs/icons package size

I noticed that the new icons build system results in a pretty large NPM tarball with a ton of files in v5.0:

npm notice === Tarball Details ===
npm notice name:          @blueprintjs/icons
npm notice version:       5.0.0-alpha.4
npm notice filename:      blueprintjs-icons-5.0.0-alpha.4.tgz
npm notice package size:  3.6 MB
npm notice unpacked size: 22.6 MB
npm notice shasum:        522c11c99d6efcb7bb31f11c14bbbcf94950d6b8
npm notice integrity:     sha512-hxBOhQbaXdRya[...]U8tYZvZUIrb6w==
npm notice total files:   17640

This is the result of attempting to support many different ways to export tree-shakable icon components & paths. I don't have a concrete plan right now to mitigate this tarball size or number of files, but just wanted to put it out there that I'm aware of this size inflation.

@adidahiya
Copy link
Contributor

I decided to change the icon loading API in #6212 based on @justinbhopper's suggestion. Icons.load() now loads paths instead of components, and shared logic for rendering the <svg> element container now lives in <SVGIconContainer>.

Bundle size is slightly reduced after de-duplicating some of the generated code:

npm notice === Tarball Details === 
npm notice name:          @blueprintjs/icons                      
npm notice version:       5.0.0-beta.0                            
npm notice filename:      blueprintjs-icons-5.0.0-beta.0.tgz      
npm notice package size:  3.5 MB                                  
npm notice unpacked size: 18.4 MB                                 
npm notice shasum:        2c73c2de5826b1a4e446c7d2eec486d0ebc62fa2
npm notice integrity:     sha512-IDgwYepbxLgCL[...]m+GBvXeudfT/Q==
npm notice total files:   17650 

I'm going to close this out as complete, and I'm signaling that v5.0 is basically done with the 5.0.0-beta.0 version I just released. We can discuss further improvements to icons in new issue threads.

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

Successfully merging a pull request may close this issue.