react-bootstrap-icons icon indicating copy to clipboard operation
react-bootstrap-icons copied to clipboard

Why not use a single component with content or prop as icon?

Open Devmond opened this issue 4 years ago • 12 comments

Instead of constructing potentially hundreds of individual components to represent each icon, why not a single component that uses either a prop or content for the icon name?

We build a low-code developer tool hence the perspective that using a single component would be a much better approach.

Devmond avatar Jun 09 '20 22:06 Devmond

Hi @Devmond, how would you do that? Do you have an example to show me?

ismamz avatar Jun 09 '20 23:06 ismamz

Hi @ismamz you can checkout font awesome: https://github.com/FortAwesome/react-fontawesome

they implement it using a single icon component then use a prop to select a specific icon.

Devmond avatar Jun 09 '20 23:06 Devmond

This library is highly inspired by react-feather. Bootstrap has more icons and therefore this module is bigger.

The approach that you propose is super valid, but I think that there are two different paths and each one has their pros and cons.

However, I think that react-feather and this package has a simpler API. In your final bundle you will only include the icons you use and they are simple React components (SVG) with no dependencies.

I'm open to listen to any proposal of how to even mix these two approaches in the same library.

ismamz avatar Jun 10 '20 13:06 ismamz

I'm with @Devmond on this one. I also prefer react-fontawesome approach because it allows me to abstract the icons components more easily. Like this:

const Icon = (props: Props) => {
  const { icon, className: iconClasses, ...otherProps } = props;
  return (
    <FontAwesomeIcon
      {...(otherProps as any)}
      icon={icon as any}
      className={iconClasses}
      fixedWidth
    />
  );

In this case I'm only rendering fontawesome icons but I could easily pass a bootstrapIcon prop to tell to render a bootstrap icon instead, where I'd have a <BootstrapIcon /> component responsible for the rendering.

rasgo-cc avatar Nov 05 '20 19:11 rasgo-cc

@rasgo-cc

because it allows me to abstract the icons components

No problem to do that with react-bootstrap-icons approach, this works perfectly:

// File Icon.tsx

import React from 'react';
import * as bootstrapIcons from 'react-bootstrap-icons';

// FIXME https://github.com/ismamz/react-bootstrap-icons/issues/14
interface BootstrapIconProps extends React.SVGAttributes<SVGElement> {
  color?: string;
  size?: string | number;
}

interface Props extends BootstrapIconProps {
  // Cannot simply use "name" as it is a valid SVG attribute
  // "iconName", "filename", "icon"... will do it instead
  iconName: keyof typeof bootstrapIcons;
}

export function Icon({ iconName, ...props }: Props) {
  const BootstrapIcon = bootstrapIcons[iconName];
  return <BootstrapIcon {...props} />;
}
import { Icon } from './Icon';
...
<Icon iconName="Stopwatch" className="align-top" />
...

tkrotoff avatar Nov 06 '20 08:11 tkrotoff

Oh nice! Thanks @tkrotoff! I take back my point on the comment above 😄

rasgo-cc avatar Nov 06 '20 10:11 rasgo-cc

btw in a similar way, here a trick when testing with Jest:

// File src/__mocks__/react-bootstrap-icons.tsx

import React from 'react';
import * as icons from 'react-bootstrap-icons';

function mockIcon({ iconName }: { iconName: string }) {
  return <span>{iconName}</span>;
}

Object.keys(icons).forEach(iconName => {
  (icons as any)[iconName] = () => mockIcon({ iconName });
});

module.exports = icons;

You will output <span>Stopwatch</span> in the DOM instead of the whole SVG. This simplifies Jest snapshots and is easy to test with react-testing-library: screen.getByText('Stopwatch').

tkrotoff avatar Nov 07 '20 12:11 tkrotoff

Hi @ismamz. Thanks for your work! Regretfully, this project is not so useful as it might be. Some thoughts of mine...

The project has to keep up with the original Bootstrap icons. Every change from them requires update here. In a time you'll loose an interest in doing this.

It's better to create Webpack plugin that ~somehow~ generates the icons required in the project directly from Bootstrap content. How to? Lets see...

So, we want a general component, common for every icon. We pass the icon name as a property. During the build Webpack does not statically know what icons do we actually use. Bootstrap has SVG sprite image with every of 1200+ icons that weights 600k. Also, there are various fonts around, Bootstraps' WOFF2 takes 80k.

But what if we actually use 10 icons from 1200. Have we to serve huge SVG sprites or fonts? The best way is to create own SVG sprite from a list of icon names. Just add icon name to JSON file of your project. Take the same JSON file for icon name property validation. That's it.

I'm going to use svg-spritemap-webpack-plugin for this.

AntonBaukin avatar Jan 22 '21 18:01 AntonBaukin

Hi @AntonBaukin, I appreciate your feedback!

About keeping sync between this library and the original Bootstrap icons, I agree with you that we need to find some consistent solution for that.

I think that the sprite solution is a good idea, also the single component approach we will probably finish adding soon.

Let me know if you have progress on that idea 👍

ismamz avatar Jan 25 '21 17:01 ismamz

I think @Devmond made a really good suggestion. The code that @tkrotoff wrote works but is really bad for bundle size because it causes Webpack to include all of the components.

johannpayer avatar Apr 01 '21 17:04 johannpayer

Actually, I think that bundle size would be an issue regardless of whether there was a single component or multiple. To fix the bundle size issue, you would either have to continue using a key prop but lazy load the components or you could replace the key prop with an Icon prop that takes in a React component and uses React.createElement to create the element. I chose the second approach for my website and saw a massive decrease in the bundle size of about 117KB.

johannpayer avatar Apr 01 '21 18:04 johannpayer

Actually, I think that bundle size would be an issue regardless of whether there was a single component or multiple. To fix the bundle size issue, you would either have to continue using a key prop but lazy load the components or you could replace the key prop with an Icon prop that takes in a React component and uses React.createElement to create the element. I chose the second approach for my website and saw a massive decrease in the bundle size of about 117KB.

Could you please share how you did it? The exact code will be much appreciated. I faced the same bundle size problem in my project and I don't know how to solve it since I'm new to web development.

woodrunsdeep avatar Apr 07 '23 17:04 woodrunsdeep