[Icons] Improve cache warmup by looking at every string

[kbond]

Q	A
Bug fix?	no
New feature?	yes
Issues	n/a
License	MIT

Improve working with dynamic icon names (inspired by https://v2.tailwindcss.com/docs/optimizing-for-production#writing-purgeable-html)

{# This will be cached #}
{{ ux_icon('flag-fr') }}

{# This will NOT be cached #}
{{ ux_icon('flag-' ~ locale) }}

{% set flags = {fr: 'flag-fr', de: 'flag-de'} %} {# both "flag-fr" and "flag-de" will be cached #}
{{ ux_icon(flags[locale]) }}

Closes #1601

[kbond]

@smnandre made some fair points about performance/security.

Regarding security: I want to clarify that only strings that match 'foo:bar' will be used to make http requests to iconify. 'foo:bar' will create an http request with this value in the request path but 'something-sensitive' will not. However, 'something:sensitive' would create an http request with this value in the request path.

I'd like to find a compromise that we're all ok with, some thoughts:

• opt-in to this much looser string check
• configure specific paths/globs (like tailwind does)

[smnandre]

We can add a failsafe: store locally the list of available icon prefixes.

Then we reverse the logic:

• collect all the icons as you do now
• list the prefixes
• download the list of icons for each prefix

--> finally download the icons.

It can be even faster in the end, because we can group parallelize + group icon downloads (up to 500 per request)

[smnandre]

I'm reading the doc

During development, if you modify an icon, you will need to clear the cache (bin/console cache:clear) to see the changes.

Could we add a isFresh / mtime check about that ? What do you think ?

[kbond]

I believe the security concerns have been alleviated. I now only make the http request if the prefix is valid. 'something:sensitive' will no longer create a request.

Could we add a isFresh / mtime check about that ? What do you think ?

I'd prefer to avoid this complexity until this is determined to be a problem. I feel this situation is be rare.

[smnandre]

If (i insist on if) this become the bottleneck, we have a card in hand: grouping icons per set https://iconify.design/docs/api/icon-data.html#query

[smnandre]

Should we exclude the Bundle ?

[kbond]

What bundle?

[smnandre]

At this point, i think Twig loader has been registered with all the bundles of the app. So it contains all the files in "vendor/acme/*-bundle/Resources/templates" and we should (imho) not go there to extract things.

Imagine a twig file there with a list of icons (think any template using JS icons)... the probability to have 'bi:check' string is far from null.

We even could see this as a vector "attack", a single file in twig from a 3rd party bundle would make you downlaod icons locally / generate a lot of download.

Oh and i'll check something about that we may have a problem there.

So here i'd like us to only look in userland files :)

[kbond]

What if you had a bundle of that uses icons that you wanted to use? I prefer to keep it simple and adjust when/if we come across these type of problems.

[smnandre]

I love this!