jacobparis-insiders / sly

Monorepo for Sly CLI
https://sly-cli.fly.dev/
153 stars 11 forks source link

feat: add material-design-icons to registry #12

Closed wKovacs64 closed 8 months ago

wKovacs64 commented 8 months ago

@jacobparis Material Design Icons have five different variants (filled, outlined, round, sharp, and two-tone). I considered adding each one to the registry individually for easier consumption (e.g., developers will find the icon names like mobile_friendly and if these were separate registry entries, they could use Sly to add that name exactly as opposed to mobile_friendly-filled), but I followed suit in regards to how you handled the two variants in heroicons. Open to changing it, though.

Also, it may be worth noting that the source for these icons is technically a third-party repository. The reason for this is that the official Google repository is too large for the rest.git.getTree Octokit API used in github.server.ts as it contains all related assets for multiple platforms (not just SVGs). I looked into alternative ways to get this much data, but everything I found required a GitHub API token and would still run into rate limiting without some throttling. Luckily, some kind soul has broken out the various files into individual repositories (e.g., https://github.com/marella/material-design-icons) that automatically update whenever Google updates the source repository. Google links to these from their official repository (https://github.com/google/material-design-icons#npm-packages), so I feel fairly confident using them over some other solution.

wKovacs64 commented 8 months ago

Material Design Icons have five different variants (filled, outlined, round, sharp, and two-tone). I considered adding each one to the registry individually for easier consumption (e.g., developers will find the icon names like mobile_friendly and if these were separate registry entries, they could use Sly to add that name exactly as opposed to mobile_friendly-filled), but I followed suit in regards to how you handled the two variants in heroicons. Open to changing it, though.

And actually, filled is kind of the de facto default, so I could also be convinced to drop the -filled naming suffix (but keep the others). That way, you're only required to add a -whatever variant modifier if you want something other than filled. ¯\(ツ)\

jacobparis commented 8 months ago

Hey thanks for this, this looks great!

Yeah I don't love having duplicates for every possible variant, it was ok with heroicons since there were only two (solid and outlined) but I don't think it scales very well

I'm thinking of adding a "variants" option to the registries, and then we could just fetch the matching variants. That pattern would also work with utility function libraries (like Just) where users may want ESM or CJS variants

This looks good as-is though, I'll merge it through and deploy tonight (still haven't set up auto deploys)