From 032b5ed9eaa87f610beb34f977992aa1c0c8ff0c Mon Sep 17 00:00:00 2001 From: Christopher Allen Date: Tue, 20 Aug 2024 19:50:29 +0100 Subject: [PATCH] refactor(events): Introduce and use event type predicates (#8538) * refactor(events): Introduce type predicates for event classes Introduce predicates for testing Abstract event subclasses based on their .type properties. These are useful because there are places where it is not possible to use instanceof tests for type narrowing due to load ordering issues that would be caused by the need to import (rather than just import type) the class constructors in question. * refactor(events): Use event type predicates Simplify several sections of code by using type predicates for type narrowing and thereby avoiding the need for explicit casts. * chore(events): Fix copyright date of recently-added files * chore: Remove unused import --- core/bump_objects.ts | 11 +-- core/events/predicates.ts | 172 +++++++++++++++++++++++++++++++++++++ core/events/type.ts | 2 +- core/events/utils.ts | 60 ++++++------- core/icons/mutator_icon.ts | 6 +- core/procedures.ts | 20 +++-- core/trashcan.ts | 34 +++----- 7 files changed, 231 insertions(+), 74 deletions(-) create mode 100644 core/events/predicates.ts diff --git a/core/bump_objects.ts b/core/bump_objects.ts index 7fe1e38513..2aae257dde 100644 --- a/core/bump_objects.ts +++ b/core/bump_objects.ts @@ -13,7 +13,7 @@ import type {BlockMove} from './events/events_block_move.js'; import type {CommentCreate} from './events/events_comment_create.js'; import type {CommentMove} from './events/events_comment_move.js'; import type {CommentResize} from './events/events_comment_resize.js'; -import type {ViewportChange} from './events/events_viewport.js'; +import {isViewportChange} from './events/predicates.js'; import {BUMP_EVENTS, EventType} from './events/type.js'; import * as eventUtils from './events/utils.js'; import type {IBoundedElement} from './interfaces/i_bounded_element.js'; @@ -128,13 +128,8 @@ export function bumpIntoBoundsHandler( ); } eventUtils.setGroup(existingGroup); - } else if (e.type === EventType.VIEWPORT_CHANGE) { - const viewportEvent = e as ViewportChange; - if ( - viewportEvent.scale && - viewportEvent.oldScale && - viewportEvent.scale > viewportEvent.oldScale - ) { + } else if (isViewportChange(e)) { + if (e.scale && e.oldScale && e.scale > e.oldScale) { bumpTopObjectsIntoBounds(workspace); } } diff --git a/core/events/predicates.ts b/core/events/predicates.ts new file mode 100644 index 0000000000..79d8ca284e --- /dev/null +++ b/core/events/predicates.ts @@ -0,0 +1,172 @@ +/** + * @license + * Copyright 2024 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * @file Predicates for testing Abstract event subclasses based on + * their .type properties. These are useful because there are places + * where it is not possible to use instanceof tests + * for type narrowing due to load ordering issues that would be caused + * by the need to import (rather than just import type) the class + * constructors in question. + */ + +import type {Abstract} from './events_abstract.js'; +import type {BlockChange} from './events_block_change.js'; +import type {BlockCreate} from './events_block_create.js'; +import type {BlockDelete} from './events_block_delete.js'; +import type {BlockDrag} from './events_block_drag.js'; +import type {BlockFieldIntermediateChange} from './events_block_field_intermediate_change.js'; +import type {BlockMove} from './events_block_move.js'; +import type {BubbleOpen} from './events_bubble_open.js'; +import type {Click} from './events_click.js'; +import type {CommentChange} from './events_comment_change.js'; +import type {CommentCollapse} from './events_comment_collapse.js'; +import type {CommentCreate} from './events_comment_create.js'; +import type {CommentDelete} from './events_comment_delete.js'; +import type {CommentDrag} from './events_comment_drag.js'; +import type {CommentMove} from './events_comment_move.js'; +import type {CommentResize} from './events_comment_resize.js'; +import type {MarkerMove} from './events_marker_move.js'; +import type {Selected} from './events_selected.js'; +import type {ThemeChange} from './events_theme_change.js'; +import type {ToolboxItemSelect} from './events_toolbox_item_select.js'; +import type {TrashcanOpen} from './events_trashcan_open.js'; +import type {VarCreate} from './events_var_create.js'; +import type {VarDelete} from './events_var_delete.js'; +import type {VarRename} from './events_var_rename.js'; +import type {ViewportChange} from './events_viewport.js'; +import type {FinishedLoading} from './workspace_events.js'; + +import {EventType} from './type.js'; + +/** @returns true iff event.type is EventType.BLOCK_CREATE */ +export function isBlockCreate(event: Abstract): event is BlockCreate { + return event.type === EventType.BLOCK_CREATE; +} + +/** @returns true iff event.type is EventType.BLOCK_DELETE */ +export function isBlockDelete(event: Abstract): event is BlockDelete { + return event.type === EventType.BLOCK_DELETE; +} + +/** @returns true iff event.type is EventType.BLOCK_CHANGE */ +export function isBlockChange(event: Abstract): event is BlockChange { + return event.type === EventType.BLOCK_CHANGE; +} + +/** @returns true iff event.type is EventType.BLOCK_FIELD_INTERMEDIATE_CHANGE */ +export function isBlockFieldIntermediateChange( + event: Abstract, +): event is BlockFieldIntermediateChange { + return event.type === EventType.BLOCK_FIELD_INTERMEDIATE_CHANGE; +} + +/** @returns true iff event.type is EventType.BLOCK_MOVE */ +export function isBlockMove(event: Abstract): event is BlockMove { + return event.type === EventType.BLOCK_MOVE; +} + +/** @returns true iff event.type is EventType.VAR_CREATE */ +export function isVarCreate(event: Abstract): event is VarCreate { + return event.type === EventType.VAR_CREATE; +} + +/** @returns true iff event.type is EventType.VAR_DELETE */ +export function isVarDelete(event: Abstract): event is VarDelete { + return event.type === EventType.VAR_DELETE; +} + +/** @returns true iff event.type is EventType.VAR_RENAME */ +export function isVarRename(event: Abstract): event is VarRename { + return event.type === EventType.VAR_RENAME; +} + +/** @returns true iff event.type is EventType.BLOCK_DRAG */ +export function isBlockDrag(event: Abstract): event is BlockDrag { + return event.type === EventType.BLOCK_DRAG; +} + +/** @returns true iff event.type is EventType.SELECTED */ +export function isSelected(event: Abstract): event is Selected { + return event.type === EventType.SELECTED; +} + +/** @returns true iff event.type is EventType.CLICK */ +export function isClick(event: Abstract): event is Click { + return event.type === EventType.CLICK; +} + +/** @returns true iff event.type is EventType.MARKER_MOVE */ +export function isMarkerMove(event: Abstract): event is MarkerMove { + return event.type === EventType.MARKER_MOVE; +} + +/** @returns true iff event.type is EventType.BUBBLE_OPEN */ +export function isBubbleOpen(event: Abstract): event is BubbleOpen { + return event.type === EventType.BUBBLE_OPEN; +} + +/** @returns true iff event.type is EventType.TRASHCAN_OPEN */ +export function isTrashcanOpen(event: Abstract): event is TrashcanOpen { + return event.type === EventType.TRASHCAN_OPEN; +} + +/** @returns true iff event.type is EventType.TOOLBOX_ITEM_SELECT */ +export function isToolboxItemSelect( + event: Abstract, +): event is ToolboxItemSelect { + return event.type === EventType.TOOLBOX_ITEM_SELECT; +} + +/** @returns true iff event.type is EventType.THEME_CHANGE */ +export function isThemeChange(event: Abstract): event is ThemeChange { + return event.type === EventType.THEME_CHANGE; +} + +/** @returns true iff event.type is EventType.VIEWPORT_CHANGE */ +export function isViewportChange(event: Abstract): event is ViewportChange { + return event.type === EventType.VIEWPORT_CHANGE; +} + +/** @returns true iff event.type is EventType.COMMENT_CREATE */ +export function isCommentCreate(event: Abstract): event is CommentCreate { + return event.type === EventType.COMMENT_CREATE; +} + +/** @returns true iff event.type is EventType.COMMENT_DELETE */ +export function isCommentDelete(event: Abstract): event is CommentDelete { + return event.type === EventType.COMMENT_DELETE; +} + +/** @returns true iff event.type is EventType.COMMENT_CHANGE */ +export function isCommentChange(event: Abstract): event is CommentChange { + return event.type === EventType.COMMENT_CHANGE; +} + +/** @returns true iff event.type is EventType.COMMENT_MOVE */ +export function isCommentMove(event: Abstract): event is CommentMove { + return event.type === EventType.COMMENT_MOVE; +} + +/** @returns true iff event.type is EventType.COMMENT_RESIZE */ +export function isCommentResize(event: Abstract): event is CommentResize { + return event.type === EventType.COMMENT_RESIZE; +} + +/** @returns true iff event.type is EventType.COMMENT_DRAG */ +export function isCommentDrag(event: Abstract): event is CommentDrag { + return event.type === EventType.COMMENT_DRAG; +} + +/** @returns true iff event.type is EventType.COMMENT_COLLAPSE */ +export function isCommentCollapse(event: Abstract): event is CommentCollapse { + return event.type === EventType.COMMENT_COLLAPSE; +} + +/** @returns true iff event.type is EventType.FINISHED_LOADING */ +export function isFinishedLoading(event: Abstract): event is FinishedLoading { + return event.type === EventType.FINISHED_LOADING; +} diff --git a/core/events/type.ts b/core/events/type.ts index 71060efbb3..db9ad6c96a 100644 --- a/core/events/type.ts +++ b/core/events/type.ts @@ -1,6 +1,6 @@ /** * @license - * Copyright 2021 Google LLC + * Copyright 2024 Google LLC * SPDX-License-Identifier: Apache-2.0 */ diff --git a/core/events/utils.ts b/core/events/utils.ts index d0aad5f534..63217168fc 100644 --- a/core/events/utils.ts +++ b/core/events/utils.ts @@ -13,13 +13,19 @@ import * as idGenerator from '../utils/idgenerator.js'; import type {Workspace} from '../workspace.js'; import type {WorkspaceSvg} from '../workspace_svg.js'; import type {Abstract} from './events_abstract.js'; -import type {BlockChange} from './events_block_change.js'; import type {BlockCreate} from './events_block_create.js'; import type {BlockMove} from './events_block_move.js'; import type {CommentCreate} from './events_comment_create.js'; import type {CommentMove} from './events_comment_move.js'; import type {CommentResize} from './events_comment_resize.js'; -import type {ViewportChange} from './events_viewport.js'; +import { + isBlockChange, + isBlockCreate, + isBlockMove, + isBubbleOpen, + isClick, + isViewportChange, +} from './predicates.js'; import {EventType} from './type.js'; /** Group ID for new events. Grouped events are indivisible. */ @@ -188,46 +194,35 @@ export function filter(queueIn: Abstract[], forward: boolean): Abstract[] { // move events. hash[key] = {event, index: i}; mergedQueue.push(event); - } else if ( - event.type === EventType.BLOCK_MOVE && - lastEntry.index === i - 1 - ) { - const moveEvent = event as BlockMove; + } else if (isBlockMove(event) && lastEntry.index === i - 1) { // Merge move events. - lastEvent.newParentId = moveEvent.newParentId; - lastEvent.newInputName = moveEvent.newInputName; - lastEvent.newCoordinate = moveEvent.newCoordinate; - if (moveEvent.reason) { + lastEvent.newParentId = event.newParentId; + lastEvent.newInputName = event.newInputName; + lastEvent.newCoordinate = event.newCoordinate; + if (event.reason) { if (lastEvent.reason) { // Concatenate reasons without duplicates. - const reasonSet = new Set( - moveEvent.reason.concat(lastEvent.reason), - ); + const reasonSet = new Set(event.reason.concat(lastEvent.reason)); lastEvent.reason = Array.from(reasonSet); } else { - lastEvent.reason = moveEvent.reason; + lastEvent.reason = event.reason; } } lastEntry.index = i; } else if ( - event.type === EventType.BLOCK_CHANGE && - (event as BlockChange).element === lastEvent.element && - (event as BlockChange).name === lastEvent.name + isBlockChange(event) && + event.element === lastEvent.element && + event.name === lastEvent.name ) { - const changeEvent = event as BlockChange; // Merge change events. - lastEvent.newValue = changeEvent.newValue; - } else if (event.type === EventType.VIEWPORT_CHANGE) { - const viewportEvent = event as ViewportChange; + lastEvent.newValue = event.newValue; + } else if (isViewportChange(event)) { // Merge viewport change events. - lastEvent.viewTop = viewportEvent.viewTop; - lastEvent.viewLeft = viewportEvent.viewLeft; - lastEvent.scale = viewportEvent.scale; - lastEvent.oldScale = viewportEvent.oldScale; - } else if ( - event.type === EventType.CLICK && - lastEvent.type === EventType.BUBBLE_OPEN - ) { + lastEvent.viewTop = event.viewTop; + lastEvent.viewLeft = event.viewLeft; + lastEvent.scale = event.scale; + lastEvent.oldScale = event.oldScale; + } else if (isClick(event) && isBubbleOpen(lastEvent)) { // Drop click events caused by opening/closing bubbles. } else { // Collision: newer events should merge into this event to maintain @@ -381,10 +376,7 @@ export function get( * @param event Custom data for event. */ export function disableOrphans(event: Abstract) { - if ( - event.type === EventType.BLOCK_MOVE || - event.type === EventType.BLOCK_CREATE - ) { + if (isBlockMove(event) || isBlockCreate(event)) { const blockEvent = event as BlockMove | BlockCreate; if (!blockEvent.workspaceId) { return; diff --git a/core/icons/mutator_icon.ts b/core/icons/mutator_icon.ts index 5c137917a3..d02c7e1871 100644 --- a/core/icons/mutator_icon.ts +++ b/core/icons/mutator_icon.ts @@ -11,6 +11,7 @@ import type {BlocklyOptions} from '../blockly_options.js'; import {MiniWorkspaceBubble} from '../bubbles/mini_workspace_bubble.js'; import type {Abstract} from '../events/events_abstract.js'; import {BlockChange} from '../events/events_block_change.js'; +import {isBlockChange, isBlockCreate} from '../events/predicates.js'; import {EventType} from '../events/type.js'; import * as eventUtils from '../events/utils.js'; import type {IHasBubble} from '../interfaces/i_has_bubble.js'; @@ -308,9 +309,8 @@ export class MutatorIcon extends Icon implements IHasBubble { static isIgnorableMutatorEvent(e: Abstract) { return ( e.isUiEvent || - e.type === EventType.BLOCK_CREATE || - (e.type === EventType.BLOCK_CHANGE && - (e as BlockChange).element === 'disabled') + isBlockCreate(e) || + (isBlockChange(e) && e.element === 'disabled') ); } diff --git a/core/procedures.ts b/core/procedures.ts index bad4ef05a2..a16b0fce44 100644 --- a/core/procedures.ts +++ b/core/procedures.ts @@ -15,6 +15,13 @@ import {Blocks} from './blocks.js'; import * as common from './common.js'; import type {Abstract} from './events/events_abstract.js'; import type {BubbleOpen} from './events/events_bubble_open.js'; +import { + isBlockChange, + isBlockCreate, + isBlockDelete, + isBlockFieldIntermediateChange, + isBubbleOpen, +} from './events/predicates.js'; import {EventType} from './events/type.js'; import * as eventUtils from './events/utils.js'; import {Field, UnattachedFieldError} from './field.js'; @@ -355,9 +362,8 @@ function updateMutatorFlyout(workspace: WorkspaceSvg) { * @internal */ export function mutatorOpenListener(e: Abstract) { - if (e.type !== EventType.BUBBLE_OPEN) { - return; - } + if (!isBubbleOpen(e)) return; + const bubbleEvent = e as BubbleOpen; if ( !(bubbleEvent.bubbleType === 'mutator' && bubbleEvent.isOpen) || @@ -387,10 +393,10 @@ export function mutatorOpenListener(e: Abstract) { */ function mutatorChangeListener(e: Abstract) { if ( - e.type !== EventType.BLOCK_CREATE && - e.type !== EventType.BLOCK_DELETE && - e.type !== EventType.BLOCK_CHANGE && - e.type !== EventType.BLOCK_FIELD_INTERMEDIATE_CHANGE + !isBlockCreate(e) && + !isBlockDelete(e) && + !isBlockChange(e) && + !isBlockFieldIntermediateChange(e) ) { return; } diff --git a/core/trashcan.ts b/core/trashcan.ts index 32b56221ae..d30c4a4e22 100644 --- a/core/trashcan.ts +++ b/core/trashcan.ts @@ -17,8 +17,8 @@ import * as browserEvents from './browser_events.js'; import {ComponentManager} from './component_manager.js'; import {DeleteArea} from './delete_area.js'; import type {Abstract} from './events/events_abstract.js'; -import type {BlockDelete} from './events/events_block_delete.js'; import './events/events_trashcan_open.js'; +import {isBlockDelete} from './events/predicates.js'; import {EventType} from './events/type.js'; import * as eventUtils from './events/utils.js'; import type {IAutoHideable} from './interfaces/i_autohideable.js'; @@ -604,30 +604,22 @@ export class Trashcan private onDelete(event: Abstract) { if ( this.workspace.options.maxTrashcanContents <= 0 || - event.type !== EventType.BLOCK_DELETE + !isBlockDelete(event) || + event.wasShadow ) { return; } - const deleteEvent = event as BlockDelete; - if (event.type === EventType.BLOCK_DELETE && !deleteEvent.wasShadow) { - if (!deleteEvent.oldJson) { - throw new Error('Encountered a delete event without proper oldJson'); - } - const cleanedJson = JSON.stringify( - this.cleanBlockJson(deleteEvent.oldJson), - ); - if (this.contents.includes(cleanedJson)) { - return; - } - this.contents.unshift(cleanedJson); - while ( - this.contents.length > this.workspace.options.maxTrashcanContents - ) { - this.contents.pop(); - } - - this.setMinOpenness(HAS_BLOCKS_LID_ANGLE); + if (!event.oldJson) { + throw new Error('Encountered a delete event without proper oldJson'); } + const cleanedJson = JSON.stringify(this.cleanBlockJson(event.oldJson)); + if (this.contents.includes(cleanedJson)) return; + this.contents.unshift(cleanedJson); + while (this.contents.length > this.workspace.options.maxTrashcanContents) { + this.contents.pop(); + } + + this.setMinOpenness(HAS_BLOCKS_LID_ANGLE); } /**