Introduce Icons package

[senadir]

Description

closes #9647
This PR is a preparation for #16484
there has been a lot of talk of standardizing the current approach about using some standard icon library & pattern of use.
the main reason I needed this is to build a searchable-filterable icon block, the current approach uses a switch case for each icon, making them impossible to filter or getting them as a list.

while this package only includes the icons in Dashicon, it helps us pave the way to an easier experience adding icons.

the build process will be expanded in the future to easily include icons from svg files directly (the current approach uses an iconObject object that contain the svg paths with names, essentially turning the switch case to a traversable object.

This should replace all future uses of Dashicon component, and graduilty deprecate it & replacing it with this.

How has this been tested?

• run npm install
• include any icon you want from icons

import { AdminPost } from '@wordpress/icons';

Notes

the builder is a quick draft, I expect it to be expanded more and would love any feedback in it

Checklist:

• My code is tested.
• My code follows the WordPress code style.
• My code follows the accessibility standards.
• My code has proper inline documentation.
• I've included developer documentation if appropriate.

[jasmussen]

Thank you! Thank you very much for taking a stab at this. There has been such a desire for an easier way to use icons, both for users in the block editor, but for developers too. Additionally there have been discussions on ways to expand and refresh the Dashicons set, perhaps add additional sets.

while this package only includes the icons in Dashicon, it helps us pave the way to an easier experience adding icons.

I love the sound of that. It's definitely clear we need access to a ton of icons, but we also need to reduce the bottleneck it is to add new icons. Social logos for "sharing" blocks is another candidate that might benefit from this component.

So in that vein: this PR feels necessary and appropriate. Some questions, though:

1. Will this component be able to work with multiple icon sets? For example it needs to work with Dashicons for the moment, but you could imagine it expanding to potentially include icons from other sets such as Material Icons, or FontAwesome, or yet another set, just as examples.

2. In relation to 1, how will a developer include an icon from a specific set? Based on your examples, I imagine it's this way:

import { AdminPost } from '@wordpress/icons';

...

<AdminPost />

Provided I understood the above right, it would output the admin-post.svg from the Dashicons set. However this feels very tied to the Dashicons set specifically, and the component names seem very arbitrary. It feels like multiple components, rather than a single Icon component.

Would it be possible to work towards the following syntax instead?

<Icon src="post" set="dashicons" /> // specifies a specific set
<Icon src="post" /> // picks whatever "post" icon is available
<Icon src="post" set="material" /> // picks the "post" icon from the material set
<Icon src="post" set="material" type="font" /> // picks the "post" icon from the material set, outputs it as an icon font sprite set, svg would be another option

I'll defer to you, Riad, and others for the technical feasibility of the precise syntax. But the point I want to make is that if we can add some sort of system that better scales to multiple icon sets, that seems important.

3. Different icon sets have different dimensions. Icons from Dashicons are 20x20px, icons from the Material set are 24x24px. I assume that this component doesn't touch any of those dimensions, and simply outputs the SVG with the dimensions it's born with. This would be fine. What we want to avoid is outputting the same fixed dimensions on all icons, regardless of set.

I know those are all big, complicated questions. We don't need to address all of those questions in this PR — but the questions are very important to think about before we move forward, because we have to address them in the future. So we have to build this component keeping those challenges in mind, so we don't paint ourselves into a corner.

[youknowriad]

Some quick thoughts:

• I feel the Icon component should live in this package (like proposed by @jasmussen)

• We should definitely think about scalability. I belive the reason @senadir used separate component/files for each icon is to introduce the possibility to "tree-shake" the module: only bundle the icons that are used instead of including the whole set when you use the package. But, this is only true if you don't consider this package as an external, as its own script.

I do wonder whether we should treat this package as a special case in our packages. I don't think it makes sense to load this package entirely in its own WordPress script/style because it will be too big.

The other approach is to bundle its usage in its consumers (editor, block-editor, block-library....) with tree-shaking enabled. That means each consumer will include the icons it uses in its own script, it also means that if an icon is used in two scripts, it's duplicated. We should measure but I think even with this potential duplication, we'll still end up with way smaller JS loaded in general.

Now, there's a third use-case, it can be thought of as a follow-up but it's good to keep in mind too. How do we allow third-party usage of these icons if this package is not available as a WordPress script. We could encourage third-party plugins to use the npm package and bundling the same way Core does, which IMO is good enough even if it forces these plugins to have a "build tool".

---

Finally, there's another approach we could think of. Have each icon be its own file/script in WordPress. This also solves the scalability issue but it is a lot of burden to setup and it also increases the size of WordPress itself especially if we include a lot of icon sets. For that particular reason, I'd still go with the approach outlined above personally.

[senadir]

1. Will this component be able to work with multiple icon sets?
2. how will a developer include an icon from a specific set?

My Idea (which is not implemented in this PR yet) is to have it this way:

import { AdminPost } from '@wordpress/icons/dashicon';
import { Post } from '@wordpress/icons/material';

I do feel like the components names are a bit all over the place, with dashicon having 288 icon and material over a 1000+ we're bound to create a confusion, so maybe a prefix can help

import { IconAdminPost } from '@wordpress/icons/dashicon';
import { IconPost } from '@wordpress/icons/material';

however, the best solution is to use a single component like you said, but I have concerns regarding tree shaking for a single component that reference other components, a technical question for anyone who have any knowledge in tree shaking:
does it work well with HOCs? essentially I'm thinking we will have a single HOC named Icon that loads other components, the problem is since this is dynamic, I'm not sure we can tree shake it.
I would to hear from @youknowriad and @epiqueras about this

3. Different icon sets have different dimensions
   they all accept the same props as the Dashicon component, so things like size, class & pressed aria are still there, you can pass a variable to it, but if not, it will differ to 20px
   the size is used on viewbox, it should scale properly, we can define other things to each set though.

like @youknowriad said, duplicating components is still the best choice, currently we have 288 icons, in the future they could be 3000, I've seen sets with 4500 icon (material UI), and only less then 1% of them will be used on core, so including them would make sense

[jasmussen]

The tree-shaking issue you mention is definitely the challenge to solve — I definitely understand this from a technical perspective. But it's also crucially important to solve this in a considerate way. I would definitely defer to technical observations for how to do this.

But it would be good to think about creative solutions to this. Because if we go with a "component per icon" approach, we'll be stuck with it forever, and we'll have most literally thousands of components. What if we choose to deprecate a set? If we had <Icon src="confirm"> the deprecation policy could be to replace the old set with the new set, and a developer would not have to do anything.

I trust that the single Icon component might not be possible for us. But before we blaze on with the 3000+ components approach, we have to be absolutely sure. Can we:

• Look at how other JS based systems have handled this?
• Look at other icon system best practices?
• Would it be possible to keep the SVGs as individual files in a folder, and convert them to proper aria-labelled React-friendly icons on the fly? That way we could presumably have the single Icon component, yet still have a stack of separate files on a per-icon basis?
• Is there a spin on the above, where we have a single Icon component that only accepts parameters and loads a different component based on those parameters?

Any idea we might think of, that lets us avoid having a named component per icon would be great. Many of the icons created for Dashicons were created half a decade ago and were not named in a consistent or friendly manner. It feels like clutter to suddenly have a component called <LockDuplicate /> to refer to https://github.com/WordPress/dashicons/blob/master/svg-min/lock-duplicate.svg, which honestly is an icon we should refactor away or deprecate if we can.

[senadir]

some questions for me to understand your approach correctly:

1- how is <Icon src="confirm"> different from import { Confirm } from '@wordpress/icons'
in both ways, us changing the icon will cause no problem to the developer since he will get the a result each time. this is however if we go with a generic icon set that lives in @wordpress/icons and extra icon sets that live in @wordpress/icons/setName

I'm still looking into other possible solutions, but I based this one on material UI solution, having each component as a file, Shopify Polaris also does the same
Polaris
Ants Design system uses this approach too

Polaris use case is closely related to ours, since Polaris is used for both building shopify admin interface, and for third party developers to extend the UI with plugins, so might be able to see why was this a good choice.

I would still investigate more to find the most appropriate way to do this.

[jasmussen]

1- how is different from import { Confirm } from '@wordpress/icons'

To be clear I think the full evaluation of which way to go should come from someone like Riad, as there are layers of technical aspects I may not fully grok.

But to me the difference is in implementation. When building a component that references <Icon src="confirm" /> rather than <Confirm />, (or IconConfirm or whatever naming format we might choose), it's easier to imagine the Icon component, being iterated than 3000 separate components, one for each icon.

[epiqueras]

I haven't fully looked into this, but I also think we should have a single icon component.

About module size, I see two solutions off the top of my head:

• Make the component use dynamic imports to get the icon on-demand.
• Do what https://github.com/FortAwesome/react-fontawesome did and have users explicitly preload icons.

Now that I think about it, we should do both. Allow explicit preloading, but also load it for them dynamically if they don't preload it.

[senadir]

The approach used by font awesome is the same one as Polaris, essentially a HOC named Icon that takes another component, the icon and render it, this is still however require each icon to be its own component, just slightly abstracted.

So this is how Polaris & FontAwesome does it, adapted to our situation:

import { Icon } from '@wordpress/components';
// Our current workflow doesn't allow such imports but we can get around that later.
import { Post, Save } from '@wordpress/icons/dashicon';
import { Phone } from '@wordpress/icons/material';

// ...

<Icon src={ Post } />
<Icon src={ Save } />
<Icon src={ Phone } size={ 24 } />

This is still a bit of a hassle to include each component manually, another approach that uses dynamic imports could be like this:

// index.js
import { Icon } from '@wordpress/components';

// ...

<Icon src="post" />
<Icon src="save" set="dashicon" />
<Icon src="phone" set="material" size={ 24 } />

// components/icon/index.js
import { Lazy } from '@wordpress/element';
import { SVG, Path } from '@wordpress/primitives';

// ...

// dashicon is used as a default for example only
export default function Icon( { src, set = "dashicon", ...props } ) {
    const iconLocation = `@wordpress/icons/${set}/${src}`;
    const path = Lazy( () => import( /* webpackMode: "eager" */ iconLocation ) );
    return (
        <SVG { ...props }><Path d={ path } /></SVG>
    );
}

However, there are many cons to this method:

• It makes the build process very slow
• It create a single file for each icon, so we could result in hundred of files, with an HTTP 1.1 connection, this could be a catastrophe.
• It not stable, so there could be random crushes in the build.

Dynamically importing chunks is useful if we're talking a vendor library, a set of components, on anything that makes the extra HTTP request worth it.
For that, I suggest we go with the first method, statically importing icons is much more predictable & stable.

further reading:
facebook/create-react-app#5940

[epiqueras]

The approach used by font awesome is the same one as Polaris, essentially a HOC named Icon that takes another component, the icon and render it, this is still however require each icon to be its own component, just slightly abstracted.

No, although that is supported, it is not their recommended usage for when you have lots of icons:

https://github.com/FortAwesome/react-fontawesome#usage

It makes the build process very slow

It's OK, because we won't change icons frequently and the result is cached.

It create a single file for each icon, so we could result in hundred of files, with an HTTP 1.1 connection, this could be a catastrophe.

You would preload the icons like in FontAwesome to avoid that.

It not stable, so there could be random crushes in the build.

Why? Dynamic imports are super stable.

[youknowriad]

Make the component use dynamic imports to get the icon on-demand.

Dynamic imports might not be that easy in the WordPress context. How do you configure the URL properly? How do configure your bundler to generate scripts that work across WordPress installations (we don't know the path or the relative path before hand...)

I'd love for us to start using dynamic imports, not only for icons, but also for lazy loading things (say the syntax highlighter...). At the time of writing, we don't have a solution for it though.

[senadir]

No, although that is supported, it is not their recommended usage for when you have lots of icons

because when you're building your own app, you know exactly how many icons you're going to use, we're however, not looking at this from a developer perspective but from fontAwesome perspective, and with the recommended method, building a library, you still have to selectively load each component, so we need to selectively export each one.

the explicit & library methods are just developer experience enhancements but under the cover they're the same.

You would preload the icons like in FontAwesome to avoid that

fontAwesome doesn't preload the icons (in the sense of browser resource preloading) it just suggest that you use a library for easier development experience, but performance wise, it's both static importing.

dynamic importing will be problematic because of the number of files generated, when you generate 300 file, even you preload, you will still have to do 600 trip (a trip to the chunk file & a trip to the actual file), if we go with eager, then it's 300 extra chunk in our main bundle, making all this dynamic importing useless.

code splitting & dynamic importing makes a lot of sense when the chunk file size is big and the number is low, when you have 300 file that are 500byte in size, you end up wasting a lot of resources just to fetch them.

I still stand with statically importing them, we can search more into using dynamic import for Gutenberg core packages thou

[epiqueras]

fontAwesome doesn't preload the icons (in the sense of browser resource

preloading) it just suggest that you use a library for easier development experience, but performance wise, it's both static importing. Yes, I was referring to that. It still lets the consumers have a single Icon component with a string prop, instead of having to import multiple icons everywhere.

…

On Fri, Aug 16, 2019 at 4:04 PM Seghir Nadir ***@***.***> wrote: No, although that is supported, it is not their recommended usage for when you have lots of icons because when you're building your own app, you know exactly how many icons you're going to use, we're however, not looking at this from a developer perspective but from fontAwesome perspective, and with the recommended method, building a library, you still have to selectively load each component, so we need to selectively export each one. the explicit & library methods are just developer experience enhancements but under the cover they're the same. You would preload the icons like in FontAwesome to avoid that fontAwesome doesn't preload the icons (in the sense of browser resource preloading) it just suggest that you use a library for easier development experience, but performance wise, it's both static importing. dynamic importing will be problematic because of the number of files generated, when you generate 300 file, even you preload, you will still have to do 600 trip (a trip to the chunk file & a trip to the actual file), if we go with eager, then it's 300 extra chunk in our main bundle, making all this dynamic importing useless. code splitting & dynamic importing makes a lot of sense when the chunk file size is big and the number is low, when you have 300 file that are 500byte in size, you end up wasting a lot of resources just to fetch them. I still stand with statically importing them, we can search more into using dynamic import for Gutenberg core packages thou — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#17055?email_source=notifications&email_token=AESFA2FPKWNE24WQACL6RITQE4XANA5CNFSM4IL7MID2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4P4Q7Q#issuecomment-522176638>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AESFA2CQOCVYJST7QLSJ3VTQE4XANANCNFSM4IL7MIDQ> .

[senadir]

but someone will still have to statically import them, and we can't import everything for them, and we can't import based on that string since it's dynamic importing and we're back at the main issue.

unless developers build their own library, if this is what you're referring to then yes, it sounds like a good solution

[epiqueras]

Yes, it’s still better than explicitly importing them everywhere.

…

On Fri, Aug 16, 2019 at 4:15 PM Seghir Nadir ***@***.***> wrote: but someone will still have to statically import them, and we can't import everything for them, and we can't import based on that string since it's dynamic reloading and we're back at the main issue. unless developers build their own library, if this is what you're referring to then yes, it sounds like a good solution — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#17055?email_source=notifications&email_token=AESFA2GBLXA4SRIZE6WCBZTQE4YK5A5CNFSM4IL7MID2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4P5ABY#issuecomment-522178567>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AESFA2DVVOFTAT6P5TXVNMTQE4YK5ANCNFSM4IL7MIDQ> .

[senadir]

one problem that just came to my mind, the goal of this PR was a preparation for the icon block, the problem is, somehow we need to load all the available icons in a set at once (for the user to select from). All the cases we looked at was from a developer point of view, were you know beforehand what icon you need to use and statically select it, but for an icon block and for normal users, they need a way to see and select icons, which renders this PR useless for that block case if we go with statically typing each one.

I guess we will have to define a way to export a whole set at once as well individually exporting them and then dynamically import it for the user when he wants to select

import( `@wordpress/icons/${set}` );

How does that solution sound? I know it's out of the scope but I think it falls in here

[jasmussen]

Again deferring to the technical considerations, I would just state that what @epiqueras states here definitely seems to address all concerns I've noted on this ticket.

I'd love for us to start using dynamic imports, not only for icons, but also for lazy loading things (say the syntax highlighter...). At the time of writing, we don't have a solution for it though.

In that vein, would it make sense to wait until we do? While it would be very nice to have a better icon component, what we have today technically functions. If we go the path previously suggested with named components one for each icon, we potentially build up a slew of technical debt while awaiting a better solution.

If the goal of this refactor is to enable an icon block, that does not seem like it should make this refactor be that much more urgent, and perhaps an approach such as that of coblocks might make more sense there.

[senadir]

@jasmussen as we stated, dynamic importing as a concept doesn't fit with icons, since it create a single file for each icon, an additional request to each one, coblocks uses the same concept se use right now by directly including the icons as an object, it works since there is only 69 icon on coblocks, it's the same concept I used for the icon block (see this unpublished PR).

For us, we're going to use an <Icon> block, but developers & core contributors has to define the components they're going to use somehow #17055 (comment)

// some initial file 
import { Library } from '@wordpress/icons';
// Our current workflow doesn't allow such imports but we can get around that later.
import { Post } from '@wordpress/icons/dashicon';
import { Phone, Save } from '@wordpress/icons/material';

// ...

Library.add( {
    default: [ Post, Save ],
    material: [ Phone ]
} );

// the place you're going to use the icons in
import { Icon } from '@wordpress/components';

// ...

<Icon name="post" />
<Icon name="phone" set="material" />

The current problem we're trying to tackle right now is not there yet. after much consideration, this PR will not help much in the icon block now, but should pave the way to include other icons, for developers and core contributors and an easier way to extend the icon block.

By no mean I think this PR is urgent, there is a lot of talk that needs to happen, 2 work days already and we tackled a lot of technical & implementation details aIrealy.

so @jasmussen from a developer point of view, how do you think about this approach and what kind of technical debt it may introduce?

[senadir]

dynamic import creates a single file for each icon

I think I should expand on this more

Each icons is a component:

• each icon is a separate file.
• you have to statically import them.
• at build time, all the used components are combined into a single file.

Dynamic importing icons:

• each icon is a separate file.
• we can dynamically import them at build time.
• at build time, each icon is a file, so if your interface uses 50 icon, you will load 50 file, sending cookies, request headers, and doing round trips for 50 times.

[jasmussen]

I don't know that I'm the right person to suggest how much technical debt this solution might introduce. I may in fact be wrong entirely.

So instead of talking too much about the specific implementation, I would rather suggest that the more we can abstract away from the developer, the better. The idea being that icons as used in WordPress as a whole are likely to change drastically over the coming years. If we abstract away as much as possible, it seems more feasible to refactor the underlying code without the developer having to do a thing.

For example, and as referenced in the discussion here: #9647 (comment) — we might be looking at replacing Dashicons with a different set entirely in the future, for example Material icons. Whatever we can do to ease that transition if/when the time comes, the better.

In that vein, I like the sound of this:

It still lets the consumers have a single Icon component with a string prop, instead of having to import multiple icons everywhere.

But again, I should not be the arbiter here, I would defer to @youknowriad or @mtias for final thoughts.

[hacknug]

somehow we need to load all the available icons in a set at once (for the user to select from)

at build time, each icon is a file, so if your interface uses 50 icon, you will load 50 file, sending cookies, request headers, and doing round trips for 50 times.

Ignoring all of the React stuff, wouldn't an SVG sprite solve this issue?

[senadir]

an SVG sprite will still need to be built for select icons, and you can't have a SVG sprite for all the icons (it will be in tens of mbs)

[hacknug]

Of course, but you could always pack icons in sprites of, let's say, a 100 icons. That would reduce the number of requests, downloads… and would even let you lazy load them since users wouls only see the firsts few icons until they've scrolled further or searched for an icon.

Only mentioning it because there's some plugins on the ecosystem that already use svg sprites to allow devs to add icons to their plugins and it works fine although it's a different paradigm than Gutenberg's and React.

[senadir]

if you're talking about the search & select process then yes, sprites makes a lot of sense, the idea I had is to paginate them somehow, showing like 30 each time or something similar, or going an advanced road & stream the file

[epiqueras]

What about grouping dynamic imports for icons in the client, having the server build a single sprite-sheet for them, and shipping that 🤣

[paaljoachim]

Similar to: #9648

[senadir]

Those latest changes are working fine for me, now that this has minimal effects, we can progressively update icons in Gutenberg while deprecating older ones.

[youknowriad]

I'm wondering whether we should move all current Gutenberg icons there in this PR or in follow-up ones. I'm leaning towards just the latter.

[senadir]

I'm wondering whether we should move all current Gutenberg icons there in this PR or in follow-up ones. I'm leaning towards just the latter.

leaving it for other PRs will allow it to be done by multiple people without the fear of breaking this

[jasmussen]

Thank you all for working on this. I'm super excited for a modern icon component to land, and what that will enable with regards to a vast library of icons available for developers, some of which we can polish and make just right for WordPress.

For this PR specifically, the only apparent visual change is the Save icon:

As for the individual icons, from what I can tell the story is now this:

<Icon icon={ saved } />

That references saved.js which looks like this:

/**
 * WordPress dependencies
 */
import { SVG, Path } from '@wordpress/primitives';

const saved = (
	<SVG
		aria-hidden
		role="img"
		focusable="false"
		xmlns="http://www.w3.org/2000/svg"
		viewBox="-2 -2 24 24"
	>
		<Path d="M15.3 5.3l-6.8 6.8-2.8-2.8-1.4 1.4 4.2 4.2 8.2-8.2" />
	</SVG>
);

export default saved;

So once this PR lands, the only blocker to expanding that set beyond a "saved" icon, it seems, is to get to work and create a build system that outputs JS files from SVGs, of which we already have a good start. Nice work!

Two quick action items before this PR lands, though:

• The "saved" icon is black. We should likely apply some CSS to make sure it is correctly colored like other icons. fill: currentColor; should do it.
• Can we rename it "check" instead of "saved"? We might want to use that very same icon for other things, such as inside menus, so best to keep names descriptive of their motif rather than their current function.

[youknowriad]

So once this PR lands, the only blocker to expanding that set beyond a "saved" icon, it seems, is to get to work and create a build system that outputs JS files from SVGs, of which we already have a good start. Nice work!

You mentioned that we want to rename some icons. For example . "saved" here should be "check", correct? For this reason and for the fact that it's not just about Dashicons, I think we should avoid an automated script to generate these icons.

[youknowriad]

The "saved" icon is black. We should likely apply some CSS to make sure it is correctly colored like other icons. fill: currentColor; should do it.

This should be already taken care of for icon buttons, the fact that it's not applied in the "save indicator" is because it's not a button, it requires a specific fix I think. Global styles for svgs are not great.

[jasmussen]

You mentioned that we want to rename some icons. For example . "saved" here should be "yes", correct? For this reason and for the fact that it's not just about Dashicons, I think we should avoid an automated script to generate these icons.

I do think we should have a new heuristic for naming icons. In the case of the check, it should be called "check", neither "saved" nor "yes". I do know it's called "yes" in Dashicons, but that's an ambiguous name, and unclear for developers seeking a checkmark icon for other aspects of their plugin.

While I don't have any strong opinions about how we add icons, one goal is to add a BIG baseline set of icons so developers don't have to search far to find an icon that fits. For example we could import the entirety of the material set, which is up to 1000+ icons.

We also want to honor the tradition of Dashicons by adding a WordPress visual flavor to the most visible icons in the interface, so while we might start with a big baseline set, we'd want to replace as many of them with our own style as we need to. Some of that icon work as been explored also in #18667.

Each SVG file needs a little cleanup and minification, and to ensure that the SVG syntax is React Native compatible. Are there other methods of automating this, than a build script? I'm thinking something like when a new icon is created, its design and name needs some approval first, then it goes into a source folder, the script is run, and a new file is generated which can be added.

[youknowriad]

For example we could import the entirety of the material set, which is up to 1000+ icons.

noting that with the current package, we still rely on WordPress scripts which means all icons are loaded no matter if they're used or not.

The package allows tree-shaking though so if the package is installed as an npm dependency, we can choose which icon to include in the bundle. So I believe ultimately this package will have two sets:

• main set: bundled by default in a WordPress script and loaded in all pages.
• extra set: you can install the package as npm dependency and bundle the icons you want (for third-party usage).

There's no technical way around that IMO.

[youknowriad]

a new icon is created, its design and name needs some approval first, then it goes into a source folder,

If all this is manual, the remaining part creating the file is very quick and only applies to a single icon (not in batch) which means for me, there's no need for automation. But again with flows, it's something we learn over time, so if we do find burden, it is possible to write code generators.

[jasmussen]

The package allows tree-shaking though so if the package is installed as an npm dependency, we can choose which icon to include in the bundle. So I believe ultimately this package will have two sets:

So if we created that extra set of a thousand icons, that's a pretty big single component that would have to be downloaded for any plugin that needed to use even just one icon, correct? A lot of the benefit of doing an icon component like this in the first place is lost in that case.

[youknowriad]

So if we created that extra set of a thousand icons, that's a pretty big single component that would have to be downloaded for any plugin that needed to use even just one icon, correct?

If you consume them through WordPress script yes, if you consume them through npm no.

[jasmussen]

Thanks for your patience answering my questions!

If you consume them through WordPress script yes, if you . consume them through npm no.

So, say I have this plugin, and instead of providing my own icons for 1, 2, 3 or 4 columns, I wanted to provide icons provided by the expanded set, would I be using the WordPress script or NPM, or could I use either depending on my skill as a developer?

[youknowriad]

would I be using the WordPress script or NPM, or could I use either depending on my skill as a developer?

You can use both but I think to avoid the bundle size problem, we'll limit the number of icons available in the WordPress script. So if it's an icon only available in the "extra set", you'll be forced to use npm and build your script.

[jasmussen]

You can use both but I think to avoid the bundle size problem, we'll limit the number of icons available in the WordPress script. So if it's an icon only available in the "extra set", you'll be forced to use npm and build your script.

That makes sense, and using NPM is also the preferred way to build blocks/use icons, so that seems okay.

I still think we'll want some automation if nothing else then one-off scripts for the initial creation of the two sets. But this remains a separate discussion.

[youknowriad]

Ok, so let's move forward with this package and try to expand it in follow-up PRs. It's still marked as experimental, so we can still change the API, the bundling...

[gziolo]

It should be ## Master :)

[gziolo]

Aren't 3 first props covered by SVG component wrapper?

[gziolo]

Confirmed:

gutenberg/packages/primitives/src/svg/index.js

Lines 21 to 23 in 46fcca7

	role: 'img',
	'aria-hidden': 'true',
	focusable: 'false',

[youknowriad]

Thanks, I fixed your comments in #19809