[Icons] ux:icons:lock showing 200 when really a 404

tacman commented 1 week ago

This works as expected.

bin/console ux:icons:import tabler:bugxxx

 // Importing tabler:bugxxx...                                                                                          

 [ERROR] The icon "tabler:bugxxx" does not exist on iconify.design.

But if I have some twig

{% set iconMap = {
    bug : ux_icon("tabler:bugxxx"),
    url : ux_icon("tabler:world-www"),
    api : ux_icon("tabler:api"),
    wiki : ux_icon("simple-icons:wikidata"),
    github: ux_icon("tabler:brand-github"),
    ds1: ux_icon("tabler:database"),
    citation: ux_icon("formkit:url"),
    rights: ux_icon("tabler:copyright")
}
%}

And then run

bin/console ux:icons:lock -vv 

 // Scanning project for icons...                                                                                       

11:48:07 INFO      [http_client] Request: "GET https://api.iconify.design/tabler/bugxxx.svg"
11:48:07 INFO      [http_client] Response: "200 https://api.iconify.design/tabler/bugxxx.svg"

 [OK] Imported 0 icons.                                                                                              `

It shouldn't be reporting a 200, and I'm hoping instead that it reports an error. I have a situation where it's running locally but not on production, but I can't figure out which icon I forgot to import.

smnandre commented 5 days ago

It is in fact a 200, so the http client reports this status.

But i see your point about reporting icons not found.

tacman commented 5 days ago

Yes, but I think iconify should be handled, as they return a status code 200 but a text body of 404.

Opened an issue at https://github.com/iconify/api/issues/30

smnandre commented 1 day ago

I forgot... but the "200" is in fact handled in the code. What you saw is just the HttpClient debug log.

When the icon is not found, the Iconify services does throw a IconNotFoundException. But in the lock command (and the cache warmer) we decided to ignore these, because the service extracting icon names from your code is a bit... greedy by design.

To illustrate, on ux.symfony.com when we run the "lock" command, among others, the following icon "names" are returned:

og:title
twitter:card

These are just "meta" tags : we dont want to throw an exception here.

Now, i see different things we could do:

display the icons "not found" in the command log instead of just the downloaded ones
add a --strict mode to throw during the command (depending on your app this could do the job)
throw if the icon does not exist but the iconset does (so "lucide:circleee" would throw / return an error code, but "lucidee:circle" would not)

What do you think ?

tacman commented 1 day ago

Sometime in the next few days the deployment should be fixed and it will return a 404 instead of a 200.

wget https://api.iconify.design/tabler/bugxxx.svg
--2024-10-20 09:12:00--  https://api.iconify.design/tabler/bugxxx.svg
Resolving api.iconify.design (api.iconify.design)... 172.67.71.159, 104.26.13.204, 104.26.12.204, ...
Connecting to api.iconify.design (api.iconify.design)|172.67.71.159|:443... connected.
HTTP request sent, awaiting response... 200 OK
Length: 3 [application/json]
Saving to: ‘bugxxx.svg’

bugxxx.svg                                              100%[=============================================================================================================================>]       3  --.-KB/s    in 0s      

2024-10-20 09:12:00 (4.47 MB/s) - ‘bugxxx.svg’ saved [3/3]

A question related to lock: how can I make sure icons from a bundle are probably loaded? Should I install them in the bundle assets, or flag them in some way to that lock picks them up and the application should install them.

I'm sure there's some way in the compiler pass to make all that happen, though I'm never quite confident of how to properly configure it so that two bundles can work together.

A --strict option and displaying more info in the log seem like solid ideas.

smnandre commented 1 day ago

(regarding my previous message, This could look like this)

php bin/console ux:icons:lock -v

php bin/console ux:icons:lock -vv

smnandre commented 1 day ago

Sometime in the next few days the deployment should be fixed and it will return a 404 instead of a 200.

This is not a good news. It was documented like this, so we coded it to expect a 200. This will probably break things for a lot of users. we need to send a patch ASAP.

So we'll talk about this later.

smnandre commented 1 day ago

how can I make sure icons from a bundle are probably loaded

First thing is, a bundle has almost no way to be 100% if the user uses it, in what environment, if the ux:icon is installed / configured / etc. And not everyone uses the "lock" command, or has network available during cache:warm. So I'd advise you to be very explicit in documentation / install notes.

Now, if your bundle uses UX Icons, and you have tested each of them does exist on iconify, they will be found by the icon names extracting servie (as every template --bundle ones included-- is parsed)

tacman commented 1 day ago

I use KnpDictionary to associate icons with actions, so they won't be found unless I explicitly add them somehow. I've required ux_icon in my bundle, and create the map in the dictionary config.

knp_dictionary:
  dictionaries:
    action_icons:      # your dictionary name
      type: 'key_value' # your dictionary type
      content:          # your dictionary content
        edit: "tabler:edit"
        delete: "tabler:trash"
        show: "tabler:eye"

In my menu bundle, I want to display those icons next to the link. But obviously ux_icons has no way know those values are icons.

My hack solution is to create a command that looks for those icons and runs the icon install command. But that's really what lock does, I'm just wondering if there's some other way to let ux_icon know that I have these icons I want locked. Maybe there's an event somewhere that says "get me the icons I need to lock"?

smnandre commented 1 day ago

There is none today.

I think we had this idea user should run this command and no vendors.

But you make a very good point regarding this config and icon "discoverability".

We can think about it.

But again lets keep in mind many users will let the icon beeing rendered on the fly (first time only they are cached after)... and this is not a problem in 99% of the time :)

tacman commented 1 day ago

But again lets keep in mind many users will let the icon beeing rendered on the fly (first time only they are cached after)... and this is not a problem in 99% of the time :)

Yes, but somehow I got burned where my app wouldn't start, and after some debugging, the problem was that an icon that wasn't discovered (deep in some bundle) and was being discovered via a terribly slow internet connection.

I've set my config to avoid this, I think.

  iconify:
      enabled:              true

      # Whether to download icons "on demand".
      on_demand:            false

    # Ignore error when an icon is not found.
    # Set to 'true' to fail silently.
  ignore_not_found:     true

Although I haven't finished the migration, I love ux_icons, I've finally started putting icons in more places.

In another example of mapping icons, I also associate an icon with every class that has CRUD routes. So in addition to the action icons above, every class has a icon that I can use. So if I'm trying to create a dashboard showing how many items each entity has, like this, I can normally it by saying "loop through each entity class, get the repository and count, and the get icon associated with the class, and pass it to the template"

All this is to say, ux_icons rock, and Symfony developers are lucky to have such a tool. I can easily imagine more bundles integrating it. For example, it seems natural that the workflow bundle optionally supported icons for transitions and places. It can be put in the workflow metadata, or (hackish) in the translation file (marking.complete.icon|trans) or in a map like knp_dictionary. A tool to expose those icons (aka make them discoverable) would be welcome.

I realize at this point I should make this a feature request, as it's deviated quite a bit from the initial topic.

symfony / ux

[Icons] ux:icons:lock showing 200 when really a 404 #2272