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

Automatically subset icons #45

Open
mhluska opened this issue May 1, 2018 · 11 comments
Open

Automatically subset icons #45

mhluska opened this issue May 1, 2018 · 11 comments
Labels

Comments

@mhluska
Copy link

mhluska commented May 1, 2018

Is it possible to automatically subset icons based on usage in the code? I run into the problem where I sometimes forget to include an icon in my icon subset and the server builds without complaint. So a production bug occurs where the icon is missing. This could be solved by either showing a warning or error when a missing icon is used or somehow dynamically generating the icon subset.

@mhluska
Copy link
Author

mhluska commented May 1, 2018

Nevermind, I see that this is happening in the latest version. Going to close.

@mhluska mhluska closed this as completed May 1, 2018
@mhluska
Copy link
Author

mhluska commented May 1, 2018

Actually, there is still the issue of including too many icons in the subset and not being warned about it.

@mhluska mhluska reopened this May 1, 2018
@jrjohnson
Copy link
Collaborator

@mhluska yes. This is the goal. Currently you have to control the icons that are included yourself with the configuration outlined in https://github.com/FortAwesome/ember-fontawesome#subsetting-icons

It isn't documented because there are still many pieces to work out, but you can also add enableExperimentalBuildTimeTransform: true to your configuration which will detect many of the icons you are using in your templates and include them automatically as well. Even with that enabled though it will probably never be possible to automatically include icons based on a JS property or those using some types of transforms.

My advice at this point would be to include nothing by default and pay attention to your console logs where missing icons will be highlighted - this will give you the slimmest build with the best forward compatibility.

@st-h
Copy link
Contributor

st-h commented May 22, 2018

@jrjohnson does the workaround still work? I tried setting the includes to an empty array and even tried to removed the whole config, but the icons still show up. no logs showing up.

@st-h
Copy link
Contributor

st-h commented May 24, 2018

Here are some more details about what I found is currently going on:

    fontawesome: {
      icons: {
      }
    }

includes all icons.

this config:

    fontawesome: {
      icons: {
        'free-solid-svg-icons': [],
        'pro-solid-svg-icons': [],
        'pro-light-svg-icons': [],
        'pro-regular-svg-icons': []
      }
    }

leads to runtime errors:

vendor.js:3464 Uncaught TypeError: Cannot convert undefined or null to object
    at vendor.js:3464
    at vendor.js:3464
(anonymous) @ vendor.js:3464
(anonymous) @ vendor.js:3464
vendor.js:11 Uncaught Error: Could not find module `ember-resolver` imported from `app/resolver`
    at vendor.js:11
    at l (vendor.js:11)
    at a.findDeps (vendor.js:23)
    at l (vendor.js:11)
    at a.findDeps (vendor.js:23)
    at l (vendor.js:11)
    at requireModule (vendor.js:5)

subsetting still seems to work unless the two cases above, however I don't see any errors logged to the console about missing includes. and after two days of not seeing any log message about missing icons, they returned today without any change. I have no idea what's going on...

@lougreenwood
Copy link

I'm getting similar errors to @st-h - however for me, not specifying a subset renders the page with no icons, specifying a subset throws undefined errors...

@robmadole robmadole added the bug label May 31, 2018
@nickschot
Copy link

I am also seeing the same issue as @st-h . This is combined with the experimental build time transform.

@robmadole
Copy link
Member

Can anyone provide a reproducible example?

@nickschot
Copy link

nickschot commented Sep 27, 2018

It's caused at this line:

and seems to be a runtime error.

It's reproducible in any app when enabling the experimental build time transform and setting your icon pack subsets to [].

Something like:

    'fontawesome': {
      enableExperimentalBuildTimeTransform: true,
      icons: {
        'free-brands-svg-icons': [],
        'pro-solid-svg-icons': [],
        'pro-regular-svg-icons': [],
        'pro-light-svg-icons': []
      }
    }

@ehubbell
Copy link

I'm running into the same issue as @nickschot. For now, I've included a single icon ['coffee'] for packs that I don't intend on using such as the free-solid-svg-icons pack. Honestly, I feel like that pack should require opt-in vs opt-out since it adds 125kb to your vendor file after gzip and the instructions aren't super clear around it.

I had the following in my vendor file:

    "@fortawesome/ember-fontawesome": "^0.1.9",
    "@fortawesome/free-brands-svg-icons": "^5.6.3",
    "@fortawesome/pro-solid-svg-icons": "^5.5.0",

And, without any configuration in my environment.js file, that was adding ~500KB to my vendor file which is significant. I also didn't realize this until I did a bundle analysis. I feel like there should be better documentation / support surrounding this for unsuspecting developers.

@jdurand
Copy link

jdurand commented Sep 2, 2021

I'm experiencing this issue as @ehubbell and @nickschot. It's not related to enableExperimentalBuildTimeTransform: true.
Setting an empty array for any of the icon packs will cause this :

// config/icons.js
module.exports = function() {
  return {
    'free-brands-svg-icons': []
  };
};

Not a big issue as you just need to whitelist a single icon to fix this issue as @ehubbell pointed out; but it did give me good 15 min of rm -fr node_modules && yarn install -> 😐😑😐... 😶

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

No branches or pull requests

8 participants