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

Icons not showing in production if @vaadin/icons isn't imported #218

Closed
tarekoraby opened this issue Jan 15, 2024 · 2 comments · Fixed by vaadin/docs#3175
Closed

Icons not showing in production if @vaadin/icons isn't imported #218

tarekoraby opened this issue Jan 15, 2024 · 2 comments · Fixed by vaadin/docs#3175
Assignees
Labels
documentation Improvements or additions to documentation DX Impact: Low

Comments

@tarekoraby
Copy link

Related to vaadin/hilla#1957.

If a react component Icon is used without importing @vaadin/icons, the icon is displayed in development mode, but not when a production bundle is created.

To reproduce:

  1. Create a new app: npx @hilla/cli init my-hilla-app
  2. Add an Icon element to the hello view, only importing @hilla/react-components/Icon.js (without @vaadin/icons)
  3. Run the project in dev mode mvn (observe that the icon is showing)
  4. Create a production bundle using mvn clean package -Pproduction, and run it using java -jar (observe the icon isn't there).

Desired solution

I can think of two fixes/improvements:

  • Ideally, only importing @hilla/react-components/Icon.js should be sufficient for rendering icons in dev and prod.
  • If the first option isn't (easily) possible, the dev mode should throw an error + NOT show the icon if @vaadin/icons isn't imported.
@web-padawan
Copy link
Member

In dev mode we use bundles which include @vaadin/icons import.

Ideally, only importing @hilla/react-components/Icon.js should be sufficient for rendering icons in dev and prod.

I'm not sure if that's a good idea as some users might want to use Icon component with a custom iconset.
Forcing the Icon component to import @vaadin/icons would mean extra 300 kB of minified JS for these users.

@yuriy-fix
Copy link

yuriy-fix commented Jan 29, 2024

We would probably need to document this behaviour in Icon docs as we cannot exclude icons from the bundle, nor we want to have an explicit check for development mode within icons themself as it's not a part of the component.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation DX Impact: Low
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants