From 5b7cf09e2cdccd566d27611e86a2f60d714910ac Mon Sep 17 00:00:00 2001 From: Christopher Allen Date: Thu, 22 May 2025 15:44:29 +0100 Subject: [PATCH 01/10] refactor(interfaces): Use typeof ... === 'function' to test for methods Testing for 'name' in object or obj.name !== undefined only checks for the existence of the property (and in the latter case that the property is not set to undefined). That's fine if the interface specifies a property of indeterminate type, but in the usual case that the interface member is a method we can do one better and check to make sure the property's value is callable. --- core/interfaces/i_autohideable.ts | 2 +- core/interfaces/i_comment_icon.ts | 14 +++++------ core/interfaces/i_copyable.ts | 2 +- core/interfaces/i_deletable.ts | 6 ++--- core/interfaces/i_draggable.ts | 12 ++++----- core/interfaces/i_focusable_node.ts | 14 +++++------ core/interfaces/i_focusable_tree.ts | 16 ++++++------ core/interfaces/i_has_bubble.ts | 3 ++- core/interfaces/i_icon.ts | 26 ++++++++++---------- core/interfaces/i_legacy_procedure_blocks.ts | 15 +++++------ core/interfaces/i_observable.ts | 5 +++- core/interfaces/i_paster.ts | 2 +- core/interfaces/i_procedure_block.ts | 7 +++--- core/interfaces/i_rendered_element.ts | 2 +- core/interfaces/i_selectable.ts | 10 ++++---- core/interfaces/i_serializable.ts | 4 ++- 16 files changed, 74 insertions(+), 66 deletions(-) diff --git a/core/interfaces/i_autohideable.ts b/core/interfaces/i_autohideable.ts index 41e761f57ca..2665f9a4f63 100644 --- a/core/interfaces/i_autohideable.ts +++ b/core/interfaces/i_autohideable.ts @@ -23,5 +23,5 @@ export interface IAutoHideable extends IComponent { /** Returns true if the given object is autohideable. */ export function isAutoHideable(obj: any): obj is IAutoHideable { - return obj.autoHide !== undefined; + return typeof obj.autoHide === 'function'; } diff --git a/core/interfaces/i_comment_icon.ts b/core/interfaces/i_comment_icon.ts index 05f86f40ff9..1ab5bead447 100644 --- a/core/interfaces/i_comment_icon.ts +++ b/core/interfaces/i_comment_icon.ts @@ -31,17 +31,17 @@ export interface ICommentIcon extends IIcon, IHasBubble, ISerializable { } /** Checks whether the given object is an ICommentIcon. */ -export function isCommentIcon(obj: object): obj is ICommentIcon { +export function isCommentIcon(obj: any): obj is ICommentIcon { return ( isIcon(obj) && hasBubble(obj) && isSerializable(obj) && - (obj as any)['setText'] !== undefined && - (obj as any)['getText'] !== undefined && - (obj as any)['setBubbleSize'] !== undefined && - (obj as any)['getBubbleSize'] !== undefined && - (obj as any)['setBubbleLocation'] !== undefined && - (obj as any)['getBubbleLocation'] !== undefined && + typeof (obj as any).setText === 'function' && + typeof (obj as any).getText === 'function' && + typeof (obj as any).setBubbleSize === 'function' && + typeof (obj as any).getBubbleSize === 'function' && + typeof (obj as any).setBubbleLocation === 'function' && + typeof (obj as any).getBubbleLocation === 'function' && obj.getType() === IconType.COMMENT ); } diff --git a/core/interfaces/i_copyable.ts b/core/interfaces/i_copyable.ts index 6c354926a64..84125af4ecf 100644 --- a/core/interfaces/i_copyable.ts +++ b/core/interfaces/i_copyable.ts @@ -35,5 +35,5 @@ export type ICopyData = ICopyable.ICopyData; /** @returns true if the given object is an ICopyable. */ export function isCopyable(obj: any): obj is ICopyable { - return obj.toCopyData !== undefined; + return typeof obj.toCopyData === 'function'; } diff --git a/core/interfaces/i_deletable.ts b/core/interfaces/i_deletable.ts index 0467709409a..2673ae2b982 100644 --- a/core/interfaces/i_deletable.ts +++ b/core/interfaces/i_deletable.ts @@ -27,8 +27,8 @@ export interface IDeletable { /** Returns whether the given object is an IDeletable. */ export function isDeletable(obj: any): obj is IDeletable { return ( - obj['isDeletable'] !== undefined && - obj['dispose'] !== undefined && - obj['setDeleteStyle'] !== undefined + typeof obj.isDeletable === 'function' && + typeof obj.dispose === 'function' && + typeof obj.setDeleteStyle === 'function' ); } diff --git a/core/interfaces/i_draggable.ts b/core/interfaces/i_draggable.ts index cb723e7b88b..f266122e3f8 100644 --- a/core/interfaces/i_draggable.ts +++ b/core/interfaces/i_draggable.ts @@ -62,11 +62,11 @@ export interface IDragStrategy { /** Returns whether the given object is an IDraggable or not. */ export function isDraggable(obj: any): obj is IDraggable { return ( - obj.getRelativeToSurfaceXY !== undefined && - obj.isMovable !== undefined && - obj.startDrag !== undefined && - obj.drag !== undefined && - obj.endDrag !== undefined && - obj.revertDrag !== undefined + typeof obj.getRelativeToSurfaceXY === 'function' && + typeof obj.isMovable === 'function' && + typeof obj.startDrag === 'function' && + typeof obj.drag === 'function' && + typeof obj.endDrag === 'function' && + typeof obj.revertDrag === 'function' ); } diff --git a/core/interfaces/i_focusable_node.ts b/core/interfaces/i_focusable_node.ts index 00557168afa..246b64ece99 100644 --- a/core/interfaces/i_focusable_node.ts +++ b/core/interfaces/i_focusable_node.ts @@ -105,13 +105,13 @@ export interface IFocusableNode { * @param object The object to test. * @returns Whether the provided object can be used as an IFocusableNode. */ -export function isFocusableNode(object: any | null): object is IFocusableNode { +export function isFocusableNode(obj: any): obj is IFocusableNode { return ( - object && - 'getFocusableElement' in object && - 'getFocusableTree' in object && - 'onNodeFocus' in object && - 'onNodeBlur' in object && - 'canBeFocused' in object + obj && + typeof obj.getFocusableElement === 'function' && + typeof obj.getFocusableTree === 'function' && + typeof obj.onNodeFocus === 'function' && + typeof obj.onNodeBlur === 'function' && + typeof obj.canBeFocused === 'function' ); } diff --git a/core/interfaces/i_focusable_tree.ts b/core/interfaces/i_focusable_tree.ts index f4f25f7f518..7ffe5bfce9a 100644 --- a/core/interfaces/i_focusable_tree.ts +++ b/core/interfaces/i_focusable_tree.ts @@ -131,14 +131,14 @@ export interface IFocusableTree { * @param object The object to test. * @returns Whether the provided object can be used as an IFocusableTree. */ -export function isFocusableTree(object: any | null): object is IFocusableTree { +export function isFocusableTree(obj: any): obj is IFocusableTree { return ( - object && - 'getRootFocusableNode' in object && - 'getRestoredFocusableNode' in object && - 'getNestedTrees' in object && - 'lookUpFocusableNode' in object && - 'onTreeFocus' in object && - 'onTreeBlur' in object + obj && + typeof obj.getRootFocusableNode === 'function' && + typeof obj.getRestoredFocusableNode === 'function' && + typeof obj.getNestedTrees === 'function' && + typeof obj.lookUpFocusableNode === 'function' && + typeof obj.onTreeFocus === 'function' && + typeof obj.onTreeBlur === 'function' ); } diff --git a/core/interfaces/i_has_bubble.ts b/core/interfaces/i_has_bubble.ts index 85c6f099031..c186585a50e 100644 --- a/core/interfaces/i_has_bubble.ts +++ b/core/interfaces/i_has_bubble.ts @@ -30,6 +30,7 @@ export interface IHasBubble { /** Type guard that checks whether the given object is a IHasBubble. */ export function hasBubble(obj: any): obj is IHasBubble { return ( - obj.bubbleIsVisible !== undefined && obj.setBubbleVisible !== undefined + typeof obj.bubbleIsVisible === 'function' && + typeof obj.setBubbleVisible === 'function' ); } diff --git a/core/interfaces/i_icon.ts b/core/interfaces/i_icon.ts index 74489dc5e09..a2a3fb68f59 100644 --- a/core/interfaces/i_icon.ts +++ b/core/interfaces/i_icon.ts @@ -98,19 +98,19 @@ export interface IIcon extends IFocusableNode { /** Type guard that checks whether the given object is an IIcon. */ export function isIcon(obj: any): obj is IIcon { return ( - obj.getType !== undefined && - obj.initView !== undefined && - obj.dispose !== undefined && - obj.getWeight !== undefined && - obj.getSize !== undefined && - obj.applyColour !== undefined && - obj.hideForInsertionMarker !== undefined && - obj.updateEditable !== undefined && - obj.updateCollapsed !== undefined && - obj.isShownWhenCollapsed !== undefined && - obj.setOffsetInBlock !== undefined && - obj.onLocationChange !== undefined && - obj.onClick !== undefined && + typeof obj.getType === 'function' && + typeof obj.initView === 'function' && + typeof obj.dispose === 'function' && + typeof obj.getWeight === 'function' && + typeof obj.getSize === 'function' && + typeof obj.applyColour === 'function' && + typeof obj.hideForInsertionMarker === 'function' && + typeof obj.updateEditable === 'function' && + typeof obj.updateCollapsed === 'function' && + typeof obj.isShownWhenCollapsed === 'function' && + typeof obj.setOffsetInBlock === 'function' && + typeof obj.onLocationChange === 'function' && + typeof obj.onClick === 'function' && isFocusableNode(obj) ); } diff --git a/core/interfaces/i_legacy_procedure_blocks.ts b/core/interfaces/i_legacy_procedure_blocks.ts index d74eaec220a..c723a5ed77c 100644 --- a/core/interfaces/i_legacy_procedure_blocks.ts +++ b/core/interfaces/i_legacy_procedure_blocks.ts @@ -28,9 +28,9 @@ export interface LegacyProcedureDefBlock { /** @internal */ export function isLegacyProcedureDefBlock( - block: object, -): block is LegacyProcedureDefBlock { - return (block as any).getProcedureDef !== undefined; + obj: any, +): obj is LegacyProcedureDefBlock { + return obj && typeof obj.getProcedureDef === 'function'; } /** @internal */ @@ -41,10 +41,11 @@ export interface LegacyProcedureCallBlock { /** @internal */ export function isLegacyProcedureCallBlock( - block: object, -): block is LegacyProcedureCallBlock { + obj: any, +): obj is LegacyProcedureCallBlock { return ( - (block as any).getProcedureCall !== undefined && - (block as any).renameProcedure !== undefined + obj && + typeof obj.getProcedureCall === 'function' && + typeof obj.renameProcedure === 'function' ); } diff --git a/core/interfaces/i_observable.ts b/core/interfaces/i_observable.ts index 96a2a0bc4e8..e15004b006e 100644 --- a/core/interfaces/i_observable.ts +++ b/core/interfaces/i_observable.ts @@ -20,5 +20,8 @@ export interface IObservable { * @internal */ export function isObservable(obj: any): obj is IObservable { - return obj.startPublishing !== undefined && obj.stopPublishing !== undefined; + return ( + typeof obj.startPublishing === 'function' && + typeof obj.stopPublishing === 'function' + ); } diff --git a/core/interfaces/i_paster.ts b/core/interfaces/i_paster.ts index 321ff118f70..77b11da5582 100644 --- a/core/interfaces/i_paster.ts +++ b/core/interfaces/i_paster.ts @@ -21,5 +21,5 @@ export interface IPaster> { export function isPaster( obj: any, ): obj is IPaster> { - return obj.paste !== undefined; + return typeof obj.paste === 'function'; } diff --git a/core/interfaces/i_procedure_block.ts b/core/interfaces/i_procedure_block.ts index f8538052749..3a6dc4847b9 100644 --- a/core/interfaces/i_procedure_block.ts +++ b/core/interfaces/i_procedure_block.ts @@ -20,9 +20,10 @@ export interface IProcedureBlock { export function isProcedureBlock( block: Block | IProcedureBlock, ): block is IProcedureBlock { + block = block as IProcedureBlock; return ( - (block as IProcedureBlock).getProcedureModel !== undefined && - (block as IProcedureBlock).doProcedureUpdate !== undefined && - (block as IProcedureBlock).isProcedureDef !== undefined + typeof block.getProcedureModel === 'function' && + typeof block.doProcedureUpdate === 'function' && + typeof block.isProcedureDef === 'function' ); } diff --git a/core/interfaces/i_rendered_element.ts b/core/interfaces/i_rendered_element.ts index fe9460c7f6a..a94097ff1b2 100644 --- a/core/interfaces/i_rendered_element.ts +++ b/core/interfaces/i_rendered_element.ts @@ -15,5 +15,5 @@ export interface IRenderedElement { * @returns True if the given object is an IRenderedElement. */ export function isRenderedElement(obj: any): obj is IRenderedElement { - return obj['getSvgRoot'] !== undefined; + return typeof obj.getSvgRoot === 'function'; } diff --git a/core/interfaces/i_selectable.ts b/core/interfaces/i_selectable.ts index 639972e45cb..428849c8d1a 100644 --- a/core/interfaces/i_selectable.ts +++ b/core/interfaces/i_selectable.ts @@ -30,12 +30,12 @@ export interface ISelectable extends IFocusableNode { } /** Checks whether the given object is an ISelectable. */ -export function isSelectable(obj: object): obj is ISelectable { +export function isSelectable(obj: any): obj is ISelectable { return ( - typeof (obj as any).id === 'string' && - (obj as any).workspace !== undefined && - (obj as any).select !== undefined && - (obj as any).unselect !== undefined && + typeof obj.id === 'string' && + typeof obj.workspace === 'function' && + typeof obj.select === 'function' && + typeof obj.unselect === 'function' && isFocusableNode(obj) ); } diff --git a/core/interfaces/i_serializable.ts b/core/interfaces/i_serializable.ts index 380a277095d..cf61c47996e 100644 --- a/core/interfaces/i_serializable.ts +++ b/core/interfaces/i_serializable.ts @@ -24,5 +24,7 @@ export interface ISerializable { /** Type guard that checks whether the given object is a ISerializable. */ export function isSerializable(obj: any): obj is ISerializable { - return obj.saveState !== undefined && obj.loadState !== undefined; + return ( + typeof obj.saveState === 'function' && typeof obj.loadState === 'function' + ); } From 28d7f411dbaef753a6a179dac60d04576b1869cb Mon Sep 17 00:00:00 2001 From: Christopher Allen Date: Thu, 22 May 2025 16:36:13 +0100 Subject: [PATCH 02/10] refactor(interfaces): Always check obj is not null/undefined Since most type predicates take an argument of type any but then check for the existence of certain properties, explicitly check that the argument is not null or undefined (or check implicitly by calling another type predicate that does so first, which necessitates adding a few casts because tsc infers the type of the argument too narrowly). --- core/interfaces/i_autohideable.ts | 2 +- core/interfaces/i_copyable.ts | 2 +- core/interfaces/i_deletable.ts | 1 + core/interfaces/i_draggable.ts | 1 + core/interfaces/i_icon.ts | 28 +++++++++++++-------------- core/interfaces/i_observable.ts | 1 + core/interfaces/i_paster.ts | 2 +- core/interfaces/i_rendered_element.ts | 2 +- core/interfaces/i_selectable.ts | 10 +++++----- core/interfaces/i_serializable.ts | 4 +++- 10 files changed, 29 insertions(+), 24 deletions(-) diff --git a/core/interfaces/i_autohideable.ts b/core/interfaces/i_autohideable.ts index 2665f9a4f63..1193023d21b 100644 --- a/core/interfaces/i_autohideable.ts +++ b/core/interfaces/i_autohideable.ts @@ -23,5 +23,5 @@ export interface IAutoHideable extends IComponent { /** Returns true if the given object is autohideable. */ export function isAutoHideable(obj: any): obj is IAutoHideable { - return typeof obj.autoHide === 'function'; + return obj && typeof obj.autoHide === 'function'; } diff --git a/core/interfaces/i_copyable.ts b/core/interfaces/i_copyable.ts index 84125af4ecf..8d1853967d4 100644 --- a/core/interfaces/i_copyable.ts +++ b/core/interfaces/i_copyable.ts @@ -35,5 +35,5 @@ export type ICopyData = ICopyable.ICopyData; /** @returns true if the given object is an ICopyable. */ export function isCopyable(obj: any): obj is ICopyable { - return typeof obj.toCopyData === 'function'; + return obj && typeof obj.toCopyData === 'function'; } diff --git a/core/interfaces/i_deletable.ts b/core/interfaces/i_deletable.ts index 2673ae2b982..156e43ddc50 100644 --- a/core/interfaces/i_deletable.ts +++ b/core/interfaces/i_deletable.ts @@ -27,6 +27,7 @@ export interface IDeletable { /** Returns whether the given object is an IDeletable. */ export function isDeletable(obj: any): obj is IDeletable { return ( + obj && typeof obj.isDeletable === 'function' && typeof obj.dispose === 'function' && typeof obj.setDeleteStyle === 'function' diff --git a/core/interfaces/i_draggable.ts b/core/interfaces/i_draggable.ts index f266122e3f8..9130381163f 100644 --- a/core/interfaces/i_draggable.ts +++ b/core/interfaces/i_draggable.ts @@ -62,6 +62,7 @@ export interface IDragStrategy { /** Returns whether the given object is an IDraggable or not. */ export function isDraggable(obj: any): obj is IDraggable { return ( + obj && typeof obj.getRelativeToSurfaceXY === 'function' && typeof obj.isMovable === 'function' && typeof obj.startDrag === 'function' && diff --git a/core/interfaces/i_icon.ts b/core/interfaces/i_icon.ts index a2a3fb68f59..06f416424ef 100644 --- a/core/interfaces/i_icon.ts +++ b/core/interfaces/i_icon.ts @@ -98,19 +98,19 @@ export interface IIcon extends IFocusableNode { /** Type guard that checks whether the given object is an IIcon. */ export function isIcon(obj: any): obj is IIcon { return ( - typeof obj.getType === 'function' && - typeof obj.initView === 'function' && - typeof obj.dispose === 'function' && - typeof obj.getWeight === 'function' && - typeof obj.getSize === 'function' && - typeof obj.applyColour === 'function' && - typeof obj.hideForInsertionMarker === 'function' && - typeof obj.updateEditable === 'function' && - typeof obj.updateCollapsed === 'function' && - typeof obj.isShownWhenCollapsed === 'function' && - typeof obj.setOffsetInBlock === 'function' && - typeof obj.onLocationChange === 'function' && - typeof obj.onClick === 'function' && - isFocusableNode(obj) + isFocusableNode(obj) && + typeof (obj as IIcon).getType === 'function' && + typeof (obj as IIcon).initView === 'function' && + typeof (obj as IIcon).dispose === 'function' && + typeof (obj as IIcon).getWeight === 'function' && + typeof (obj as IIcon).getSize === 'function' && + typeof (obj as IIcon).applyColour === 'function' && + typeof (obj as IIcon).hideForInsertionMarker === 'function' && + typeof (obj as IIcon).updateEditable === 'function' && + typeof (obj as IIcon).updateCollapsed === 'function' && + typeof (obj as IIcon).isShownWhenCollapsed === 'function' && + typeof (obj as IIcon).setOffsetInBlock === 'function' && + typeof (obj as IIcon).onLocationChange === 'function' && + typeof (obj as IIcon).onClick === 'function' ); } diff --git a/core/interfaces/i_observable.ts b/core/interfaces/i_observable.ts index e15004b006e..8db0c237874 100644 --- a/core/interfaces/i_observable.ts +++ b/core/interfaces/i_observable.ts @@ -21,6 +21,7 @@ export interface IObservable { */ export function isObservable(obj: any): obj is IObservable { return ( + obj && typeof obj.startPublishing === 'function' && typeof obj.stopPublishing === 'function' ); diff --git a/core/interfaces/i_paster.ts b/core/interfaces/i_paster.ts index 77b11da5582..128913a26b1 100644 --- a/core/interfaces/i_paster.ts +++ b/core/interfaces/i_paster.ts @@ -21,5 +21,5 @@ export interface IPaster> { export function isPaster( obj: any, ): obj is IPaster> { - return typeof obj.paste === 'function'; + return obj && typeof obj.paste === 'function'; } diff --git a/core/interfaces/i_rendered_element.ts b/core/interfaces/i_rendered_element.ts index a94097ff1b2..2f82487e9be 100644 --- a/core/interfaces/i_rendered_element.ts +++ b/core/interfaces/i_rendered_element.ts @@ -15,5 +15,5 @@ export interface IRenderedElement { * @returns True if the given object is an IRenderedElement. */ export function isRenderedElement(obj: any): obj is IRenderedElement { - return typeof obj.getSvgRoot === 'function'; + return obj && typeof obj.getSvgRoot === 'function'; } diff --git a/core/interfaces/i_selectable.ts b/core/interfaces/i_selectable.ts index 428849c8d1a..610c7e339e7 100644 --- a/core/interfaces/i_selectable.ts +++ b/core/interfaces/i_selectable.ts @@ -32,10 +32,10 @@ export interface ISelectable extends IFocusableNode { /** Checks whether the given object is an ISelectable. */ export function isSelectable(obj: any): obj is ISelectable { return ( - typeof obj.id === 'string' && - typeof obj.workspace === 'function' && - typeof obj.select === 'function' && - typeof obj.unselect === 'function' && - isFocusableNode(obj) + isFocusableNode(obj) && + typeof (obj as ISelectable).id === 'string' && + typeof (obj as ISelectable).workspace === 'function' && + typeof (obj as ISelectable).select === 'function' && + typeof (obj as ISelectable).unselect === 'function' ); } diff --git a/core/interfaces/i_serializable.ts b/core/interfaces/i_serializable.ts index cf61c47996e..132d3132a62 100644 --- a/core/interfaces/i_serializable.ts +++ b/core/interfaces/i_serializable.ts @@ -25,6 +25,8 @@ export interface ISerializable { /** Type guard that checks whether the given object is a ISerializable. */ export function isSerializable(obj: any): obj is ISerializable { return ( - typeof obj.saveState === 'function' && typeof obj.loadState === 'function' + typeof obj && + obj.saveState === 'function' && + typeof obj.loadState === 'function' ); } From cdde749374a7801c6d3c6930c31a637860a11be3 Mon Sep 17 00:00:00 2001 From: Christopher Allen Date: Thu, 22 May 2025 15:46:24 +0100 Subject: [PATCH 03/10] fix(interfaces): Add missing check to hasBubble type predicate This appears to have inadvertently been omitted in PR #9004. --- core/interfaces/i_has_bubble.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/interfaces/i_has_bubble.ts b/core/interfaces/i_has_bubble.ts index c186585a50e..0c2e257a440 100644 --- a/core/interfaces/i_has_bubble.ts +++ b/core/interfaces/i_has_bubble.ts @@ -31,6 +31,7 @@ export interface IHasBubble { export function hasBubble(obj: any): obj is IHasBubble { return ( typeof obj.bubbleIsVisible === 'function' && - typeof obj.setBubbleVisible === 'function' + typeof obj.setBubbleVisible === 'function' && + typeof obj.getBubble === 'function' ); } From f83af145dfe317216792788fc48e1c269109743e Mon Sep 17 00:00:00 2001 From: Christopher Allen Date: Tue, 17 Jun 2025 15:14:49 +0100 Subject: [PATCH 04/10] fix(interfaces): Fix misplaced typeof --- core/interfaces/i_serializable.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/interfaces/i_serializable.ts b/core/interfaces/i_serializable.ts index 132d3132a62..99e597da37a 100644 --- a/core/interfaces/i_serializable.ts +++ b/core/interfaces/i_serializable.ts @@ -25,8 +25,8 @@ export interface ISerializable { /** Type guard that checks whether the given object is a ISerializable. */ export function isSerializable(obj: any): obj is ISerializable { return ( - typeof obj && - obj.saveState === 'function' && + obj && + typeof obj.saveState === 'function' && typeof obj.loadState === 'function' ); } From 35276c1caad9f3bf2d9c99e5f5e71cdbf618c642 Mon Sep 17 00:00:00 2001 From: Christopher Allen Date: Tue, 17 Jun 2025 15:16:55 +0100 Subject: [PATCH 05/10] fix: Fix typos in JSDocs --- core/interfaces/i_focusable_node.ts | 2 +- core/interfaces/i_focusable_tree.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/core/interfaces/i_focusable_node.ts b/core/interfaces/i_focusable_node.ts index 246b64ece99..24833328d7f 100644 --- a/core/interfaces/i_focusable_node.ts +++ b/core/interfaces/i_focusable_node.ts @@ -102,7 +102,7 @@ export interface IFocusableNode { * Determines whether the provided object fulfills the contract of * IFocusableNode. * - * @param object The object to test. + * @param obj The object to test. * @returns Whether the provided object can be used as an IFocusableNode. */ export function isFocusableNode(obj: any): obj is IFocusableNode { diff --git a/core/interfaces/i_focusable_tree.ts b/core/interfaces/i_focusable_tree.ts index 7ffe5bfce9a..c33189fcdf0 100644 --- a/core/interfaces/i_focusable_tree.ts +++ b/core/interfaces/i_focusable_tree.ts @@ -128,7 +128,7 @@ export interface IFocusableTree { * Determines whether the provided object fulfills the contract of * IFocusableTree. * - * @param object The object to test. + * @param obj The object to test. * @returns Whether the provided object can be used as an IFocusableTree. */ export function isFocusableTree(obj: any): obj is IFocusableTree { From 32dc18a142bda653409842c11ae9e38602163951 Mon Sep 17 00:00:00 2001 From: Christopher Allen Date: Thu, 19 Jun 2025 15:51:24 +0100 Subject: [PATCH 06/10] fix(tests): Make Mocks conform to corresponding interfaces Introduce a new MockFocusable, and add methods to MockIcon, MockBubbleIcon and MockComment, so that they fulfil the IFocusableNode, IIcon, IHasBubble and ICommentIcon interfaces respectively. --- tests/mocha/block_test.js | 4 ++++ tests/mocha/test_helpers/icon_mocks.js | 16 +++++++++++++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/tests/mocha/block_test.js b/tests/mocha/block_test.js index eda2d82a56d..1512d62c189 100644 --- a/tests/mocha/block_test.js +++ b/tests/mocha/block_test.js @@ -1453,6 +1453,10 @@ suite('Blocks', function () { setBubbleVisible() {} + getBubble() { + return null; + } + saveState() { return {}; } diff --git a/tests/mocha/test_helpers/icon_mocks.js b/tests/mocha/test_helpers/icon_mocks.js index 5d117c71284..72d70f17732 100644 --- a/tests/mocha/test_helpers/icon_mocks.js +++ b/tests/mocha/test_helpers/icon_mocks.js @@ -4,7 +4,16 @@ * SPDX-License-Identifier: Apache-2.0 */ -export class MockIcon { +export class MockFocusable { + getFocusableElement() {} + getFocusableTree() {} + onNodeFocus() {} + onNodeBlur() {} + canBeFocused() {} +} + +export class MockIcon extends MockFocusable { + getType() { return new Blockly.icons.IconType('mock icon'); } @@ -94,4 +103,9 @@ export class MockBubbleIcon extends MockIcon { setBubbleVisible(visible) { this.visible = visible; } + + getBubble() { + return null; + } +} } From cd856a58df56e300c26cf93802cc88847c0a89d0 Mon Sep 17 00:00:00 2001 From: Christopher Allen Date: Thu, 19 Jun 2025 16:51:19 +0100 Subject: [PATCH 07/10] chore(tests): Add assertions verifying mocks conform to predicates Add (test) runtime assertions that: - isFocusableNode(MockFocusable) returns true - isIcon(MockIcon) returns true - hasBubble(MockBubbleIcon) returns true - isCommentIcon(MockCommentIcon) returns true (The latter is currently failing because Blockly is undefined when isCommentIcon calls the MockCommentIcon's getType method.) --- tests/mocha/block_test.js | 5 +++++ tests/mocha/test_helpers/icon_mocks.js | 21 ++++++++++++++++++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/tests/mocha/block_test.js b/tests/mocha/block_test.js index 1512d62c189..736cae6cff5 100644 --- a/tests/mocha/block_test.js +++ b/tests/mocha/block_test.js @@ -8,6 +8,7 @@ import {ConnectionType} from '../../build/src/core/connection_type.js'; import {EventType} from '../../build/src/core/events/type.js'; import * as eventUtils from '../../build/src/core/events/utils.js'; import {EndRowInput} from '../../build/src/core/inputs/end_row_input.js'; +import {isCommentIcon} from '../../build/src/core/interfaces/i_comment_icon.js'; import {assert} from '../../node_modules/chai/chai.js'; import {createRenderedBlock} from './test_helpers/block_definitions.js'; import { @@ -1464,6 +1465,10 @@ suite('Blocks', function () { loadState() {} } + if (!isCommentIcon(new MockComment())) { + throw new TypeError('MockComment not an ICommentIcon'); + } + setup(function () { this.workspace = Blockly.inject('blocklyDiv', {}); diff --git a/tests/mocha/test_helpers/icon_mocks.js b/tests/mocha/test_helpers/icon_mocks.js index 72d70f17732..0e549b9764c 100644 --- a/tests/mocha/test_helpers/icon_mocks.js +++ b/tests/mocha/test_helpers/icon_mocks.js @@ -4,6 +4,11 @@ * SPDX-License-Identifier: Apache-2.0 */ +import {isFocusableNode} from '../../../build/src/core/interfaces/i_focusable_node.js'; +import {hasBubble} from '../../../build/src/core/interfaces/i_has_bubble.js'; +import {isIcon} from '../../../build/src/core/interfaces/i_icon.js'; +import {isSerializable} from '../../../build/src/core/interfaces/i_serializable.js'; + export class MockFocusable { getFocusableElement() {} getFocusableTree() {} @@ -12,8 +17,11 @@ export class MockFocusable { canBeFocused() {} } -export class MockIcon extends MockFocusable { +if (!isFocusableNode(new MockFocusable())) { + throw new TypeError('MockFocusable not an IFocuableNode'); +} +export class MockIcon extends MockFocusable { getType() { return new Blockly.icons.IconType('mock icon'); } @@ -61,6 +69,10 @@ export class MockIcon extends MockFocusable { } } +if (!isIcon(new MockIcon())) { + throw new TypeError('MockIcon not an IIcon'); +} + export class MockSerializableIcon extends MockIcon { constructor() { super(); @@ -84,6 +96,10 @@ export class MockSerializableIcon extends MockIcon { } } +if (!isSerializable(new MockSerializableIcon())) { + throw new TypeError('MockSerializableIcon not an ISerializable'); +} + export class MockBubbleIcon extends MockIcon { constructor() { super(); @@ -108,4 +124,7 @@ export class MockBubbleIcon extends MockIcon { return null; } } + +if (!hasBubble(new MockBubbleIcon())) { + throw new TypeError('MockBubbleIcon not an IHasBubble'); } From 0149600f1a00a0b0f79c239af7ef839ec8577a00 Mon Sep 17 00:00:00 2001 From: Christopher Allen Date: Thu, 19 Jun 2025 16:52:28 +0100 Subject: [PATCH 08/10] fix(tests): Don't rely on Blockly being set in Mock methods For some reason the global Blockly binding is not visible at the time when isCommentIcon calls MockCommentIcon's getType method, and presumably this problem would apply to getBubbleSize too, so directly import the required items. --- tests/mocha/block_test.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/mocha/block_test.js b/tests/mocha/block_test.js index 736cae6cff5..45de5e67b76 100644 --- a/tests/mocha/block_test.js +++ b/tests/mocha/block_test.js @@ -7,8 +7,10 @@ import {ConnectionType} from '../../build/src/core/connection_type.js'; import {EventType} from '../../build/src/core/events/type.js'; import * as eventUtils from '../../build/src/core/events/utils.js'; +import {IconType} from '../../build/src/core/icons/icon_types.js'; import {EndRowInput} from '../../build/src/core/inputs/end_row_input.js'; import {isCommentIcon} from '../../build/src/core/interfaces/i_comment_icon.js'; +import {Size} from '../../build/src/core/utils/size.js'; import {assert} from '../../node_modules/chai/chai.js'; import {createRenderedBlock} from './test_helpers/block_definitions.js'; import { @@ -1429,7 +1431,7 @@ suite('Blocks', function () { suite('Constructing registered comment classes', function () { class MockComment extends MockIcon { getType() { - return Blockly.icons.IconType.COMMENT; + return IconType.COMMENT; } setText() {} @@ -1441,7 +1443,7 @@ suite('Blocks', function () { setBubbleSize() {} getBubbleSize() { - return Blockly.utils.Size(0, 0); + return Size(0, 0); } setBubbleLocation() {} From feec7cc062780993ae3cb6bf7cfa35769534ff81 Mon Sep 17 00:00:00 2001 From: Christopher Allen Date: Thu, 19 Jun 2025 16:55:40 +0100 Subject: [PATCH 09/10] refactor(tests): Make MockCommentIcon a MockBubbleIcon This slightly simplifies it and makes it less likely to accidentally stop conforming to IHasBubble. --- tests/mocha/block_test.js | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/tests/mocha/block_test.js b/tests/mocha/block_test.js index 45de5e67b76..62c61ce004c 100644 --- a/tests/mocha/block_test.js +++ b/tests/mocha/block_test.js @@ -1429,7 +1429,7 @@ suite('Blocks', function () { }); suite('Constructing registered comment classes', function () { - class MockComment extends MockIcon { + class MockComment extends MockBubbleIcon { getType() { return IconType.COMMENT; } @@ -1450,16 +1450,6 @@ suite('Blocks', function () { getBubbleLocation() {} - bubbleIsVisible() { - return true; - } - - setBubbleVisible() {} - - getBubble() { - return null; - } - saveState() { return {}; } From a909d9507c11db422ea1472d7ce378344800ce1c Mon Sep 17 00:00:00 2001 From: Christopher Allen Date: Thu, 19 Jun 2025 17:22:25 +0100 Subject: [PATCH 10/10] fix(interfaces): Fix incorrect check in isSelectable Fix an error which caused ISelectable instances to fail isSelectable() checks, one of the results of which is that Blockly.common.getSelected() would generally return null. Whoops! --- core/interfaces/i_selectable.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/interfaces/i_selectable.ts b/core/interfaces/i_selectable.ts index 610c7e339e7..5374f50cd3a 100644 --- a/core/interfaces/i_selectable.ts +++ b/core/interfaces/i_selectable.ts @@ -34,7 +34,7 @@ export function isSelectable(obj: any): obj is ISelectable { return ( isFocusableNode(obj) && typeof (obj as ISelectable).id === 'string' && - typeof (obj as ISelectable).workspace === 'function' && + typeof (obj as ISelectable).workspace === 'object' && typeof (obj as ISelectable).select === 'function' && typeof (obj as ISelectable).unselect === 'function' );