-
-
Notifications
You must be signed in to change notification settings - Fork 146
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
Implement icon caching #121
Conversation
Co-authored-by: Mark van den Broek <mvdnbrk@gmail.com>
I'm currently looking for people who want to help me test this PR. I've seen great performance improvements when caching is enabled. You can test by requiring the library like: composer require blade-ui-kit/blade-icons:"dev-caching as 0.5.1" And then require some large third party icon libraries: https://github.com/blade-ui-kit/blade-icons#third-party. It would be great if anyone could test this on a production (or staging) server to see the actual impact in a real app. Please note that this PR includes all the changes for the upcoming 1.0.0 release. There's no upgrade guide yet so you might run into a couple of things if you try to use this in an app that already uses a previous Blade Icons release. |
Hi, I just did a quick test with davidhsianturi/blade-bootstrap-icons and his famous 1270 icons and the result is amazing. The average saving in loading time is 67.5% !!! It's even 2% faster than the custom library I made which barely contained 46 icons. This is a great step forward which will make our life easier on managing icons. Well done and thank you a thousand times... Regards, |
I just pulled in all of of font-awesome, Locally it's pretty fast, there is no change, on the server it has shown a speed decrease. However, I may have to run the icons:cache on the server as well as part of my build script. I noticed when I ran the command there were no changes to git in my file-system |
Noticed something, as I added more libraries this came to light, you cant have a dash in the prefix for example, this
While I'm converting all my custom collection of icons back into their original libraries, I ran into that when I added heroicons Thanks for the great project and the awesome fix. |
@aurelien31 @roni-estein thanks for testing! @roni-estein ah that's a good catch. Yeah the dash is the delimiter which we use to slit the prefix from the rest of the icon name so it's a reserved character. I've added a note about this in the docs. Thanks! 309fca8 |
Just wanted to say I'm really, really happy with this! We've had some weird performance issues locally. Using this command we went from ~8 seconds to sub-second performance 🤯 . On production it didn't save as much, but I reckon we doubled in performance. |
This PR adds a
icons:cache
andicons:clear
command as well as a manifest implementation. It solves the performance issues when working with large icon sets by caching discovered icons.Thanks to @mvdnbrk
Closes #71
Todo: