search

phosphor-icons / phosphor-elm

A flexible icon family for Elm
https://package.elm-lang.org/packages/phosphor-icons/phosphor-elm/latest/
MIT License
18 stars 2 forks source link

Bundle size #5

Open miniBill opened 3 months ago

miniBill commented 3 months ago

While the current API feels very nice, it has the disadvantage that live code inclusion only works at the icon level, and not at the style level.

So if I use - say - Phosphor.cloud, that includes all styles in the final JS bundle.

I wonder if it would be better to have something like https://package.elm-lang.org/packages/lattyware/elm-fontawesome/latest/FontAwesome-Solid, with one module per style.

I'm up for writing a PR for this, if you want. If so, I'd be tempted to use elm-codegen instead of the current JS.

Let me know:

  1. what do you think of this issue
  2. if you're interested in a PR, and if so 2.1. if you would be fine with a PR using elm-codegen
rektdeckard commented 3 months ago

I personally tend to use 2-3 weights when building applications: Regular and Fill weights to denote statefulness (active/inactive) of icons at a larger size, and often using Bold for small indicator icons. This works well for my use case, and I like the comfort of how it is set up now.

I'm not totally against your proposal, but I'd want to hear it from more people before agreeing to a big breaking change like this, even in Elm where breaking changes are pretty easy. How many icons are you using in your application that the extra cost per icon is too much? Can you share some numbers? Would love to see real-world savings, as I not sure this is the source of any real issue.

rektdeckard commented 3 months ago

To your other point though, I'd be happy to move the codegen to elm-codegen. This was initially set up at the same time as some of our other JS libs and it made sense at the time to copy and tweak the stuff I had already written.

miniBill commented 3 months ago

I see how if you regularly change the weight then the current API is nicer.

I wonder if you could simply have the best of both worlds by having the per-weight modules in addition to the current single module.

rektdeckard commented 3 months ago

I'm cool with that, if you'd like to make the change! Are you thinking to expose as a separate or child module so there aren't namespace collisions?

miniBill commented 3 months ago

I'd call them Phosphor.Regular, Phosphor.Duotone, ...