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

(Pro, Kits): @ts-ignore & Type IconDefinition is not assignable to type IconProp #423

Open
NabilNaou opened this issue Feb 6, 2024 · 6 comments

Comments

@NabilNaou
Copy link

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 from a kit using Explicit Reference. Example:

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.

@NabilNaou
Copy link
Author

Another thing that im unsure about

image

modules/kit/custom.d.ts throws this error when opened. Using the earlier describes ts ignore etc it does not influence my build. However, i thought this might be related / important to know. Common types is downloaded and not duplicated.

@devoto13
Copy link
Collaborator

devoto13 commented Feb 8, 2024

@robmadole Will you be able to look into this?

@NabilNaou If you can provide a minimal repro, I can also dig into it. I unfortunately don't have access to Kits myself now, so will need a problematic kit to investigate. You can generate one (1-2 dummy/custom icons should be sufficient) and share it as a ZIP. Nevermind, got it sorted out!

@robmadole
Copy link
Member

@NabilNaou this is a bug. I'm working on a fix now.

@robmadole
Copy link
Member

@devoto13 If you give me your email address in our Slack channel I'll give you a contributor (comped) account if it's useful.

@NabilNaou
Copy link
Author

@NabilNaou this is a bug. I'm working on a fix now.

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!

@robmadole
Copy link
Member

@NabilNaou I don't have an ETA yet on this.

devoto13 added a commit to devoto13/angular-fontawesome that referenced this issue Apr 17, 2024
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 added a commit to devoto13/angular-fontawesome that referenced this issue Apr 17, 2024
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 added a commit that referenced this issue Apr 18, 2024
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.
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

No branches or pull requests

3 participants