ionic-team / ionicons

Premium hand-crafted icons built by Ionic, for Ionic apps and web apps everywhere 🌎
http://ionicons.com
MIT License
17.38k stars 2.06k forks source link

bug: not clear how to use setAssetPath to load svg icons #1302

Closed mamillastre closed 6 days ago

mamillastre commented 6 months ago

Prerequisites

Ionic Framework Version

v7.x

Current Behavior

If we want to import all icons manually (or custom icons) in a "svg" folder, the IonIcon standalone component does not show the icon when using a name. We also have the following error: Failed to construct 'URL': Invalid base URL

A recent update catches this error to show a warning that invites to use the new addIcons method (https://github.com/ionic-team/ionicons/pull/1297)

But if we copy icons manually in the "svg" folder, the "addIcons" call must not be necessary.

In utils.ts from ionicons repo (https://github.com/ionic-team/ionicons/blob/main/src/components/icon/utils.ts) we have the following code:

const getNamedUrl = (iconName: string, iconEl: Icon) => {
  const url = getIconMap().get(iconName);
  if (url) {
    return url;
  }
  try {
    return getAssetPath(`svg/${iconName}.svg`);
  } catch(e) {
    /**
     * In the custom elements build version of ionicons, referencing an icon
     * by name will throw an invalid URL error because the asset path is not defined.
     * This catches that error and logs something that is more developer-friendly.
     * We also include a reference to the ion-icon element so developers can
     * figure out which instance of ion-icon needs to be updated.
     */
    console.warn(`[Ionicons Warning]: Could not load icon with name "${iconName}". Ensure that the icon is registered using addIcons or that the icon SVG data is passed directly to the icon component.`, iconEl);
  }
};

If we didn't add the icon with "addIcons", there is a fallback to get the icon from the "svg" folder. But the getAssetPath method always throws an error because the base URL is computed from an empty base.

Expected Behavior

The fallback work and if the icon exists in the "svg" folder, it's rendered.

Steps to Reproduce

  1. Create new app with ionic start (angular & standalone)
  2. Copy all the icons in the assets by default (angular.json configuration):
    {
    "glob": "**/*.svg",
    "input": "node_modules/ionicons/dist/ionicons/svg",
    "output": "./svg"
    }
  3. Add an icon with the name attribute

Code Reproduction URL

https://github.com/mamillastre/ionic-standalone-icon-error

Ionic Info

Ionic:

Ionic CLI : 7.1.5 (/usr/local/lib/node_modules/@ionic/cli) Ionic Framework : @ionic/angular 7.5.4 @angular-devkit/build-angular : 16.2.10 @angular-devkit/schematics : 16.2.10 @angular/cli : 16.2.10 @ionic/angular-toolkit : 9.0.0

Capacitor:

Capacitor CLI : 5.5.1 @capacitor/android : not installed @capacitor/core : 5.5.1 @capacitor/ios : not installed

Utility:

cordova-res : 0.15.4 native-run (update available: 2.0.0) : 1.7.4

System:

NodeJS : v20.9.0 (/usr/local/bin/node) npm : 10.1.0 OS : macOS Monterey

Additional Information

No response

muuvmuuv commented 6 months ago

My workaround is just to add them all like this and call the func in the app.component constructor. This way we only have our icons included since we only use outline this also reduced our bundle size yay.

import { addIcons } from 'ionicons'
import {
  // ...
} from 'ionicons/icons'

const ALL_ICONS = {
  // ...
}

export const addAllIcons = () => addIcons(ALL_ICONS)
liamdebeasi commented 6 months ago

I'm going to move this to the Ionicons project instead. This isn't a bug in Ionicons but rather a missing asset path in your project. Stencil does not know where to look for the SVGs which is why you get an invalid URL error. However, we don't have documentation that shows how to do this, so I think we should add that.

liamdebeasi commented 6 months ago

Try adding the following to main.ts in your project:

import { setAssetPath } from '@stencil/core';

...

setAssetPath(`${window.location.origin}/`);

This will set the asset path to something like http://localhost:8100/ or http://example.com/ (wherever your app is served from).

mamillastre commented 5 months ago

It works just fine, thank you.

Do you plan to set a default assets path in ionicons or maybe expose this configuration method from ionicons or Ionic framework ? Because I think it's strange to have to import @stencil/core when using only Ionic or ionicons.

liamdebeasi commented 5 months ago

At the moment our recommendation is to use the setAssetPath import from @stencil/core.

withinsight commented 1 week ago

I can second this; I was able to get @liamdebeasi's recommendation working in the www output, but when I attempt the same using the Angular output target, in the Angular test-app, when I pause on the line where getAssetPath is called when Angular runs, it's undefined:

Screenshot 2024-04-26 at 6 19 22 PM
christian-bromann commented 1 week ago

It is required that setAssetPath and getAssetPath come from the same Stencil runtime as the components you are using. For example, if we build the Ionicon project, every output target has its own Stencil runtime. You can import { getAssetPath } from 'ionicons' because getAssetPath is being used within the project. However since it doesn't use setAssetPath, it won't be compiled into the runtime bundle, hence the import error.

At the moment our recommendation is to use the setAssetPath import from @stencil/core.

This would only work if the project that exports components has the externalRuntime set to true. I will take this to the @ionic-team/stencil team to work on a fix for this.

christian-bromann commented 1 week ago

I raised a PR with a fix 👆

liamdebeasi commented 6 days ago

Hi everyone,

This has been resolved via https://github.com/ionic-team/ionicons/pull/1372.