SAP / ui5-webcomponents

UI5 Web Components - the enterprise-flavored sugar on top of native APIs! Build SAP Fiori user interfaces with the technology of your choice.
https://sap.github.io/ui5-webcomponents/
Apache License 2.0
1.51k stars 262 forks source link

How to disable ui5-icons #8042

Closed vasicvuk closed 8 months ago

vasicvuk commented 8 months ago

Issue Description

I am trying to use UI5 web components without ui5-icon and I could not find any documentation if this is possible. In general, I got it to work by registring a custom ui5-icon web component, but I get the following warnings and errors in the console which are annoying.

Multiple UI5 Web Components instances detected.
"Runtime 0 - ver 1.20.1" failed to define 1 tag(s) as they were defined by a runtime of an older version "Older unknown runtime": ui5-icon.
WARNING! If your code uses features of the above web components, unavailable in Older unknown runtime, it might not work as expected!
To prevent other runtimes from defining tags that you use, consider using scoping or have third-party libraries use scoping: https://github.com/SAP/ui5-webcomponents/blob/main/docs/2-advanced/03-scoping.md.
No loader registered for the SAP-icons-v5 icons collection. Probably you forgot to import the "AllIcons.js" module for the respective package.

Anyway to remove this?

Issue Type

vladitasev commented 8 months ago

Hello,

ui5-icon is a direct dependency of a lot of core components (as they display icons as part of their internal structure), therefore it's very likely that if you imported some other components, ui5-icon will be imported as well transitively.

What you've done is the only possible solution, basically to register an empty class. The warning you see can be ignored. It simply says that a tag could not be defined (as it already was). But then, other components might look weird (f.e. a ui5-select without the arrow icon for the dropdown)

Is there a specific reason to want to disable ui5-icon?

Regards, Vladi

vasicvuk commented 8 months ago

Hi @vladitasev I want to use other icon set such as Material icons for example. Can you give me an example of registering this empty class?

I understand the warning but i like my console clean :)

vasicvuk commented 8 months ago

To add component works well with other icons as long as I register this custom web component replacing it

vladitasev commented 8 months ago

Hi @vasicvuk

Let me give you some more context. The ui5-icon component does not prevent you from using other icons, and does not bring the SAP icon collections along with itself. It's just the "vessel" to display the SAP-* icon collections, and by having it as part of your bundle, you're not additionally requiring any icons that you don't need.

Here is the article about icons (maybe you've seen it, but pasting here for completeness): https://github.com/SAP/ui5-webcomponents/blob/main/docs/1-getting-started/04-using-icons.md

Basically, you will bundle the actual SAP icons only if you have any of these 3 imports:

import "@ui5/webcomponents-icons/dist/AllIcons.js";
import "@ui5/webcomponents-icons-tnt/dist/AllIcons.js";
import "@ui5/webcomponents-icons-business-suite/dist/AllIcons.js";

or individual imports for a single icon.

But ui5-icon itself won't make your bundle significantly bigger.

So, in summary, if you prevent ui5-icon from registering, you won't shake off any excess icons from your bundle, as it doesn't bring any icons in the first place, but you will have side effects in a lot of other components.

Let's have ui5-select as an example. It has:

import Icon from "./Icon.js";
....
import "@ui5/webcomponents-icons/dist/decline.js";
....
import "@ui5/webcomponents-icons/dist/slim-arrow-down.js";
import "@ui5/webcomponents-icons/dist/error.js";
import "@ui5/webcomponents-icons/dist/alert.js";
import "@ui5/webcomponents-icons/dist/sys-enter-2.js";
import "@ui5/webcomponents-icons/dist/information.js";

so the select component imports the icon (as it uses it as part of its own shadow DOM) and then just imports the individual icons that it's going to need (but not the shole icon collection - the "AllIcons.js" import).

So in the end, overriding ui5-icon with an empty class will not have benefits for you, but will break some other existing components.

Does it make sense?

Regards, Vladi

vasicvuk commented 8 months ago

I fully understand how it works, what I want to achieve is to change icon set totally and use icons from font-awasome or material icons.

To achieve this I registered a custom ui5-icon element in charge for displaying the icon, and this works. But now i get this warning and error each time an element with icon appears and this is annoying.

I manage to hide the error by using:

registerIconLoader("SAP-icons-v5", () => {
  return new Promise((resolve, reject) => {
    return resolve({ collection: "SAP-icons-v5", data: {}, packageName: 'SAP-icons-v5'});
  });
});

Now i have only warning for already registering ui5-icon

vladitasev commented 8 months ago

Ironically, there used to be a method called something like "suppressFailedTagRegistrationWarnings" that you could call with true, and even the console warning itself told you what to import and call, but this was removed a long time ago with one of the rafactorings.

However, I see your point and I believe it makes sense to be able to stop these warnings, both in your scenario and in others (most notably micro-frontend scenarios). I'll bring it up to my team tomorrow morning.

BR, Vladi

Edit: In any case, even if we return this functionality, it will take maybe 2-3 weeks till the next release. If you want to work around it now, the only thing that comes to mind is monkey-patching console.warn, f.e. like this:

const original = window.console.warn;
window.console.warn = (message, ...params) => {
  if (typeof message === "string" && message.includes(`tag(s) as they were defined by a runtime`)) {
      return;
  }
  return original(message, ...params);
}
vladitasev commented 8 months ago

Hi @vasicvuk

We discussed your requirement internally, and the team is OK to provide a way to stop the console warnings. I'll get back to you with the specifics.

However, looking at the big picture, we're considering adding support to ui5-icon to display font-based icons in the future to make it more flexible and universal. Would you be interested in such a feature? Would this work for your use case?

Regards, Vladi

vasicvuk commented 8 months ago

Hi @vladitasev Thanks for fast feedback, great for the warnings :) Also, a feature for loading font based icons seems ideal for this case, then I would not have to override ui5-icon implementation which would be awasome.

nnaydenow commented 8 months ago

Hi @vasicvuk,

We've created feature request for the enhancement proposal provided by @vladitasev. You can check its status in https://github.com/SAP/ui5-webcomponents/issues/8042

Regards, Nayden