-
Notifications
You must be signed in to change notification settings - Fork 155
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
(Pro, Kits): @ts-ignore & Type IconDefinition is not assignable to type IconProp #423
Comments
@robmadole Will you be able to look into this?
|
@NabilNaou this is a bug. I'm working on a fix now. |
@devoto13 If you give me your email address in our Slack channel I'll give you a contributor (comped) account if it's useful. |
Awesome thanks! No rush but I was wondering if you have an ETA? I'm hoping to implement this in a big project and it'd be a pain to go back later and fix all the ignores. Again thanks for the quick reply! |
@NabilNaou I don't have an ETA yet on this. |
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.
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.
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 #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.
@robmadole Any update? |
We haven't forgotten. Our engineering team is currently working a bug-fix 2-week sprint and this issue is high on the list to fix. |
The issue has been fixed. Note that to get the fix for an existing kit package, you'll need to regenerate the NPM package (by e.g. adding a new icon to the Kit). Then update the package in your project to the latest version and the type errors should be gone.
|
Describe the problem
When using FA Pro, and in particular kits with explicit reference, the import statement will not work unless you add // @ts-ignore. It complains that it cannot find the module.
import { placeholder } from '@awesome.me/kit-CODE/icons/kit/custom';
I have tried re-installing multiple times. The only fix that works is adding a @ts-ignore for the import statement. The error in the HTML can be ignored as it still builds regardless.
Note that the free FA works fine.
What did you expect?
The import to work without a ts ignore
The HTML to not throw the error; Type IconDefinition is not assignable to type IconProp
Reproducible test case
import { faPlaceholder } from '@awesome.me/kit-CODE/icons/kit/custom';
faPlaceholder = faPlaceholder ;
<fa-icon [icon]="faPlaceholder">
The import should complain about not finding the path unless ts ignored, and the html should complain about IconDefinition not being assignable to iconprop.
I have checked my common types, packages, re-installed multiple times, etc. Im quite convinced my installation of FA pro itself is correct.
The text was updated successfully, but these errors were encountered: