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

Invalid Icon filenames, as defined/validated by Iconify, are silently failing #257

Open
Blackskyliner opened this issue Sep 19, 2024 · 4 comments
Labels

Comments

@Blackskyliner
Copy link

Blackskyliner commented Sep 19, 2024

While working on a new project with new developers we stumbled upon this silent failure.
We had Icons exported from Figma in the format:

  • iconCalendar.svg
  • iconPerson.svg
  • ...

Which were then loaded as customCollection via NuxtConfig

icon: {
    customCollections: [
      {
        prefix: 'theme-icon',
        dir: './assets/icons'
      },
    ],
  },

The message upon restart was reassuring that all icons were loaded and should be accessible.

Nuxt Icon loaded local collection theme-icon with 43 icons.

But we could not access them, or better said, we could only access the ones which only were lowercase. So I digged down the whole integration with the following waypoints:

In words:

  • The collection gets loaded and auto-detects all svg files within the collection folder. It also validates if the content is valid and loads them into the internal collection of nuxt-icon.
  • Then when called by the Component integration it gets converted/added to the Iconify component which then goes down its library path to add it to the/a custom collection for display.
  • This in fact validates the name given and will NOT add it if its invalid thus not displaying any icon with invalid names.
  • No error handling is done for this in the Component (addIcon in fact returns success which could be handled)
  • No validation is done when creating/loading the nuxt-icon collection.

Suggested resolution: For better DX there should also be a name validation when loading the collection and it should warn about all icons not following the convention. I can craft the corresponding PR.


Did I miss something elsewhere or is my deduction of the error correct?

@fahmifitu
Copy link

I've concluded that this module is not production-ready due to numerous errors. The naming convention is poorly thought out, making it impractical for use. In particular, it forced me to manually rename every icon individually, which is highly inefficient and frustrating.

@Blackskyliner
Copy link
Author

Blackskyliner commented Sep 20, 2024

Okay, so lets fix that then. It won't get "production-ready" if no one addresses the problems and tries to fix them.

To address this specific problem and make it more robust, instead of just warning about it, we could theoretically, in this case, transparently transform invalid names to be valid for the underlying iconify library. Thus we won't have the same limitations and less manual labor to fix the naming convention. As file naming in usage and detection would be the same transformation of the name and thus, within our library, should be mostly error/side-effect free.

@Blackskyliner
Copy link
Author

@antfu How should we proceed here?

I can supply an patch either way:

  • Adding warnings about invalid file names on startup
  • Transparently trying to fix the name to match convention on usage

@antfu
Copy link
Member

antfu commented Oct 28, 2024

Thanks for the detailed issue! Sorry I saw #265 first and already started the conversations, let's keep discussing there.

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

No branches or pull requests

3 participants