Open thedavidhoffman opened 7 months ago
bleistift-zwei
commented
7 months ago If this feature request is accepted, I’d like to point out that #440 also asks for configuration options that determine whether an error is thrown or a message is logged to console. For consistency, we should probably use the same identifiers in both cases, so #440’s options aren’t called 'throwError' and 'warnInConsole' while this issue’s options are 'throw' and 'logConsole'.
devoto13
commented
7 months ago Are you aware that you can use FaConfig.fallbackIcon to prevent component from throwing when icon is missing?
Rendering component without any icon is not a valid use, so the proposal of changing error to warning does not sound good to me. However, I would be open to add an opt-in warning when fallback icon is rendered or perhaps expose a missingIconDefinition$ observable on the FaIconLibrary to make it more flexible 🤔
I would guess you were hit by this issue when using icon library approach, because with explicit reference approach missing icon should be caught by type-checking.
thedavidhoffman
commented
7 months ago @devoto13 we use a mix of the icon library as well as strong typing (imports in the typescript and just html markup for the fa-icon tag). I tried the FaConfig.fallbackIcon, but it didn't work. I still got an error thrown by faWarnIfIconDefinitionMissing. So I'm guessing the fallbackIcon doesn't cover the icon library approach.
I don't want to change the error to a warning, I'd like an opt-in config setting that faWarnIfIconDefinitionMissing() will respect where it will just write an error to the console. So the existing behavior of throwing an error will still be applied by default.
devoto13
commented
7 months ago I understand that you don't want to change the default, but I don't think that rendering an empty component (with a warning) is a good behaviour. Let me think a bit more about this issue. You're right that fallbackIcon does not work, it's indeed only applied if no icon is provided rather than when icon is provided, but is not added to the library.
devoto13
commented
6 months ago So my thinking is to add FaIconLibrary.onMissingIcon(func: (prefix: IconPrefix, name: IconName) => IconDefinition | null). This allows consumer to log a warning or do whatever other logic they want and/or return an icon definition to be rendered instead of the missing icon.
This should solve both use cases:
Describe the problem you'd like to see solved or task you'd like to see made easier
When an icon reference is missing, and you try to use that icon in markup, it will crash an Angular component.
I'd like an option to use a soft versus hard error. So that the function faWarnIfIconDefinitionMissing would either do a console.error or throw a new Error.
Is this in relation to an existing part of angular-fontawesome or something new?
It's related to Fort Awesome checking if an icon reference has been added.
What is 1 thing that we can do when building this feature that will guarantee that it is awesome?
It will make it awesome by ensuring the stability of an application's functionality in the event of a missing icon.
Why would other angular-fontawesome users care about this?
Others might care about this because right now a missing icon will crash a component. Others, like myself might prefer a component run without finding an icon, than crash because it can't find it.
On a scale of 1 (sometime in the future) to 10 (absolutely right now), how soon would you recommend we make this feature?
8 - because it should be relatively easy to implement, it's just adding an option for how the existing faWarnIfIconDefinitionMissing function operates.
Feature request checklist
Feature request: moar cowbell)