perf(icon): skip the registry lookup for names nothing can resolve (#DS-5798)

packages/components/icon/icon-registry.spec.ts

@@ -5,6 +5,7 @@ import { DomSanitizer, SafeHtml } from '@angular/platform-browser';
 import { firstValueFrom } from 'rxjs';
 import type { MockInstance } from 'vitest';
 import { KbqIconRegistry } from './icon-registry';
+import { kbqIconsDictProvider, kbqIconsProvider } from './icon-registry-providers';
 
 const ICON_SVG = '<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 16 16"><path d="M0 0h16v16H0z"/></svg>';
 const SPRITE_SVG = `<svg xmlns="http://www.w3.org/2000/svg">
@@ -169,6 +170,31 @@ describe('KbqIconRegistry', () => {
         });
     });
 
+    describe('findSvgIcon', () => {
+        it('is null on an empty registry', () => {
+            expect(registry.findSvgIcon('check')).toBeNull();
+        });
+
+        it('finds only the names registered', () => {
+            registry.addSvgIconLiteral('check', sanitizer.bypassSecurityTrustHtml(ICON_SVG));
+            registry.addSvgIconLiteralInNamespace('brand', 'logo', sanitizer.bypassSecurityTrustHtml(ICON_SVG));
+
+            expect(registry.findSvgIcon('check')).not.toBeNull();
+            expect(registry.findSvgIcon('brand:logo')).not.toBeNull();
+            expect(registry.findSvgIcon('logo', 'brand')).not.toBeNull();
+            expect(registry.findSvgIcon('close')).toBeNull();
+            expect(registry.findSvgIcon('logo')).toBeNull();
+        });
+
+        it('leaves any name of a namespace with an icon set to the set', () => {
+            registry.addSvgIconSetInNamespace('sprite', sanitizer.bypassSecurityTrustResourceUrl('/sprite.svg'));
+
+            expect(registry.findSvgIcon('sprite:anything')).not.toBeNull();
+            expect(registry.findSvgIcon('anything', 'sprite')).not.toBeNull();
+            expect(registry.findSvgIcon('anything')).toBeNull();
+        });
+    });
+
     describe('getNamedSvgIcon errors', () => {
         it('errors when no icon registered', async () => {
             await expect(firstValueFrom(registry.getNamedSvgIcon('missing'))).rejects.toThrow('missing');
@@ -264,3 +290,20 @@ describe('KbqIconRegistry', () => {
         });
     });
 });
+
+describe('KbqIconRegistry configured through providers', () => {
+    it('finds only the names a resolver answers for', () => {
+        TestBed.configureTestingModule({ providers: [kbqIconsDictProvider({ check: '/check.svg' })] });
+
+        const registry = TestBed.inject(KbqIconRegistry);
+
+        expect(registry.findSvgIcon('check')).not.toBeNull();
+        expect(registry.findSvgIcon('close')).toBeNull();
+    });
+
+    it('leaves any bare name to a sprite configured without a namespace', () => {
+        TestBed.configureTestingModule({ providers: [kbqIconsProvider({ spriteUrl: '/sprite.svg' })] });
+
+        expect(TestBed.inject(KbqIconRegistry).findSvgIcon('anything')).not.toBeNull();
+    });
+});

packages/components/icon/icon-registry.ts

@@ -23,6 +23,21 @@ function iconKey(namespace: string, name: string): string {
     return namespace ? `${namespace}:${name}` : name;
 }
 
+/**
+ * Splits "namespace:name" at the first colon; a name without one stays in the given namespace.
+ *
+ * @internal
+ */
+export function parseIconName(name: string, namespace = ''): [namespace: string, name: string] {
+    const colonIndex = name.indexOf(':');
+
+    return colonIndex === -1 ? [namespace, name] : [name.slice(0, colonIndex), name.slice(colonIndex + 1)];
+}
+
+function isSvgMarkup(text: string): boolean {
+    return text.trimStart().startsWith('<');
+}
+
 function cloneSvg(svg: SVGElement): SVGElement {
     return svg.cloneNode(true) as SVGElement;
 }
@@ -108,24 +123,29 @@ export class KbqIconRegistry {
      * Accepts "namespace:name" syntax.
      */
     getNamedSvgIcon(name: string, namespace = ''): Observable<SVGElement> {
-        let resolvedNs = namespace;
-        let resolvedName = name;
+        const icon = this.findSvgIcon(name, namespace);
 
-        if (resolvedName.trimStart().startsWith('<')) {
-            // `defer` so that unparsable markup arrives as an error notification: every failure of this
-            // class has to reach the caller through the same channel.
-            return defer(() =>
-                of(this.svgElementFromText(this.sanitizer.sanitize(SecurityContext.HTML, resolvedName) ?? ''))
-            );
-        }
+        if (icon) return icon;
 
-        const colonIndex = name.indexOf(':');
+        const key = iconKey(...parseIconName(name, namespace));
 
-        if (colonIndex !== -1) {
-            resolvedNs = name.slice(0, colonIndex);
-            resolvedName = name.slice(colonIndex + 1);
+        return throwError(() => Error(`KbqIconRegistry: no icon registered for "${key}".`));
+    }
+
+    /**
+     * `getNamedSvgIcon` without the error for a miss: `null` when nothing registered can resolve the name. Every font
+     * icon is looked up, and an error per miss would cost a stack trace each.
+     *
+     * @internal
+     */
+    findSvgIcon(name: string, namespace = ''): Observable<SVGElement> | null {
+        if (isSvgMarkup(name)) {
+            // `defer` so that unparsable markup arrives as an error notification: every failure of this
+            // class has to reach the caller through the same channel.
+            return defer(() => of(this.svgElementFromText(this.sanitizer.sanitize(SecurityContext.HTML, name) ?? '')));
         }
 
+        const [resolvedNs, resolvedName] = parseIconName(name, namespace);
         const key = iconKey(resolvedNs, resolvedName);
 
         const iconConfig = this.iconConfigs.get(key);
@@ -146,7 +166,7 @@ export class KbqIconRegistry {
                 options: undefined
             };
 
-            if (result.trimStart().startsWith('<')) {
+            if (isSvgMarkup(result)) {
                 const sanitized = this.sanitizer.sanitize(SecurityContext.HTML, result) ?? '';
 
                 config.svgText = this.sanitizer.bypassSecurityTrustHtml(sanitized);
@@ -165,7 +185,7 @@ export class KbqIconRegistry {
             return this.loadIconFromSets(resolvedName, sets);
         }
 
-        return throwError(() => Error(`KbqIconRegistry: no icon registered for "${key}".`));
+        return null;
     }
 
     private cacheIcon(namespace: string, name: string, config: KbqSvgIconConfig): this {

packages/components/icon/icon.component.spec.ts

@@ -1,9 +1,10 @@
 import { provideHttpClient } from '@angular/common/http';
 import { HttpTestingController, provideHttpClientTesting } from '@angular/common/http/testing';
-import { ChangeDetectionStrategy, Component, Signal, signal, SimpleChange } from '@angular/core';
+import { ChangeDetectionStrategy, Component, Injectable, Signal, signal, SimpleChange } from '@angular/core';
 import { ComponentFixture, TestBed } from '@angular/core/testing';
 import { By, DomSanitizer } from '@angular/platform-browser';
 import { KbqComponentColors } from '@koobiq/components/core';
+import { Observable, of } from 'rxjs';
 import { KbqIconButton } from './icon-button.component';
 import { KBQ_ICON_ERROR_STATE_CONTEXT } from './icon-error-state-context';
 import { KbqIconRegistry } from './icon-registry';
@@ -14,6 +15,10 @@ const svgOf = (path: string) => `<svg xmlns="http://www.w3.org/2000/svg" viewBox
 
 const ICON_SVG = svgOf('M0 0h16v16H0z');
 
+const SPRITE_SVG = `<svg xmlns="http://www.w3.org/2000/svg">
+  <symbol id="check_16" viewBox="0 0 16 16"><path d="M1 8l4 4 8-8"/></symbol>
+</svg>`;
+
 @Component({
     imports: [KbqIcon],
     template: `
@@ -295,6 +300,48 @@ describe('KbqIcon', () => {
             expect(hostOf(fixture).classList).toContain('kbq-plus_16');
         });
 
+        // A miss through `getNamedSvgIcon` builds an error, with its stack trace, for every font icon on the page.
+        it('should not build an error for a font icon the registry cannot resolve', () => {
+            register('kbq-minus_16');
+
+            // On the prototype: an own property would read as a replaced lookup and be asked instead.
+            const lookup = vi.spyOn(KbqIconRegistry.prototype, 'getNamedSvgIcon');
+
+            onTestFinished(() => lookup.mockRestore());
+
+            const fixture = TestBed.createComponent(TestApp);
+
+            fixture.detectChanges();
+
+            expect(lookup).not.toHaveBeenCalled();
+            expect(hostOf(fixture).classList).toContain('kbq-plus_16');
+        });
+
+        it('should still look up a name of a namespace with an icon set', async () => {
+            registry.addSvgIconSetInNamespace('sprite', sanitizer.bypassSecurityTrustResourceUrl('/sprite.svg'));
+
+            const fixture = TestBed.createComponent(TestApp);
+
+            fixture.componentInstance.name.set('sprite:check_16');
+            fixture.detectChanges();
+
+            TestBed.inject(HttpTestingController).expectOne('/sprite.svg').flush(SPRITE_SVG);
+            await fixture.whenStable();
+
+            expect(hostOf(fixture).querySelector('svg')?.getAttribute('width')).toBe('16');
+        });
+
+        it('should size an svg whose name has a colon after the namespace', () => {
+            registry.addSvgIconLiteralInNamespace('brand', 'logo:dark_24', sanitizer.bypassSecurityTrustHtml(ICON_SVG));
+
+            const fixture = TestBed.createComponent(TestApp);
+
+            fixture.componentInstance.name.set('brand:logo:dark_24');
+            fixture.detectChanges();
+
+            expect(hostOf(fixture).querySelector('svg')?.getAttribute('width')).toBe('24');
+        });
+
         it('should render an SVG that arrives after the first render', async () => {
             registry.addSvgIcon('kbq-plus_16', sanitizer.bypassSecurityTrustResourceUrl('/icons/plus.svg'));
 
@@ -499,6 +546,47 @@ describe('KbqIcon', () => {
         expect(hostOf(fixture).querySelector('svg')).not.toBeNull();
     });
 
+    // The registry is public: an application may replace its lookup, and its specs may provide a double of it.
+    describe('replaced registry lookup', () => {
+        const svgElement = (): SVGElement => {
+            const svg = document.createElementNS('http://www.w3.org/2000/svg', 'svg');
+
+            svg.setAttribute('viewBox', '0 0 16 16');
+
+            return svg;
+        };
+
+        it('should ask a registry subclass that overrides getNamedSvgIcon', () => {
+            @Injectable()
+            class AppIconRegistry extends KbqIconRegistry {
+                override getNamedSvgIcon(name: string): Observable<SVGElement> {
+                    return name === 'kbq-plus_16' ? of(svgElement()) : super.getNamedSvgIcon(name);
+                }
+            }
+
+            TestBed.configureTestingModule({ providers: [{ provide: KbqIconRegistry, useClass: AppIconRegistry }] });
+
+            const fixture = TestBed.createComponent(TestApp);
+
+            fixture.detectChanges();
+
+            expect(hostOf(fixture).querySelector('svg')).not.toBeNull();
+            expect(hostOf(fixture).classList).not.toContain('kbq-plus_16');
+        });
+
+        it('should ask a test double that only has getNamedSvgIcon', () => {
+            TestBed.configureTestingModule({
+                providers: [{ provide: KbqIconRegistry, useValue: { getNamedSvgIcon: () => of(svgElement()) } }]
+            });
+
+            const fixture = TestBed.createComponent(TestApp);
+
+            fixture.detectChanges();
+
+            expect(hostOf(fixture).querySelector('svg')).not.toBeNull();
+        });
+    });
+
     // The color class of an error icon is `kbq-error` too; the `kbq-error` class binding must not take it away.
     describe('error color', () => {
         const iconOf = (fixture: ComponentFixture<ColoredIcon>): KbqIcon =>

packages/components/icon/icon.component.ts

@@ -15,9 +15,9 @@ import {
     ViewEncapsulation
 } from '@angular/core';
 import { KbqColorDirective, KbqComponentColors } from '@koobiq/components/core';
-import { defer, Subscription } from 'rxjs';
+import { defer, of, Subscription } from 'rxjs';
 import { KBQ_ICON_ERROR_STATE_CONTEXT } from './icon-error-state-context';
-import { KbqIconRegistry } from './icon-registry';
+import { KbqIconRegistry, parseIconName } from './icon-registry';
 
 /** Marks an injected `<svg>` with the name it renders, so hydration can recognize and adopt it. */
 const ICON_NAME_ATTRIBUTE = 'data-kbq-icon-name';
@@ -248,10 +248,15 @@ export class KbqIcon extends KbqColorDirective implements AfterContentInit, OnCh
         }
 
         const registry = this.registry;
+        // A registry that replaces the lookup, such as a subclass overriding `getNamedSvgIcon` or a test double, is
+        // asked through it: only the library's own lookup can report a miss without building an error.
+        const ownLookup = registry.getNamedSvgIcon === KbqIconRegistry.prototype.getNamedSvgIcon;
 
         // Deferred so that a resolver or a literal that throws falls back to the font class as well, and the
         // next name is resolved again.
-        this.svgIconSubscription = defer(() => registry.getNamedSvgIcon(name)).subscribe({
+        this.svgIconSubscription = defer(
+            () => (ownLookup ? registry.findSvgIcon(name) : registry.getNamedSvgIcon(name)) ?? of(null)
+        ).subscribe({
             next: (svg) => this.renderSvgIcon(svg),
             error: () => this.renderSvgIcon(null)
         });
@@ -309,8 +314,7 @@ export class KbqIcon extends KbqColorDirective implements AfterContentInit, OnCh
     }
 
     private parseIconSize(): number | undefined {
-        const iconName = this.iconName;
-        const baseName = iconName?.includes(':') ? iconName.split(':')[1] : iconName;
+        const baseName = this.iconName && parseIconName(this.iconName)[1];
         const size = parseInt(baseName?.split('_').pop() ?? '');
 
         return Number.isNaN(size) ? undefined : size;