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

Tree Shaking - automatically detecting usage #233

Open
EtienneBruines opened this issue Jun 26, 2020 · 14 comments
Open

Tree Shaking - automatically detecting usage #233

EtienneBruines opened this issue Jun 26, 2020 · 14 comments

Comments

@EtienneBruines
Copy link

Looking at the recommended usage

src/main.js

import Vue from 'vue'
import App from './App'
import { library } from '@fortawesome/fontawesome-svg-core'
import { faUserSecret } from '@fortawesome/free-solid-svg-icons'
import { FontAwesomeIcon } from '@fortawesome/vue-fontawesome'

library.add(faUserSecret)

Vue.component('font-awesome-icon', FontAwesomeIcon)

Vue.config.productionTip = false

/* eslint-disable no-new */
new Vue({
  el: '#app',
  components: { App },
  template: '<App/>'
})

src/App.vue

<template>
  <div id="app">
    <font-awesome-icon icon="user-secret" />
  </div>
</template>

<script>
export default {
  name: 'App'
}
</script>

And then the tree shaking docs:

import { library } from '@fortawesome/fontawesome-svg-core'
import { faCoffee } from '@fortawesome/free-solid-svg-icons/faCoffee'
import { faSpinner } from '@fortawesome/pro-light-svg-icons/faSpinner'

library.add(faCoffee, faSpinner)

Is your feature request related to a problem? Please describe.
Having to call library.add(faCoffee) is a pain. Using about 80 icons in a bigger project, keeping this up-to-date isn't nice.

Describe the solution you'd like
Shouldn't the usage like <font-awesome-icon icon="user-secret" /> suffice to be able to dynamically import only those icons? Especially when using webpack/rollup to parse it.

Describe alternatives you've considered
N/A

Additional context
N/A

@robmadole
Copy link
Member

@EtienneBruines are you using Babel?

@EtienneBruines
Copy link
Author

Haven't had the need.

Rollup for me personally.

@robmadole
Copy link
Member

So what mechanism would be able to dynamically identify icons used (and do so in complex situations where methods or computed props are called to get icons)?

@EtienneBruines
Copy link
Author

Any mechanism that would be made as part of this feature request.

For static icons that should be achievable? Even :icon usages that don't include variables (array syntax) would be doable.

Without knowledge about HTML even regex would be able to find and detect those usages, dynamically importing it.

That script can then be part of the webpack or Rollup build process.

Since you asked about babel, does that mean babel can already do this?

@GTANAdam
Copy link

I agree, it is kinda hard to maintain a huge list of used icons while it could be automated.

@christhofer
Copy link

@robmadole is it possible to call import icon & library.add() inside a vue component?
My idea is to make a component wrapper of , check if the icon is already exist in the library, and do import + library.add if icon hasn't been imported yet.

@daniandl
Copy link

daniandl commented Oct 2, 2020

@robmadole is it possible to call import icon & library.add() inside a vue component?
My idea is to make a component wrapper of , check if the icon is already exist in the library, and do import + library.add if icon hasn't been imported yet.

Did you have any luck with this?

@dennmat
Copy link

dennmat commented Oct 16, 2020

Even something like requiring the prefix would be preferable:
<font-awesome-icon icon="fas-circle" />

I see the React library follows the same pattern. I'm a little curious as to the reasoning behind the suggested method of passing around [string,string] everywhere for an icon, vs a kebabed name.

The split is negligibly cheap and it simplifies the code immensely. And the parsing on the library side could be non-existent if the objects stored that name instead of 2 separate fields prefix and iconName could instead just be id: 'fa(s|r|d|l)-<iconName>' and then the above example would be an easy lookup on a mapping.

Requiring it be prefaced with (fas|fal|far|fad) also should remove any barrier from the library being able to build up the library on its own. Or even if it was <font-awesome-icon prefix="fas" icon="circle" />

If I want to just put a square:

import { library } from '@fortawesome/fontawesome-svg-core';
import { faSquare } from '@fortawesome/pro-solid-svg-icons';

library.add(faSquare);

...

<font-awesome-icon :icon="['fas', 'square']" />

It's just too much imho for something that could be handled by the library and turned into:

<font-awesome-icon icon="fas-square" />

I've found it to be polluting my code base tbh. Every Vue files script tag is littered with 2 imports and a library call minimum just to have an icon somewhere.

Would love to see this simplified somehow.

@GTANAdam
Copy link

GTANAdam commented Oct 16, 2020

Even something like requiring the prefix would be preferable:
<font-awesome-icon icon="fas-circle" />

I see the React library follows the same pattern. I'm a little curious as to the reasoning behind the suggested method of passing around [string,string] everywhere for an icon, vs a kebabed name.

The split is negligibly cheap and it simplifies the code immensely. And the parsing on the library side could be non-existent if the objects stored that name instead of 2 separate fields prefix and iconName could instead just be id: 'fa(s|r|d|l)-<iconName>' and then the above example would be an easy lookup on a mapping.

Requiring it be prefaced with (fas|fal|far|fad) also should remove any barrier from the library being able to build up the library on its own. Or even if it was <font-awesome-icon prefix="fas" icon="circle" />

If I want to just put a square:

import { library } from '@fortawesome/fontawesome-svg-core';
import { faSquare } from '@fortawesome/pro-solid-svg-icons';

library.add(faSquare);

...

<font-awesome-icon :icon="['fas', 'square']" />

It's just too much imho for something that could be handled by the library and turned into:

<font-awesome-icon icon="fas-square" />

I've found it to be polluting my code base tbh. Every Vue files script tag is littered with 2 imports and a library call minimum just to have an icon somewhere.

Would love to see this simplified somehow.

Agreed.
I've just checked the Pull Requests and found this #243, it seems like someone has already provided a workaround solution to this matter but the maintainers have frozen it with the "it's planned for the next eternity" lol

Additionally, in your case, you could just create a component that accepts those attributes and get away with it :)

Example:

<template >
  <fa-icon :icon="getIcon" />
</template>

<script>
export default {
  props: {
    icon: {
      type: String,
      required: true,
      // validator: (value) => ... // TODO: Check if contains fa(s|r|d|l)
    },
  },
  computed: {
      getIcon() {
          const spl = this.icon.split('-');
          const prefix = spl[0];
          const icon = spl.slice(1).join('-');
          return [prefix, icon];
      }
  }
};
</script>

then calling it from any other components with:
<icon icon="far-thumbs-up" />

@dennmat
Copy link

dennmat commented Oct 16, 2020

Even something like requiring the prefix would be preferable:
<font-awesome-icon icon="fas-circle" />
I see the React library follows the same pattern. I'm a little curious as to the reasoning behind the suggested method of passing around [string,string] everywhere for an icon, vs a kebabed name.
The split is negligibly cheap and it simplifies the code immensely. And the parsing on the library side could be non-existent if the objects stored that name instead of 2 separate fields prefix and iconName could instead just be id: 'fa(s|r|d|l)-<iconName>' and then the above example would be an easy lookup on a mapping.
Requiring it be prefaced with (fas|fal|far|fad) also should remove any barrier from the library being able to build up the library on its own. Or even if it was <font-awesome-icon prefix="fas" icon="circle" />
If I want to just put a square:

import { library } from '@fortawesome/fontawesome-svg-core';
import { faSquare } from '@fortawesome/pro-solid-svg-icons';

library.add(faSquare);

...

<font-awesome-icon :icon="['fas', 'square']" />

It's just too much imho for something that could be handled by the library and turned into:
<font-awesome-icon icon="fas-square" />
I've found it to be polluting my code base tbh. Every Vue files script tag is littered with 2 imports and a library call minimum just to have an icon somewhere.
Would love to see this simplified somehow.

Agreed.
I've just checked the Pull Requests and found this #243, it seems like someone has already provided a workaround solution to this matter but the maintainers have frozen it with the "it's planned for the next eternity" lol

Additionally, in your case, you could just create a component that accepts those attributes and get away with it :)

Example:

<template >
  <fa-icon :icon="getIcon" />
</template>

<script>
export default {
  props: {
    icon: {
      type: String,
      required: true,
      // validator: (value) => ... // TODO: Check if contains fa(s|r|d|l)
    },
  },
  computed: {
      getIcon() {
          const spl = this.icon.split('-');
          const prefix = spl[0];
          const icon = spl.slice(1).join('-');
          return [prefix, icon];
      }
  }
};
</script>

then calling it from any other components with:
<icon icon="far-thumbs-up" />

Might just do that, it will at least help with the arrays everywhere, the only downside being having to pass through all the other props.

Excited to see what they have planned for 6, hopefully it tackles this.

To me the bigger pain is the imports and library call anywhere you want to use an icon. (Yes I could centralize it to a single file but then you have to do book keeping and if you change or delete an icon somewhere you have to do code base garbage collection making sure there's no other references) so doing it in place everywhere it's used is surprisingly the more manageable option.

@GTANAdam
Copy link

GTANAdam commented Oct 16, 2020

Might just do that, it will at least help with the arrays everywhere, the only downside being having to pass through all the other props.

Excited to see what they have planned for 6, hopefully it tackles this.

To me the bigger pain is the imports and library call anywhere you want to use an icon. (Yes I could centralize it to a single file but then you have to do book keeping and if you change or delete an icon somewhere you have to do code base garbage collection making sure there's no other references) so doing it in place everywhere it's used is surprisingly the more manageable option.

Lmao, I was about to quote all of the above, thankfully I wasn't lazy enough to edit the quotation.

Yeah well, you only have to set up the component once and make it global, it will act like a shim or proxy or whatever you want to call it. you can find the props here: https://github.com/FortAwesome/vue-fontawesome/blob/2.x/src/components/FontAwesomeIcon.js

You can have my little template, the rest you can complete from that file I have linked

<template >
  <font-awesome-icon :icon="getIcon" :border="border" :fixedWidth="fixedWidth" :flip="flip" :mask="mask"
    :listItem="listItem" :pull="pull" :pulse="pulse" :rotation="rotation" :swapOpacity="swapOpacity" :size="size"
    :spin="spin" :transform="transform" :symbol="symbol" :title="title" :inverse="inverse" />
</template>

As for manually importing icons for treeshaking, yeah well, it is indeed annoying but I have managed to successfully write an auto build script that would walk through all of my vue files, extract the icon names from them and autogenerate a fontawesome-autogen.js file which is basically importable.

The script is about 115 lines, I will have it published in my repo once I'm done with it.

@GTANAdam
Copy link

Okay, as promised, here's my repository https://github.com/GTANAdam/vue-fontawesome-autogen

@iamandrewluca
Copy link

#293 (comment)

@christhofer
Copy link

@iamandrewluca

While I agree it's better to import each icon you are using, you still can have unused icons ended up in the bundle
For example when you are working on a module, you imported icons for this specific module.
But then, the project spec changed, and you (or your) teammate deleted that module, you have to manually remove the icon from import.

Have you tried Tailwindcss?
Tailwindcss removes unused classes when generating the production bundle. And now with JIT it also works in development.

Now If only Fontawesome can do this as well.

GTANAdam wrote a plugin to do this when doing npm run build, but it doesn't support HMR currently CMIIW.

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

No branches or pull requests

7 participants