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

Implement icon caching #121

Merged
merged 6 commits into from
Mar 21, 2021
Merged

Implement icon caching #121

merged 6 commits into from
Mar 21, 2021

Conversation

driesvints
Copy link
Member

@driesvints driesvints commented Mar 14, 2021

This PR adds a icons:cache and icons: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:

  • Write docs

@driesvints
Copy link
Member Author

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.

@aurelien31
Copy link

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,
Aurelien

@roni-estein
Copy link

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

@roni-estein
Copy link

Noticed something, as I added more libraries this came to light, you cant have a dash in the prefix for example, this

sets' => [
        'heroicons-solid' => [
            'path'   => 'resources/icons/hero-icons/solid',
            'prefix' => 'hi-s',
            'class'  => 'flex-shrink-0',
        ],
        'heroicons-outline' => [
            'path'   => 'resources/icons/hero-icons/outline',
            'prefix' => 'hi-o',
            'class'  => 'flex-shrink-0',
        ],
        'font-awesome-light'   => [
            'path'   => 'resources/icons/blade-fontawesome/light',
            'prefix' => 'fal',
            'class'  => 'fill-current flex-shrink-0',
        ],
        'font-awesome-reg'     => [
            'path'   => 'resources/icons/blade-fontawesome/regular',
            'prefix' => 'far',
            'class'  => 'fill-current flex-shrink-0',
        ],
        'font-awesome-solid'   => [
            'path'   => 'resources/icons/blade-fontawesome/solid',
            'prefix' => 'fas',
            'class'  => 'fill-current flex-shrink-0',
        ],
        'font-awesome-duotone' => [
            'path'   => 'resources/icons/blade-fontawesome/duotone',
            'prefix' => 'fad',
            'class'  => 'fill-current flex-shrink-0',
        ],
        'tabler-icons' => [
            'path'   => 'resources/icons/tablericons',
            'prefix' => 'tablericon',
            'class'  => 'stroke-2',
        ],
        'custom-icons' => [
            'path'   => 'resources/icons/custom',
            'prefix' => 'ci',
            'class'  => 'flex-shrink-0',
        ],

While I'm converting all my custom collection of icons back into their original libraries, I ran into that when I added heroicons
not sure that was intentional, but I noticed changing hi-o to hio and hi-s to his resolved this issue. It leaves some clarity but I can live with it for what I'm gaining.

Thanks for the great project and the awesome fix.

@driesvints
Copy link
Member Author

@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

@driesvints driesvints merged commit 18ff445 into main Mar 21, 2021
@driesvints driesvints deleted the caching branch March 21, 2021 14:33
@ju5t
Copy link

ju5t commented Sep 6, 2021

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants