feat: add types for accessing icons. (#7132)

* feat: add types for accessing icons.

* chore: PR comments

core/block.ts

@@ -49,6 +49,7 @@ import type {Workspace} from './workspace.js';
 import {DummyInput} from './inputs/dummy_input.js';
 import {ValueInput} from './inputs/value_input.js';
 import {StatementInput} from './inputs/statement_input.js';
+import {IconType} from './icons/icon_types.js';
 
 /**
  * Class for one block.
@@ -2227,10 +2228,10 @@ export class Block implements IASTNodeLocation, IDeletable {
    * @param type The type of the icon to remove from the block.
    * @returns True if an icon with the given type was found, false otherwise.
    */
-  removeIcon(type: string): boolean {
+  removeIcon(type: IconType<IIcon>): boolean {
     if (!this.hasIcon(type)) return false;
     this.getIcon(type)?.dispose();
-    this.icons = this.icons.filter((icon) => icon.getType() !== type);
+    this.icons = this.icons.filter((icon) => !icon.getType().equals(type));
     return true;
   }
 
@@ -2238,17 +2239,22 @@ export class Block implements IASTNodeLocation, IDeletable {
    * @returns True if an icon with the given type exists on the block,
    *     false otherwise.
    */
-  hasIcon(type: string): boolean {
-    return this.icons.some((icon) => icon.getType() === type);
+  hasIcon(type: IconType<IIcon>): boolean {
+    return this.icons.some((icon) => icon.getType().equals(type));
   }
 
-  // TODO (#7126): Make this take in a generic type.
   /**
+   * @param type The type of the icon to retrieve. Prefer passing an `IconType`
+   *     for proper type checking when using typescript.
    * @returns The icon with the given type if it exists on the block, undefined
    *     otherwise.
    */
-  getIcon(type: string): IIcon | undefined {
-    return this.icons.find((icon) => icon.getType() === type);
+  getIcon<T extends IIcon>(type: IconType<T> | string): T | undefined {
+    if (type instanceof IconType) {
+      return this.icons.find((icon) => icon.getType().equals(type)) as T;
+    } else {
+      return this.icons.find((icon) => icon.getType().toString() === type) as T;
+    }
   }
 
   /** @returns An array of the icons attached to this block. */

core/block_svg.ts

@@ -61,6 +61,7 @@ import type {Workspace} from './workspace.js';
 import type {WorkspaceSvg} from './workspace_svg.js';
 import {queueRender} from './render_management.js';
 import * as deprecation from './utils/deprecation.js';
+import {IconType} from './icons/icon_types.js';
 
 /**
  * Class for a block's SVG representation.
@@ -952,7 +953,6 @@ export class BlockSvg
       text = null;
     }
 
-    // TODO: Make getIcon take in a type parameter?
     const icon = this.getIcon(WarningIcon.TYPE) as WarningIcon | undefined;
     if (typeof text === 'string') {
       // Bubble up to add a warning on top-most collapsed block.
@@ -1028,11 +1028,11 @@ export class BlockSvg
     };
   }
 
-  override removeIcon(type: string): boolean {
+  override removeIcon(type: IconType<IIcon>): boolean {
     const removed = super.removeIcon(type);
 
-    if (type === WarningIcon.TYPE) this.warning = null;
-    if (type === MutatorIcon.TYPE) this.mutator = null;
+    if (type.equals(WarningIcon.TYPE)) this.warning = null;
+    if (type.equals(MutatorIcon.TYPE)) this.mutator = null;
 
     if (this.rendered) {
       // TODO: Change this based on #7068.

core/events/events_block_change.ts

@@ -14,7 +14,7 @@ goog.declareModuleId('Blockly.Events.BlockChange');
 
 import type {Block} from '../block.js';
 import type {BlockSvg} from '../block_svg.js';
-import {MUTATOR_TYPE} from '../icons/icon_types.js';
+import {IconType} from '../icons/icon_types.js';
 import {hasBubble} from '../interfaces/i_has_bubble.js';
 import * as registry from '../registry.js';
 import * as utilsXml from '../utils/xml.js';
@@ -146,7 +146,7 @@ export class BlockChange extends BlockBase {
       );
     }
     // Assume the block is rendered so that then we can check.
-    const icon = block.getIcon(MUTATOR_TYPE);
+    const icon = block.getIcon(IconType.MUTATOR);
     if (icon && hasBubble(icon) && icon.bubbleIsVisible()) {
       // Close the mutator (if open) since we don't want to update it.
       icon.setBubbleVisible(false);

core/icons.ts

@@ -8,5 +8,6 @@ import {CommentIcon} from './icons/comment_icon.js';
 import * as exceptions from './icons/exceptions.js';
 import * as registry from './icons/registry.js';
 import {MutatorIcon} from './icons/mutator_icon.js';
+import {IconType} from './icons/icon_types.js';
 
-export {CommentIcon, exceptions, registry, MutatorIcon};
+export {CommentIcon, exceptions, registry, MutatorIcon, IconType};

core/icons/comment_icon.ts

@@ -9,7 +9,7 @@ goog.declareModuleId('Blockly.Comment');
 
 import type {Block} from '../block.js';
 import type {BlockSvg} from '../block_svg.js';
-import {COMMENT_TYPE} from './icon_types.js';
+import {IconType} from './icon_types.js';
 import {Coordinate} from '../utils.js';
 import * as dom from '../utils/dom.js';
 import * as eventUtils from '../events/utils.js';
@@ -35,7 +35,7 @@ const DEFAULT_BUBBLE_HEIGHT = 80;
 
 export class CommentIcon extends Icon implements IHasBubble, ISerializable {
   /** The type string used to identify this icon. */
-  static readonly TYPE = COMMENT_TYPE;
+  static readonly TYPE = IconType.COMMENT;
 
   /**
    * The weight this icon has relative to other icons. Icons with more positive
@@ -68,7 +68,7 @@ export class CommentIcon extends Icon implements IHasBubble, ISerializable {
     super(sourceBlock);
   }
 
-  override getType(): string {
+  override getType(): IconType<CommentIcon> {
     return CommentIcon.TYPE;
   }
 

core/icons/icon.ts

@@ -13,6 +13,7 @@ import {Coordinate} from '../utils/coordinate.js';
 import * as dom from '../utils/dom.js';
 import {Size} from '../utils/size.js';
 import {Svg} from '../utils/svg.js';
+import type {IconType} from './icon_types.js';
 
 export abstract class Icon implements IIcon {
   /**
@@ -29,7 +30,7 @@ export abstract class Icon implements IIcon {
 
   constructor(protected sourceBlock: Block) {}
 
-  getType(): string {
+  getType(): IconType<IIcon> {
     throw new Error('Icons must implement getType');
   }
 

core/icons/icon_types.ts

@@ -4,23 +4,26 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-/**
- * The type for a mutator icon. Used for registration and access.
- *
- * @internal
- */
-export const MUTATOR_TYPE = 'mutator';
+import {IIcon} from '../interfaces/i_icon.js';
+import {CommentIcon} from './comment_icon.js';
+import {MutatorIcon} from './mutator_icon.js';
+import {WarningIcon} from './warning_icon.js';
 
-/**
- * The type for a warning icon. Used for registration and access.
- *
- * @internal
- */
-export const WARNING_TYPE = 'warning';
+export class IconType<_T extends IIcon> {
+  /** @param name The name of the registry type. */
+  constructor(private readonly name: string) {}
 
-/**
- * The type for a comment icon. Used for registration and access.
- *
- * @internal
- */
-export const COMMENT_TYPE = 'comment';
+  /** @returns the name of the type. */
+  toString(): string {
+    return this.name;
+  }
+
+  /** @returns true if this icon type is equivalent to the given icon type. */
+  equals(type: IconType<IIcon>): boolean {
+    return this.name === type.toString();
+  }
+
+  static MUTATOR = new IconType<MutatorIcon>('mutator');
+  static WARNING = new IconType<WarningIcon>('warning');
+  static COMMENT = new IconType<CommentIcon>('comment');
+}

core/icons/mutator_icon.ts

@@ -19,12 +19,12 @@ import * as eventUtils from '../events/utils.js';
 import type {IHasBubble} from '../interfaces/i_has_bubble.js';
 import {Icon} from './icon.js';
 import {MiniWorkspaceBubble} from '../bubbles/mini_workspace_bubble.js';
-import {MUTATOR_TYPE} from './icon_types.js';
 import {Rect} from '../utils/rect.js';
 import {Size} from '../utils/size.js';
 import {Svg} from '../utils/svg.js';
 import type {WorkspaceSvg} from '../workspace_svg.js';
 import * as deprecation from '../utils/deprecation.js';
+import {IconType} from './icon_types.js';
 
 /** The size of the mutator icon in workspace-scale units. */
 const SIZE = 17;
@@ -37,7 +37,7 @@ const WORKSPACE_MARGIN = 16;
 
 export class MutatorIcon extends Icon implements IHasBubble {
   /** The type string used to identify this icon. */
-  static readonly TYPE = MUTATOR_TYPE;
+  static readonly TYPE = IconType.MUTATOR;
 
   /**
    * The weight this icon has relative to other icons. Icons with more positive
@@ -61,7 +61,7 @@ export class MutatorIcon extends Icon implements IHasBubble {
     super(sourceBlock);
   }
 
-  override getType() {
+  override getType(): IconType<MutatorIcon> {
     return MutatorIcon.TYPE;
   }
 

core/icons/registry.ts

@@ -7,6 +7,7 @@
 import type {Block} from '../block.js';
 import type {IIcon} from '../interfaces/i_icon.js';
 import * as registry from '../registry.js';
+import {IconType} from './icon_types.js';
 
 /**
  * Registers the given icon so that it can be deserialized.
@@ -16,10 +17,10 @@ import * as registry from '../registry.js';
  * @param iconConstructor The icon class/constructor to register.
  */
 export function register(
-  type: string,
+  type: IconType<IIcon>,
   iconConstructor: new (block: Block) => IIcon
 ) {
-  registry.register(registry.Type.ICON, type, iconConstructor);
+  registry.register(registry.Type.ICON, type.toString(), iconConstructor);
 }
 
 /**

core/icons/warning_icon.ts

@@ -17,13 +17,14 @@ import {Rect} from '../utils/rect.js';
 import {Size} from '../utils.js';
 import {Svg} from '../utils/svg.js';
 import {TextBubble} from '../bubbles/text_bubble.js';
+import {IconType} from './icon_types.js';
 
 /** The size of the warning icon in workspace-scale units. */
 const SIZE = 17;
 
 export class WarningIcon extends Icon implements IHasBubble {
   /** The type string used to identify this icon. */
-  static readonly TYPE = 'warning';
+  static readonly TYPE = IconType.WARNING;
 
   /**
    * The weight this icon has relative to other icons. Icons with more positive
@@ -42,7 +43,7 @@ export class WarningIcon extends Icon implements IHasBubble {
     super(sourceBlock);
   }
 
-  override getType(): string {
+  override getType(): IconType<WarningIcon> {
     return WarningIcon.TYPE;
   }
 

core/interfaces/i_icon.ts

@@ -4,16 +4,16 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+import type {IconType} from '../icons/icon_types.js';
 import type {Coordinate} from '../utils/coordinate.js';
 import type {Size} from '../utils/size.js';
 
 export interface IIcon {
   /**
-   * @returns the string representing the type of the icon.
-   * E.g. 'comment', 'warning', etc. This string should also be used when
-   * registering the icon class.
+   * @returns the IconType representing the type of the icon. This value should
+   *     also be used to register the icon via `Blockly.icons.registry.register`.
    */
-  getType(): string;
+  getType(): IconType<IIcon>;
 
   /**
    * Creates the SVG elements for the icon that will live on the block.

core/serialization/blocks.ts

@@ -219,7 +219,7 @@ function saveIcons(block: Block, state: State, doFullSerialization: boolean) {
   for (const icon of block.getIcons()) {
     if (isSerializable(icon)) {
       const state = icon.saveState(doFullSerialization);
-      if (state) icons[icon.getType()] = state;
+      if (state) icons[icon.getType().toString()] = state;
     }
   }
 

core/xml.ts

@@ -12,8 +12,7 @@ import type {BlockSvg} from './block_svg.js';
 import type {Connection} from './connection.js';
 import * as eventUtils from './events/utils.js';
 import type {Field} from './field.js';
-import type {CommentIcon} from './icons/comment_icon.js';
-import {COMMENT_TYPE} from './icons/icon_types.js';
+import {IconType} from './icons/icon_types.js';
 import {inputTypes} from './inputs/input_types.js';
 import * as dom from './utils/dom.js';
 import {Size} from './utils/size.js';
@@ -190,7 +189,7 @@ export function blockToDom(
 
   const commentText = block.getCommentText();
   if (commentText) {
-    const comment = block.getIcon(COMMENT_TYPE) as CommentIcon;
+    const comment = block.getIcon(IconType.COMMENT)!;
     const size = comment.getBubbleSize();
     const pinned = comment.bubbleIsVisible();
 
@@ -723,7 +722,7 @@ function applyCommentTagNodes(xmlChildren: Element[], block: Block) {
     const height = parseInt(xmlChild.getAttribute('h') ?? '50', 10);
 
     block.setCommentText(text);
-    const comment = block.getIcon(COMMENT_TYPE) as CommentIcon;
+    const comment = block.getIcon(IconType.COMMENT)!;
     if (!isNaN(width) && !isNaN(height)) {
       comment.setBubbleSize(new Size(width, height));
     }

tests/mocha/block_test.js

@@ -1443,7 +1443,7 @@ suite('Blocks', function () {
   suite('Icon management', function () {
     class MockIconA extends MockIcon {
       getType() {
-        return 'A';
+        return new Blockly.icons.IconType('A');
       }
 
       getWeight() {
@@ -1453,7 +1453,7 @@ suite('Blocks', function () {
 
     class MockIconB extends MockIcon {
       getType() {
-        return 'B';
+        return new Blockly.icons.IconType('B');
       }
 
       getWeight() {
@@ -1524,7 +1524,7 @@ suite('Blocks', function () {
       test('icons get removed from the block', function () {
         this.block.addIcon(new MockIconA());
         chai.assert.isTrue(
-          this.block.removeIcon('A'),
+          this.block.removeIcon(new Blockly.icons.IconType('A')),
           'Expected removeIcon to return true'
         );
         chai.assert.isFalse(
@@ -1535,7 +1535,7 @@ suite('Blocks', function () {
 
       test('removing an icon that does not exist returns false', function () {
         chai.assert.isFalse(
-          this.block.removeIcon('B'),
+          this.block.removeIcon(new Blockly.icons.IconType('B')),
           'Expected removeIcon to return false'
         );
       });
@@ -1543,7 +1543,7 @@ suite('Blocks', function () {
       test('removing an icon triggers a render', function () {
         this.block.addIcon(new MockIconA());
         this.renderSpy.resetHistory();
-        this.block.removeIcon('A');
+        this.block.removeIcon(new Blockly.icons.IconType('A'));
         chai.assert.isTrue(
           this.renderSpy.calledOnce,
           'Expected removing an icon to trigger a render'

tests/mocha/test_helpers/icon_mocks.js

@@ -6,7 +6,7 @@
 
 export class MockIcon {
   getType() {
-    return 'mock icon';
+    return new Blockly.icons.IconType('mock icon');
   }
 
   initView() {}
@@ -43,7 +43,7 @@ export class MockSerializableIcon extends MockIcon {
   }
 
   getType() {
-    return 'serializable icon';
+    return new Blockly.icons.IconType('serializable icon');
   }
 
   getWeight() {
@@ -66,7 +66,7 @@ export class MockBubbleIcon extends MockIcon {
   }
 
   getType() {
-    return 'bubble icon';
+    return new Blockly.icons.IconType('bubble icon');
   }
 
   updateCollapsed() {}