fix(icon): fix types for icon src and test

.changeset/olive-news-suffer.md

@@ -0,0 +1,5 @@
+---
+'@lion/icon': patch
+---
+
+Fix lion icon types

packages/icon/src/IconManager.js

@@ -1,3 +1,8 @@
+/**
+ * @typedef {import('lit-html').TemplateResult} TemplateResult
+ * @typedef {import('lit-html').nothing} nothing
+ */
+
 export class IconManager {
   constructor() {
     this.__iconResolvers = new Map();
@@ -9,7 +14,7 @@ export class IconManager {
    * icon as a TemplateResult. This function can be sync or async.
    *
    * @param {string} namespace
-   * @param {(iconset: string, icon: string) => TemplateResult | Promise<TemplateResult>} iconResolver
+   * @param {(iconset: string, icon: string) => TemplateResult | Promise<TemplateResult> | nothing | Promise<nothing> } iconResolver
    */
   addIconResolver(namespace, iconResolver) {
     if (this.__iconResolvers.has(namespace)) {
@@ -54,6 +59,6 @@ export class IconManager {
       throw new Error(`Incorrect iconId: ${iconId}. Format: <namespace>:<iconset>:<icon>`);
     }
 
-    return this.resolveIcon(...splitIconId);
+    return this.resolveIcon(splitIconId[0], splitIconId[1], splitIconId[2]);
   }
 }

packages/icon/src/LionIcon.js

@@ -1,12 +1,18 @@
 import { css, html, LitElement, nothing, render, TemplateResult } from '@lion/core';
 import { icons } from './icons.js';
 
+/**
+ * @param {?} wrappedSvgObject
+ */
 function unwrapSvg(wrappedSvgObject) {
   const svgObject =
     wrappedSvgObject && wrappedSvgObject.default ? wrappedSvgObject.default : wrappedSvgObject;
   return typeof svgObject === 'function' ? svgObject(html) : svgObject;
 }
 
+/**
+ * @param {TemplateResult|nothing} svg
+ */
 function validateSvg(svg) {
   if (!(svg === nothing || svg instanceof TemplateResult)) {
     throw new Error(
@@ -24,15 +30,13 @@ export class LionIcon extends LitElement {
       /**
        * @desc When icons are not loaded as part of an iconset defined on iconManager,
        * it's possible to directly load an svg.
-       * @type {TemplateResult|function}
        */
       svg: {
-        type: Object,
+        attribute: false,
       },
       /**
        * @desc The iconId allows to access icons that are registered to the IconManager
        * For instance, "lion:space:alienSpaceship"
-       * @type {string}
        */
       ariaLabel: {
         type: String,
@@ -42,7 +46,6 @@ export class LionIcon extends LitElement {
       /**
        * @desc The iconId allows to access icons that are registered to the IconManager
        * For instance, "lion:space:alienSpaceship"
-       * @type {string}
        */
       iconId: {
         type: String,
@@ -93,16 +96,20 @@ export class LionIcon extends LitElement {
   constructor() {
     super();
     this.role = 'img';
+    this.ariaLabel = '';
+    this.iconId = '';
+    this.__svg = nothing;
   }
 
+  /** @param {import('lit-element').PropertyValues} changedProperties */
   update(changedProperties) {
     super.update(changedProperties);
     if (changedProperties.has('ariaLabel')) {
-      this._onLabelChanged(changedProperties);
+      this._onLabelChanged();
     }
 
     if (changedProperties.has('iconId')) {
-      this._onIconIdChanged(changedProperties.get('iconId'));
+      this._onIconIdChanged(/** @type {string} */ (changedProperties.get('iconId')));
     }
   }
 
@@ -119,6 +126,7 @@ export class LionIcon extends LitElement {
   /**
    * On IE11, svgs without focusable false appear in the tab order
    * so make sure to have <svg focusable="false"> in svg files
+   * @param {TemplateResult|nothing} svg
    */
   set svg(svg) {
     this.__svg = svg;
@@ -142,6 +150,9 @@ export class LionIcon extends LitElement {
     }
   }
 
+  /**
+   * @param {TemplateResult | nothing} svgObject
+   */
   _renderSvg(svgObject) {
     validateSvg(svgObject);
     render(svgObject, this);
@@ -150,11 +161,14 @@ export class LionIcon extends LitElement {
     }
   }
 
+  /**
+   * @param {string} prevIconId
+   */
   async _onIconIdChanged(prevIconId) {
     if (!this.iconId) {
       // clear if switching from iconId to no iconId
       if (prevIconId) {
-        this.svg = null;
+        this.svg = nothing;
       }
     } else {
       const iconIdBeforeResolve = this.iconId;

packages/icon/src/icons.js

@@ -4,6 +4,7 @@ import { IconManager } from './IconManager.js';
 // eslint-disable-next-line import/no-mutable-exports
 export let icons = singletonManager.get('@lion/icon::icons::0.5.x') || new IconManager();
 
+// @ts-ignore since we don't know which singleton icon manager version will be used, we cannot type it.
 export function setIcons(newIcons) {
   icons = newIcons;
 }

packages/icon/test/IconManager.test.js

@@ -1,7 +1,12 @@
+import { nothing } from '@lion/core';
 import { expect } from '@open-wc/testing';
 import { stub } from 'sinon';
 import { IconManager } from '../src/IconManager.js';
 
+/**
+ * @typedef {import("lit-html").TemplateResult} TemplateResult
+ */
+
 describe('IconManager', () => {
   it('starts off with an empty map of resolvers', () => {
     const manager = new IconManager();
@@ -10,7 +15,15 @@ describe('IconManager', () => {
 
   it('allows adding an icon resolver', () => {
     const manager = new IconManager();
-    const resolver = () => {};
+    /**
+     * @param {string} iconset
+     * @param {string} icon
+     * @return {TemplateResult | Promise<TemplateResult> | nothing | Promise<nothing>}
+     */
+    // eslint-disable-next-line no-unused-vars
+    const resolver = (iconset, icon) => {
+      return nothing;
+    };
     manager.addIconResolver('foo', resolver);
 
     expect(manager.__iconResolvers.get('foo')).to.equal(resolver);

packages/icon/test/hammer.svg.js

@@ -1,2 +1,2 @@
-export default tag =>
+export default /** @param {(strings: TemplateStringsArray, ... expr: string[]) => string} tag */ tag =>
   tag`<svg focusable="false" viewBox="0 0 16 16" xmlns="http://www.w3.org/2000/svg" data-test-id="svg-hammer"><path d="M.476 12.915L6.7 6 9 8.556l-6.223 6.915a1.542 1.542 0 0 1-1.15.529 1.54 1.54 0 0 1-1.15-.53c-.636-.706-.636-1.85 0-2.555zm12.638-9.031L16 6.863 12.866 10 4 .919 9.35 0l1.912 1.972.251-.251c.52-.52 2.4 1.363 1.88 1.882l-.279.28z"></path></svg>`;

packages/icon/test/heart.svg.js

@@ -1,2 +1,2 @@
-export default tag =>
+export default /** @param {(strings: TemplateStringsArray, ... expr: string[]) => string} tag */ tag =>
   tag`<svg focusable="false" viewBox="0 0 16 16" xmlns="http://www.w3.org/2000/svg" data-test-id="svg-heart"><path d="M8.002 14.867a.828.828 0 0 1-.324-.066C7.36 14.665 0 11.466 0 6.153 0 3.592 1.543.825 4.98 1.01c1.465.077 2.437.828 3.018 1.491.581-.667 1.553-1.414 3.023-1.491.107-.008.207-.008.31-.008C13.58 1.001 16 2.614 16 6.153c0 5.313-7.36 8.512-7.671 8.644a.787.787 0 0 1-.327.07z"></path></svg>`;

packages/icon/test/lion-icon.test.js

@@ -1,19 +1,28 @@
-import { until } from '@lion/core';
+import { nothing, until } from '@lion/core';
-import { aTimeout, expect, fixture, fixtureSync, html } from '@open-wc/testing';
+import { aTimeout, expect, fixture as _fixture, fixtureSync, html } from '@open-wc/testing';
 import '../lion-icon.js';
 import { icons } from '../src/icons.js';
 import hammerSvg from './hammer.svg.js';
 import heartSvg from './heart.svg.js';
 
+/**
+ * @typedef {(strings: TemplateStringsArray, ... expr: string[]) => string} TaggedTemplateLiteral
+ * @typedef {import('../src/LionIcon').LionIcon} LionIcon
+ * @typedef {import('lit-html').TemplateResult} TemplateResult
+ */
+const fixture = /** @type {(arg: TemplateResult|string) => Promise<LionIcon>} */ (_fixture);
+
 describe('lion-icon', () => {
   it('supports svg icon as a function which recieves a tag function as an argument and returns a tagged template literal', async () => {
-    const iconFunction = tag => tag`<svg data-test-id="svg"></svg>`;
+    const iconFunction = /** @param {TaggedTemplateLiteral} tag */ tag =>
+      tag`<svg data-test-id="svg"></svg>`;
     const el = await fixture(html`<lion-icon .svg=${iconFunction}></lion-icon>`);
     expect(el.children[0].getAttribute('data-test-id')).to.equal('svg');
   });
 
   it('is hidden when attribute hidden is true', async () => {
-    const iconFunction = tag => tag`<svg data-test-id="svg"></svg>`;
+    const iconFunction = /** @param {TaggedTemplateLiteral} tag */ tag =>
+      tag`<svg data-test-id="svg"></svg>`;
     const el = await fixture(html`<lion-icon .svg=${iconFunction} hidden></lion-icon>`);
     expect(el).not.to.be.displayed;
   });
@@ -126,7 +135,7 @@ describe('lion-icon', () => {
     await svgLoading;
     // We need to await the until directive is resolved and rendered to the dom
     // You can not use updateComplete as until renders on it's own
-    await aTimeout();
+    await aTimeout(0);
 
     expect(el.children[0].getAttribute('data-test-id')).to.equal('svg-heart');
   });
@@ -134,7 +143,7 @@ describe('lion-icon', () => {
   it('does not render "undefined" if changed from valid input to undefined', async () => {
     const el = await fixture(html`<lion-icon .svg=${heartSvg}></lion-icon>`);
     await el.updateComplete;
-    el.svg = undefined;
+    el.svg = nothing;
     await el.updateComplete;
     expect(el.innerHTML).to.equal('<!----><!---->'); // don't use lightDom.to.equal(''), it gives false positives
   });
@@ -142,7 +151,7 @@ describe('lion-icon', () => {
   it('does not render "null" if changed from valid input to null', async () => {
     const el = await fixture(html`<lion-icon .svg=${heartSvg}></lion-icon>`);
     await el.updateComplete;
-    el.svg = null;
+    el.svg = nothing;
     await el.updateComplete;
     expect(el.innerHTML).to.equal('<!----><!---->'); // don't use lightDom.to.equal(''), it gives false positives
   });
@@ -152,7 +161,7 @@ describe('lion-icon', () => {
       icons.addIconResolver('foo', () => heartSvg);
       const el = await fixture(html`<lion-icon icon-id="foo:lorem:ipsum"></lion-icon>`);
 
-      expect(el.children[0].dataset.testId).to.equal('svg-heart');
+      expect(/** @type {HTMLElement} */ (el.children[0]).dataset.testId).to.equal('svg-heart');
     } finally {
       icons.removeIconResolver('foo');
     }
@@ -189,10 +198,10 @@ describe('lion-icon', () => {
       await aTimeout(4);
 
       // heart is still loading at this point, but hammer came later so that should be later
-      expect(el.children[0].dataset.testId).to.equal('svg-hammer');
+      expect(/** @type {HTMLElement} */ (el.children[0]).dataset.testId).to.equal('svg-hammer');
       await aTimeout(10);
       // heart finished loading, but it should not be rendered because hammer came later
-      expect(el.children[0].dataset.testId).to.equal('svg-hammer');
+      expect(/** @type {HTMLElement} */ (el.children[0]).dataset.testId).to.equal('svg-hammer');
     } finally {
       icons.removeIconResolver('foo');
       icons.removeIconResolver('bar');

tsconfig.json

@@ -25,7 +25,6 @@
     "packages/babel-plugin-extend-docs/**/*.js",
     "packages/providence-analytics/**/*.js",
     "packages/remark-extend/**/*.js",
-    "packages/icon/**/*.js", // FIXME: Needs to get typed!
     "packages/select-rich/test/**/*.js", // TODO: Needs to get typed!
     "packages/overlays/test/utils-tests/**/*.js", // TODO: Needs to get typed!
     "packages/helpers/**/*.js", // FIXME: Needs to get typed!