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

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

Closed
3 tasks done
mamillastre opened this issue Nov 9, 2023 · 9 comments · Fixed by #1372
Closed
3 tasks done

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

mamillastre opened this issue Nov 9, 2023 · 9 comments · Fixed by #1372

Comments

@mamillastre
Copy link

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 (#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"
}
  1. 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
Copy link

muuvmuuv commented Nov 9, 2023

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 liamdebeasi self-assigned this Nov 9, 2023
@liamdebeasi
Copy link
Contributor

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 liamdebeasi transferred this issue from ionic-team/ionic-framework Nov 9, 2023
@liamdebeasi liamdebeasi changed the title bug: Icon error when using the angular standalone component even if the icon exists in source bug: not clear how to use setAssetPath to load svg icons Nov 9, 2023
@liamdebeasi liamdebeasi removed their assignment Nov 9, 2023
@liamdebeasi
Copy link
Contributor

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
Copy link
Author

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
Copy link
Contributor

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

@withinsight
Copy link

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
Copy link
Member

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
Copy link
Member

I raised a PR with a fix 👆

liamdebeasi pushed a commit that referenced this issue May 1, 2024
@liamdebeasi
Copy link
Contributor

Hi everyone,

This has been resolved via #1372.

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

Successfully merging a pull request may close this issue.

5 participants