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

Default string import for icons #5122

Closed
C3ntraX opened this issue Apr 26, 2022 · 3 comments · Fixed by #5207
Closed

Default string import for icons #5122

C3ntraX opened this issue Apr 26, 2022 · 3 comments · Fixed by #5207

Comments

@C3ntraX
Copy link

C3ntraX commented Apr 26, 2022

Is your feature request related to a problem? Please describe.
I don't want to import the big AllIcons.js module to reduce the package size. If I need an icon instead, I import it separately at each place I need the icon. However, the problem is that it is very difficult to determine whether imported icons are still used in the module. Or whether icons are even completely missing, which is why, in the best case, a warning appears in the console.

Describe the solution you'd like
Icons should have a default export. The export should be the represented icon-string. Let me demonstrate:

Current situation:
import "@ui5/webcomponents-icons/dist/accept";
<Icon name="accept" />

Problems:
The IDE cann´t track the usage of the import if the icons changes. This causes following problems

  1. The import can be obsolete, which causes additional bundle size
  2. The import can be missing, which causes in best case a warning the the console

Solution:
import accept from "@ui5/webcomponents-icons/dist/accept";
<Icon name={accept} />

or/and:

import { accept, decline } from "@ui5/webcomponents-icons";
<Icon name={accept} />
<Icon name={decline} />

MUI handels this the same way: https://mui.com/material-ui/icons/#main-content
They mentioned the bundle size as well, it should always be possible, to reduce the bundle size.

Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.

Additional context
Add any other context or screenshots about the feature request here.

@MarcusNotheis
Copy link
Collaborator

Thanks for reporting! I'll forward this issue to our UI5 Web Components Colleagues as the affected component is developed in their repository.

@MarcusNotheis MarcusNotheis transferred this issue from SAP/ui5-webcomponents-react Apr 26, 2022
@unazko unazko added TOPIC Core enhancement New feature or request feature request and removed enhancement New feature or request labels Apr 27, 2022
@unazko
Copy link
Contributor

unazko commented Apr 27, 2022

Hello @SAP/ui5-webcomponents-team,

As this request is related to module import enhancements I've dispatched it to your component.

Best Regards,
Boyan Rakilovski

@ilhan007 ilhan007 self-assigned this May 12, 2022
@ilhan007 ilhan007 added this to the 1.4.0 milestone May 12, 2022
ilhan007 added a commit that referenced this issue May 13, 2022
We now use the icon name string as default export. This will allow developers to directly set the imported icon name string
to the Icon component (as shown below) improving the DEV experience:
easier to determine (within the IDE) if imported icons are still used in the module
immediate feedback (within the IDE) when import is missing for an icon that is actually used

FIXES: #5122
@C3ntraX
Copy link
Author

C3ntraX commented Jun 2, 2022

Hi @ilhan007,

I still cant use the named imports in 1.4 because the type definitions for typescript are missing. Its only working in JS.

Can you pls fix this issue, or this change is useless for typescript projects.

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Completed
Development

Successfully merging a pull request may close this issue.

4 participants