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

Introduce more permissive icon types #436

Merged
merged 1 commit into from
Apr 18, 2024

Conversation

devoto13
Copy link
Collaborator

With this change angular-fontawesome exposes more permissive variants of some types (IconPrefix, IconName, IconLookup, IconDefinition and IconPack) from the fontawesome-svg-core. These new types allow arbitrary string values as icon name and icon prefix while maintaining auto-completion for the core Font Awesome icons.

Firstly, this makes it possible to define and use custom icons without any casts, thus implementing part of the #172 and addressing part of the #423 (Kit packages with custom icons). The documentation for custom icons is coming later in a separate PR.

Secondly, this makes angular-fontawesome resilient to multiple instance of fontawesome-common-types packages, thus helps with issues like #125.

The drawback of this change is that if a user makes a typo in a core icon name or an icon prefix it will no longer produce a compile-time error, but will throw a runtime error instead. However, this trade-off seems to be overall the best option. Considerations:

  1. To keep type safety while supporting custom icons, we'll need to somehow extend the mentioned icon types. It was investigated in the Better support for custom icons #172 (comment). The conclusion is that it requires very convoluted code to be added to the project and therefore is undesired.
  2. For the explicit reference approach, the type safety/completion is not really needed as icon definitions are imported as runtime symbols and importing a symbol which does not exist will always result in complication error.
  3. For the icon library approach, the type safety isn't perfect either. While it will catch cases where one specifies a completely incorrect icon name, it does not catch all problems. Icons are added to the library at runtime and if an icon name is correct, but the icon was not added to the library it will still result in a runtime error.

With this change `angular-fontawesome` exposes more permissive variants of some types (`IconPrefix`, `IconName`, `IconLookup`, `IconDefinition` and `IconPack`) from the `fontawesome-svg-core`. These new types allow arbitrary string values as icon name and icon prefix while maintaining auto-completion for the core Font Awesome icons.

Firstly, this makes it possible to define and use custom icons without any casts, thus implementing part of the FortAwesome#172 and addressing part of the FortAwesome#423 (Kit packages with custom icons). The documentation for custom icons is coming later in a separate PR.

Secondly, this makes `angular-fontawesome` resilient to multiple instance of `fontawesome-common-types` packages, thus helps with issues like FortAwesome#125.

The drawback of this change is that if a user makes a typo in a core icon name or an icon prefix it will no longer produce a compile-time error, but will throw a runtime error instead. However, this trade-off seems to be overall the best option. Considerations:

1. To keep type safety while supporting custom icons, we'll need to somehow extend the mentioned icon types. It was investigated in the FortAwesome#172 (comment). The conclusion is that it requires very convoluted code to be added to the project and therefore is undesired.
2. For the explicit reference approach, the type safety/completion is not really needed as icon definitions are imported as runtime symbols and importing a symbol which does not exist will always result in complication error.
3. For the icon library approach, the type safety isn't perfect either. While it will catch cases where one specifies a completely incorrect icon name, it does not catch all problems. Icons are added to the library at runtime and if an icon name is correct, but the icon was not added to the library it will still result in a runtime error.
@devoto13 devoto13 merged commit 9e9798c into FortAwesome:main Apr 18, 2024
3 checks passed
@devoto13 devoto13 deleted the bundle-types branch April 20, 2024 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant