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

Subsetting Thoughts #151

Open
sandstrom opened this issue Jun 17, 2020 · 2 comments
Open

Subsetting Thoughts #151

sandstrom opened this issue Jun 17, 2020 · 2 comments

Comments

@sandstrom
Copy link

Awesome addon! 🏅

Some thoughts:

  1. Would be great if the syntax was agnostic to the package, i.e. I don't want to keep track of whether an icon comes from 'pro-light-svg-icons' or 'free-solid-svg-icons'.

  2. Would also be great if I could specify "all styles" or select number of styles, easily. Perhaps something like this:

{
  'adjust': true,
  'ambulance': ['solid'],
  // …
}
  1. Perhaps a simple script could be used to parse the template files and search for a string. Maybe it could be left up to the developer to define the function function(templateString) { return myFuncThatExtractsIconNames(templateString); }.That way you'd only do simple string parsing of the templates and could easily handle different syntax for the icons.
@jrjohnson
Copy link
Collaborator

Thanks @sandstrom. For 1/2 that seems to lead devs towards including more icons than they need as the easiest path forward is to specify all. However I'm not really oposed, just a though. Maybe the syntax

module.exports = function() {
  return {
    'pro-light-svg-icons': [
      'adjust',
      'ambulance',
      'pencil-alt'
    ],
  'all-styles': [
    'pencil'
  ]
  };
};

Would help you and make it clearer what happens if you use that key? If so I'd be ore than happy to look at a PR that adds that.

3 is the real kicker. We should be able to parse templates for icons unfortunately the current ember build doesn't give us a path to walk the AST for templates before outputting assets (or if it does I haven't figured it out!).

I have high hopes for a combination of two ember enhancements that will make this a non-issue very soon.

  1. Embroider builds which have some options around macros that may make it easier to include assets in the final output. I'm not sure yet and I'm waiting to see how this evolves.
  2. RFC 496 proposes some changes to the template system that may make it possible to do something like:
---
import { pencil } from '@fortawesome/free-solid-icons';
---
<FaIcon @icon={{pencil}} />

Which would alleviate the need for any kind of pre-processing on the part of this addon at all. This is currently possible by importing the icon in the component backing class, but we don't really talk about it much because the ergonomics aren't great. However with the addition of embroiderer and a the possibility of tree-shaking in our immediate future this may be the best way to move forward since it ties the icon import directly to the template where it is needed.

That is all to say... thanks for sharing thoughts, I'm very much open to making some changes here to improve the situation, I just haven't come up with any that I'm super confident will help.

@sandstrom
Copy link
Author

Thanks for taking time responding!

  1. Agree with problem including more icons than needed. It's something to consider.

  2. Good point, maybe something like this:

{
  'adjust': ['all'], // explicit all instead
  'ambulance': ['solid'],
  // …
}

Problem with 'pro-light-svg-icons' is that it's less convenient. I want a developer (that didn't setup FontAwesome) to not care about these details. Should just be able to set a string in the UI + add the icon to the inclusion list and be done with it.

  1. That sounds interesting, especially the Embroider stuff. Personally I wouldn't want to use template imports for every single icon, I think it's too verbose. I'd prefer {{icon 'pencile'}} since the syntax is leaner.

My idea around (3) was that the script would be run manually, sort of like a sanity check + for setup. You'd run the script over all your template strings and extract the used icons. These would be e.g. printed to the console, and you'd then copy-paste them into config/icons.js or similar. The script could then easily be modified to look for different syntax etc. By exposing this as a lower-level primitive it's also easier to adapt to different code bases, and different delivery mechanism.

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

2 participants