Prod ready

[M1CK431]

Almost complete rewrite to make this package prod ready.

Changes compare to original version:

🆕 Automagically handle all installed styles, even the one which will be created in the futur (as long as font awesome keep the same naming pattern 🤞)
🆕 Detect used icons even in .js files (previously only .vue files was handled, useful for computed icon 🎉 )
🆕 Warn in console about unknown icon (for example if trying to use the regular style but don't have it installed)
🆕 Could be used in watch mode (fix #1)
🆕 Compatible with existing project (by using directly the regular vue-fontawesome component) - *see Breaking change below
🚀 About 50% faster

⚠️ Breaking change (kind of) ⚠️

This new rewritten version don't provide a custom Fa.vue component and so the previous syntaxe <fa icon="fas-check"> will not work out of the box anymore. Instead, you should use the regular vue-fontawesome provided component (aka FontAwesomeIcon).

But if you absolutely want to keep the previous custom syntaxe (despite there is good reasons to use the regular one), you can override the default pattern used to match font awesome icons in your projet using an environnement variable. A complete example of how to do it is provided in the updated README, among a custom component example.

Despite it's a (kind of) breaking change, it will be hugely simpler to use this package in an existing projet which already use the regular component.

However, a (slight) change is still required in the code base of an existing project which want to use this new version. Check the README for details (#noTeaser 🤫).

How it works

This rewritten version does the following steps:

• parse your package.json file to detect which font awesome styles are available in your project
• parse any .vue and .js files in your src folder naively looking for font awesome icon definitions (must by an array with explicit prefix)
• generate the "import" file in src/plugins/fontawesome-autogen.js (only if needed - aka icon added/removed in project 😎)

Benefits compare to original

• Require by far less change to be used in an existing projet (only have to explicit the usage of fas style)
• Take care of icons defined in .js files (useful for generated list based on an array/object/whatever imported from a .js file)
• Handle webpack watchRun mode
• Should be compatible with any futur font awesome style (no hard-coded list anymore)
• Hint the developer about unknown icon (probably caused by forgotten style installation)
• Faster (well, the original one is probably fast enough, but still 😉)
• Less code (less bug )

[meirroth]

Hi @M1CK431, This looks awesome! Thank you :)

[phuze]

@M1CK431 Cheers mate, I agree with all of the changes you've made here.

@GTANAdam I understand it may not be your intended design pattern, but I agree that this package should not stray from the official/intended Font Awesome syntax. I had to fork and modify your work for the very reason identified by @M1CK431 --- it meant a significant amount of code change to adopt your proposed custom syntax. By maintaining the official syntax, you make it easy for a developer to introduce this package into their project, especially when considering an established/large project. I would encourage you to adopt this proposed change.

[GTANAdam]

Greetings, thank you for the contributions.
Everything looks quite decent excluding the breaking change that I'm insisting on having a backward compatibility for already set up projects.

I would also like to note that, the use of the official vue-fontawesome syntax goes against the main idea of this project. I suggest we have compatibility for both syntaxes.

[M1CK431]

Hi @GTANAdam ,

Thanks for your reply.

Perhaps I misunderstood what is the "main idea of this project" but from the README:

this script was written to make managing imported fontawesome icons in VueJS uncumbersome. It implements simple parsing techniques that would generate a file that imports all of your used icons without having to manage them every single time.

And so there is nothing about a custom syntaxe. Also, please notice the reason why the array syntaxe is needed: https://github.com/FortAwesome/vue-fontawesome#why-so-explicit-the-iconfar-coffee-syntax

Anyway, It's very easy to handle the custom syntaxe, just by updating the regex in index.js at L.19. I will do it ASAP.

[M1CK431]

@GTANAdam I have updated this PR to add a way (environnement variable) to handle any custom syntaxe you wish simply by overriding the pattern used to match font awesome icons in the project. I have added an example in the README. Also, I have rewrite your Fa.vue component to make it simpler and handling events (like if you want to handle a click on it for example). I hope it will be helpful for your projet.

From the updated README:
-------------------------------------------------------------8<-------------------------------------------------------------

Customization

Sometimes you may want to customize the pattern used to match font awesome icons in your projet.
It may happens if you have written a wrapper around the regular vue-fontawesome provided component (aka FontAwesomeIcon).

Wrapper example (Fa.vue):

<template>
  <FontAwesomeIcon :icon="faIcon" v-bind="$attrs" v-on="$listeners" />
</template>

<script>
  export default {
    props: { icon: { type: String, required: true } },
    computed: {
      faIcon: ({ icon }) => {
        const [, style, name] = icon.match(/(fa[a-z])-([a-z-]+)/);
        return [style, name];
      }
    }
  };
</script>

In that situation, a font awesome icon will looks like <fa icon="fas-check" /> (html) or const icon = 'fas-check' (js).
To handle that custom syntaxe, you can pass a custom pattern as env variable like this:

pattern="['\"](fa[a-z])-([a-z-]+)['\"]" npm explore @adambh/vue-fontawesome-autogen -- npm run gen

or (for the automated version):

var WebpackBeforeBuildPlugin = require('before-build-webpack');
// ...
  module: {
    plugins: [
        new WebpackBeforeBuildPlugin(function(stats, callback) {
            const {execSync} = require('child_process');
            console.log(execSync(
              'npm explore @adambh/vue-fontawesome-autogen -- npm run gen',
              { env: { pattern: `['"](fa[a-z])-([a-z-]+)['"]` } }
            ).toString());
            callback();
        }, ['run', 'watchRun'])
    ]
  },
// ...

-------------------------------------------------------------8<-------------------------------------------------------------


I guess we could merge this PR now? 🙏

[GTANAdam]

@M1CK431 Greetings! My apologies for the late response as I have just checked my email inbox.
Thank you for investing your time into improving this tool and adapting it to fit everyone's needs.
So far, the changes look solid! allow me a day or two to test it and I'll have it merged.

cheers!

[M1CK431]

Hi @GTANAdam,
Any update regarding your test?

[GTANAdam]

Hi @GTANAdam,
Any update regarding your test?

Greetings @M1CK431
Here's a follow up based on my tinkering with the pull request:

• The proposed changes introduce a different syntax than already established, this comes as a breaking change that messes with established projects. Although It is understood that the change brings the tool closer to the default standards of vue-fontawesome but not everyone agrees with them, they make specifying an icon from the library cumbersome which can/could have been avoided.
• The proposed solution to allowing normal syntax brings additional hassle to developers. should be avoided.

Based on the stated above, I have concluded that the following points would solve multiple problems and keep everyone happy:

• Avoid breaking changes at all costs. which means, no end-user project reconfiguration after introducing these changes/updates.
• Allow the coexistence of both syntaxes without introducing additional parameters by parsing both syntaxes at once, for example: ['"](fa[a-z])-([a-z-]+)['"]|\[['"](fa[a-z])['"], *['"]([a-z-]+)['"]\]

Once we have the changes on par with the stated requirements, the pull request will be merged with master.

[phuze]

@M1CK431 Would you consider releasing your package to NPM? I think there is a fundamental difference of opinion here, and the code change is substantive enough that I believe it would be worth while, as opposed to trying to reconcile the two projects.

[M1CK431]

Hi @phuze,

As far as possible, I prefer to avoid forking a project because I think it's better for the open source community to works all together on a single project instead of wasting time and resource on multiple projects which reach the same goal.

However, after the last comment of @GTANAdam, I'm thinking about it very seriously, because IMHO I have handled the need of @GTANAdam to deal with custom syntaxe (and even I have provided a better implementation of the custom component in the README, despite I'm personally against it).

Now @GTANAdam you have to take a decision:

• if the current state of this PR is enough for you, please merge it right now
• if not, I'm really sorry but I will just create my own NPM package (with credits to this repository as "inspiration")

Please @GTANAdam it's not a big deal to add an env var... In addition it's well documented in the README... 🙏

@phuze My own deadline for this decision is around the end July.

[GTANAdam]

Hi @phuze,

As far as possible, I prefer to avoid forking a project because I think it's better for the open source community to works all together on a single project instead of wasting time and resource on multiple projects which reach the same goal.

However, after the last comment of @GTANAdam, I'm thinking about it very seriously, because IMHO I have handled the need of @GTANAdam to deal with custom syntaxe (and even I have provided a better implementation of the custom component in the README, despite I'm personally against it).

Now @GTANAdam you have to take a decision:

• if the current state of this PR is enough for you, please merge it right now
• if not, I'm really sorry but I will just create my own NPM package (with credits to this repository as "inspiration")

Please @GTANAdam it's not a big deal to add an env var... In addition it's well documented in the README... 🙏

@phuze My own deadline for this decision is around the end July.

Greetings @M1CK431,

I do agree with you on the fact that collaborating would solve fragmentation issues but I disagree on the fact that a PR should decide the future of the project by a submission of one sided solutions. here are a couple of reasons why and what we should stick to in order to solve issues.

• I, unfortunately, do not find the proposed approach to solving custom syntax to be appropriate. if you are willing to improve, then it will be more than welcome.
• I strongly insist on the compromises stated in my prior response to finding a middle point without breaking changes.

I personally use this tool in my projects and I am not willing to break changes for cosmetical purposes.

[M1CK431]

Hi @GTANAdam ,

I'm a bit surprised about that "fear" of the breaking change. I have provided clear instruction to easily handle it. In your case it means:

• add a (very small, provided) component in your code base (because you have made the personal choice to use a custom syntaxe)
• update your webpack config to add an env var (provided in the README, anyway you will have to update it to use the watchRun hook so no real extra work here)

And so it should take less than 5 minutes.

I can't agree with your point of "one sided solution" because I have added an easy way to handle custom syntaxe (with example in the README). From my point of view, the "futur of the project" is decided every time a contributor make a PR (in fact it's exactly what we are doing right now 😉).

I personally use this tool in my projects and I am not willing to break changes for cosmetical purposes.

Here is the root issue I guess. IMHO a lib like this one should handle the official vue-fontawesome syntaxe by default first, and optionally provide a way to be customized for the few people which absolutely want a custom syntaxe. It's exactly what I understand while reading:

As per demand from the Vue community using vue-fontawesome, this script was written to make managing imported fontawesome icons in VueJS uncumbersome. It implements simple parsing techniques that would generate a file that imports all of your used icons without having to manage them every single time.

AFAIK, never, never in the "Vue community demand" there is something about handling a custom syntaxe.

To conclude, you don't want to be annoyed for cosmetical purposes? Others think the same about there projects! So please, don't "force" everyone to use a custom syntaxe while the "Vue community demand" is only about managing imported fontawesome icons 🙏

[GTANAdam]

Hey @M1CK431,
I'm all open arms, all about collaboration and we should clearly find a compromise, which is not impossible.

Alright, so about the custom syntax.. I personally do not like the vue-fontawesome's syntax, so I decided to go with the vanilla fontawesome's way which, in my opinion, is much better. But okay, I get it, different tastes, different approaches, we should have an easy way to have both worlds existing at once without forgetting that the goal is to maintain compatibility which means, no breaking changes and updating should be 0-config.

So, here's the following propositions:

• Utilize smarter parsing techniques which would automatically recognize both syntaxes, for example as I had proposed in my earlier comment the following regex: ['"](fa[a-z])-([a-z-]+)['"]|\[['"](fa[a-z])['"], *['"]([a-z-]+)['"]\], which is compatible with both syntaxes.
  

• Or, as you have proposed, utilize the current parsing technique and have a webpack config which requires users to configure, thus breaking the 0-config updating rule, unless the custom syntax remains the default one.

[M1CK431]

Hi @GTANAdam ,

I'm all open arms, all about collaboration and we should clearly find a compromise, which is not impossible.

I'm really happy to read that, thanks!

the vanilla fontawesome's way which, in my opinion, is much better.

I'm align with you and to be honest I was not very happy nor convince when font awesome made this change, but in fact our personal opinion doesn't matter here. Because we haven't the power to change the official syntaxe, we have to follow 🤷‍♂️

Now let's speak about your proposition:

1. changing the regex with a more complexe one (to handle both syntaxe by default) has performance implication because the more the regex is complexe, the more time will be needed for the parsing.
2. It break the current implementation (and it seems not trivial to fix):

s = `<fa :icon="['fad', 'trash']" class="mt-1" />` // sample string using official syntaxe
re = new RegExp(/['"](fa[a-z])-([a-z-]+)['"]|\[['"](fa[a-z])['"], *['"]([a-z-]+)['"]\]/, "g") // your proposition
re2 = new RegExp(/\[['"](fa[a-z])['"], *['"]([a-z-]+)['"]\]/, "g") // current implementation

console.log([...s.matchAll(re)][0])
["['fad', 'trash']", undefined, undefined, "fad", "trash", index: 11, input: "<fa :icon=\"['fad', 'trash']\" class=\"mt-1\" />", groups: undefined]

console.log([...s.matchAll(re2)][0])
["['fad', 'trash']", "fad", "trash", index: 11, input: "<fa :icon=\"['fad', 'trash']\" class=\"mt-1\" />", groups: undefined]

As you can see, important values (fad and trash) aren't at the same place in the array depending of the syntaxe used in the project. At least for now, I don't know how to handle that easily 😞

3. font awesome already change the syntaxe one time and we can't know if they will not change it again in the futur. If it happens, the community (aka "we") will have to update this package in order to switch on that potential new syntaxe. It will not be instant. Providing a way for the end user to update the regex in the webpack config offer a workaround for that kind of situation, until we could update this package.
4. I'm using the official syntaxe, you are using a custom one. So we could imagine that another guy want to use another custom syntaxe (for example it could be fad trash or fad_trash or whatever else). If we hard-code the regex, that guy is fucked.

For all these reasons, I have handle your request of custom syntaxe support with an env var in the webpack config. I hope I have convinced you of the benefits of this method, despite the slight breaking change it implies.

[GTANAdam]

Greets @M1CK431,

changing the regex with a more complexe one (to handle both syntaxe by default) has performance implication because the more the regex is complexe, the more time will be needed for the parsing.

This is irrelevant, performance isn't the critical aspect of this tool and nor it would cause any noticeable lag with the worst possible regex code

As you can see, important values (fad and trash) aren't at the same place in the array depending of the syntaxe used in the project. At least for now, I don't know how to handle that easily 😞

Well, that's due to different groupings of matches, I couldn't figure out a better way to have the regex output the same groups for different combinations at the moment, I'll look into it again later. for now, this can be trivially fixed by filtering the array.

const s = `<fa :icon="['fad', 'trash']" class="mt-1" />`;
const re = new RegExp(/['"](fa[a-z])-([a-z-]+)['"]|\[['"](fa[a-z])['"], *['"]([a-z-]+)['"]\]/, "g");

console.log([...s.matchAll(re)][0]);
// result:   [ "['fad', 'trash']", undefined, undefined, "fad", "trash" ]

console.log([...s.matchAll(re)][0].filter(e => e != null));
// result:   [ "['fad', 'trash']", "fad", "trash" ]

[M1CK431]

And what about point 3 and 4?

[GTANAdam]

And what about point 3 and 4?

Sure, as long as those introductions are optional, it's always welcome!

[M1CK431]

Ok so, just to be sure, your requested change now are:

• use your regex (+ filter)
• keep the overriding feature through webpack config

After that you will merge this PR?

[GTANAdam]

Ok so, just to be sure, your requested change now are:

• use your regex (+ filter)
• keep the overriding feature through webpack config

After that you will merge this PR?

If it fits the previously discussed requirements, then I would have no reason not to merge the PR 🤷

[M1CK431]

You will still need to add a (very small, provided in README) component in your code base so it's a (kind of) breaking change.
Are you okay with that?

[GTANAdam]

You will still need to add a (very small, provided in README) component in your code base so it's a (kind of) breaking change.
Are you okay with that?

What component exactly? As far as I'm aware, with the improved regex, the vue-fontawesome syntax requires no shim to operate and for the normal fontawesome syntax, it requires the Fa.vue shim component. that's the difference.

So including the imported component can be optional.

[M1CK431]

It's not about parsing, it's the component used to transform the custom syntaxe to the official one. Please read the "Customization" section already provided here.