All icons are bundled unless explicitly specified

[bgentry]

When switching from ember-font-awesome to ember-fontawesome I noticed the following payload changes for my app that uses about 30 icons.

Asset	FA 4	FA 5	Diff
vendor.js	545 KB	761 KB	+216 KB
font	67 KB	0 KB	-67 KB
app.css	31 KB	23.9 KB	-7.1 KB

So overall, an increase of 141.9 KB from the switch.

I have not added anything to my ember-cli-build.js, and I'm using both the fontawesome-free-brands and fontawesome-free-solid packages on v5.1.0-3. These were installed with yarn add [pkgname] per the readme (not dev dependencies).

From this comment I was under the impression that this addon implemented tree shaking to get rid of unused icons. Is that not the case? Is there something else I need to do to get the build size back down?

[bgentry]

If I manually specify all 31 icons I'm using, then my vendor.js bundle shrinks down to 562 KB, which makes this upgrade smaller overall.

[mlwilkerson]

In order to sub-set the icon bundles, you do have to declare the ones you're using in the ember-cli-build.js. It is using tree-shaking (Rollup under the hood), but—at this time—it's based on those icons you declare in that build config.

[mlwilkerson]

The README shows how to configure the icon subsetting in ember-cli-build.js

[bgentry]

@mlwilkerson ok, got it, so this issue is really more like: "addon bundles all icons unless an exact subset is specified"

Feel free to repurpose/rename this issue if you'd like to leave it open to help others that run into it or to track your progress on this issue. Otherwise, if you're tracking this remaining work elsewhere, go ahead and close it. Thanks!

[mlwilkerson]

Yes, I thought I wrote the README in such a way as to convey what you said:

addon bundles all icons unless an exact subset is specified

But I'm glad to tweak the documentation further to make it clearer.

So, specifying the 31 icons in ember-cli-build.js brought your vendor.js down to about 538 KB? Did I do the math right?

[bgentry]

I guess I misunderstood the readme earlier. It probably does make sense as you've written it.

So, specifying the 31 icons in ember-cli-build.js brought your vendor.js down to about 538 KB? Did I do the math right?

562 KB. So the overall JS increase is 17 KB, but with the other decreases that makes the upgrade total out to a 57.1 KB savings over the old version.

[mlwilkerson]

Well, I'm really glad to hear that it's smaller.

And I think that if I were in your shoes, I would have approached it similarly (i.e. if I'd read "tree shaking", I would probably would not have expected to have to add some config in order to actually shake the tree). I do want to pave the way smoothly for those who come along later. So I welcome any further recommendations for doc improvements (or otherwise) you may have.

[jrjohnson]

I've been working on bringing the AST transform work from ember-font-awesome over to this addon. There are several steps left, but in the end I think we're going to be able to transform any static invocations like {{fa-icon 'user'}} to <svg><path></svg> at build time. There are some significant runtime advantages to this, but it occurs to me that it also means those icons will not need to be bundled in to the final build. Only icons in the form of {{fa-icon iconName}} where the icon can't be detected at build time will need to be available in the final output tree.

I bring this up here because I think that means we should work with @bgentry's original assumption and remove all icons by default unless the user explicitly requests all or a subset of icons we should include none of them. Since the list will only need to include dynamic icons it will be much more manageable.

This also may solve the problem of how addons should list their required icons - they don't. Addons would just need to never use a dynamic invocation or else instruct developers to add icons to their ember-cli-build.js file when the addon is installed.

[bgentry]

That sounds great!

One concern that came to mind was helping devs make sure they haven’t forgotten dynamic icons in their build config. If a user tries to invoke a dynamic icon that’s not listed, will they see a warning log (outside production) and either blank space or the “missing icon” icon?

Other than that this sounds pretty close to ideal.

[mlwilkerson]

@jrjohnson First, regarding what to bundle (or not) by default: If I understand you correctly, you recommend that by default, we include no icons in the bundle, and only include those that are specified in ember-cli-build.js? If so, fine by me, if that provides a good/best-practice Ember development experience.

But my previous understanding is that we were going for the opposite approach: that by default we include more (or all) icons in the bundle, and that those who know they want to optimize the bundle size would be able to specify a subset in the ember-cli-build.js.

Both approaches make sense to me, so it seems to be a matter of optimizing the Ember development experience, and for that, I would defer to the more seasoned Ember devs here. Just bear in mind the newcomers who should be able to have an easy on-boarding experience. (So I'm concerned that if, by default, no icons are bundled, then there might be some support complaints about missing icons.)

Second, regarding the build time static transforms, I'll open a separate issue to carry on that discussion, since it's a whole other thing (though admittedly has overlap here). It's over at #14.

[jrjohnson]

I'm not sure what we should optimize for here. The Ember way is generally to provide the best experience possible with the least configuration. In this case though I think a minimal build is the best experience. Since it looks like it will be possible to handle static icon names in a transform it won't require a lot of work on a developers part to just register their dynamic icons (especially since missing icons throw a console error). If we leave icons in by default then it becomes much more annoying later to attempt to discover which icons are missing once a developer realizes that they can save a lot of file size by doing so.

[bgentry]

I think it's reasonable to expect folks to whitelist the dynamic icons they want to include. The "all" option makes it easy for people who really don't care about payload size and just want it to work.

The truly bad part of the whitelisting experience is that users have to restart their app server each time they want to add an icon to the list and have it be available in development. But I don't think there is any way to avoid that in dev while still warning them about non-whitelisted icons, is there?

[hoIIer]

what's the rationale for having to declare the icons in config/icons.js vs in ember-cli-build.js which is where most other addons do their config?

[jrjohnson]

@burritoIand I was forced to invent a bit of a new config for this addon because it needs to be configurable from within other addons and that configured list of icons needs to filter up to the top level app in order to include only the needed icons in the build. If there is another way to accomplish this using the ember-cli-build.js file please let me know because it would be nice to use that standard.