From 3f15f1ecb75ed898fce883e5a4fc01205e2225aa Mon Sep 17 00:00:00 2001 From: Aaron Dodson Date: Fri, 12 Aug 2022 09:18:43 -0700 Subject: [PATCH] refactor: Use maps and sets instead of plain objects. (#6331) * refactor: Convert objects to sets and maps * refactor: Use maps instead of objects in workspace.ts. * refactor: Use maps instead of objects in theme_manager.ts. * refactor: Use maps instead of objects in block_svg.ts. * refactor: Use sets instead of objects in variables.ts. * refactor: Use maps instead of objects in marker_manager.ts. * refactor: Use maps instead of objects in touch_gesture.ts. * refactor: Use maps instead of objects in variable_map.ts. * refactor: Use maps and sets instead of objects in path_object.ts. * refactor: Use maps instead of objects in shortcut_registry.ts. * refactor: Use a map instead of an object in workspace_audio.ts. * refactor: Use better ivar names and enforce the singleton ShortcutRegistry object and type. * refactor: Use public API in the shortcut registry test. * refactor: Simplify implementation of getAllVariableNames(). * refactor: Remove unnecessary emptiness check in block_svg.ts. * fix: clang-format variable_map.ts. --- core/block_svg.ts | 45 +++++------ core/component_manager.ts | 64 +++++++--------- core/contextmenu_registry.ts | 23 +++--- core/marker_manager.ts | 25 +++--- core/names.ts | 68 ++++++++--------- core/renderers/zelos/path_object.ts | 69 ++++++++--------- core/shortcut_registry.ts | 92 +++++++++++----------- core/theme_manager.ts | 30 +++----- core/touch_gesture.ts | 47 ++++-------- core/variable_map.ts | 88 ++++++++++----------- core/variables.ts | 22 +++--- core/workspace.ts | 41 +++++----- core/workspace_audio.ts | 19 ++--- tests/mocha/shortcut_registry_test.js | 105 +++++++++++++++----------- tests/mocha/test_helpers/workspace.js | 42 +++++------ tests/mocha/variable_map_test.js | 20 ++--- tests/mocha/workspace_comment_test.js | 12 +-- 17 files changed, 375 insertions(+), 437 deletions(-) diff --git a/core/block_svg.ts b/core/block_svg.ts index e937c42a3..7f49efa93 100644 --- a/core/block_svg.ts +++ b/core/block_svg.ts @@ -115,10 +115,7 @@ export class BlockSvg extends Block implements IASTNodeLocationSvg, * Map from IDs for warnings text to PIDs of functions to apply them. * Used to be able to maintain multiple warnings. */ - // AnyDuringMigration because: Type 'null' is not assignable to type '{ [key: - // string]: number; }'. - private warningTextDb_: {[key: string]: AnyDuringMigration} = - null as AnyDuringMigration; + private warningTextDb = new Map>(); /** Block's mutator icon (if any). */ mutator: Mutator|null = null; @@ -880,14 +877,10 @@ export class BlockSvg extends Block implements IASTNodeLocationSvg, this.rendered = false; // Clear pending warnings. - if (this.warningTextDb_) { - for (const n in this.warningTextDb_) { - clearTimeout(this.warningTextDb_[n]); - } - // AnyDuringMigration because: Type 'null' is not assignable to type '{ - // [key: string]: number; }'. - this.warningTextDb_ = null as AnyDuringMigration; + for (const n of this.warningTextDb.values()) { + clearTimeout(n); } + this.warningTextDb.clear(); const icons = this.getIcons(); for (let i = 0; i < icons.length; i++) { @@ -1045,33 +1038,29 @@ export class BlockSvg extends Block implements IASTNodeLocationSvg, * multiple warnings. */ override setWarningText(text: string|null, opt_id?: string) { - if (!this.warningTextDb_) { - // Create a database of warning PIDs. - // Only runs once per block (and only those with warnings). - this.warningTextDb_ = Object.create(null); - } const id = opt_id || ''; if (!id) { // Kill all previous pending processes, this edit supersedes them all. - for (const n of Object.keys(this.warningTextDb_)) { - clearTimeout(this.warningTextDb_[n]); - delete this.warningTextDb_[n]; + for (const timeout of this.warningTextDb.values()) { + clearTimeout(timeout); } - } else if (this.warningTextDb_[id]) { + this.warningTextDb.clear(); + } else if (this.warningTextDb.has(id)) { // Only queue up the latest change. Kill any earlier pending process. - clearTimeout(this.warningTextDb_[id]); - delete this.warningTextDb_[id]; + clearTimeout(this.warningTextDb.get(id)!); + this.warningTextDb.delete(id); } if (this.workspace.isDragging()) { // Don't change the warning text during a drag. // Wait until the drag finishes. const thisBlock = this; - this.warningTextDb_[id] = setTimeout(function() { - if (!thisBlock.disposed) { // Check block wasn't deleted. - delete thisBlock.warningTextDb_[id]; - thisBlock.setWarningText(text, id); - } - }, 100); + this.warningTextDb.set( + id, setTimeout(() => { + if (!this.disposed) { // Check block wasn't deleted. + this.warningTextDb.delete(id); + this.setWarningText(text, id); + } + }, 100)); return; } if (this.isInFlyout) { diff --git a/core/component_manager.ts b/core/component_manager.ts index a7648ba7f..36f6e1d29 100644 --- a/core/component_manager.ts +++ b/core/component_manager.ts @@ -50,20 +50,13 @@ class Capability { export class ComponentManager { static Capability = Capability; - // static Capability: AnyDuringMigration; - private readonly componentData_: {[key: string]: ComponentDatum}; - private readonly capabilityToComponentIds_: {[key: string]: string[]}; + /** + * A map of the components registered with the workspace, mapped to id. + */ + private readonly componentData = new Map(); - /** Creates a new ComponentManager instance. */ - constructor() { - /** - * A map of the components registered with the workspace, mapped to id. - */ - this.componentData_ = Object.create(null); - - /** A map of capabilities to component IDs. */ - this.capabilityToComponentIds_ = Object.create(null); - } + /** A map of capabilities to component IDs. */ + private readonly capabilityToComponentIds = new Map(); /** * Adds a component. @@ -74,23 +67,23 @@ export class ComponentManager { addComponent(componentInfo: ComponentDatum, opt_allowOverrides?: boolean) { // Don't throw an error if opt_allowOverrides is true. const id = componentInfo.component.id; - if (!opt_allowOverrides && this.componentData_[id]) { + if (!opt_allowOverrides && this.componentData.has(id)) { throw Error( 'Plugin "' + id + '" with capabilities "' + - this.componentData_[id].capabilities + '" already added.'); + this.componentData.get(id)?.capabilities + '" already added.'); } - this.componentData_[id] = componentInfo; + this.componentData.set(id, componentInfo); const stringCapabilities = []; for (let i = 0; i < componentInfo.capabilities.length; i++) { const capability = String(componentInfo.capabilities[i]).toLowerCase(); stringCapabilities.push(capability); - if (this.capabilityToComponentIds_[capability] === undefined) { - this.capabilityToComponentIds_[capability] = [id]; + if (!this.capabilityToComponentIds.has(capability)) { + this.capabilityToComponentIds.set(capability, [id]); } else { - this.capabilityToComponentIds_[capability].push(id); + this.capabilityToComponentIds.get(capability)?.push(id); } } - this.componentData_[id].capabilities = stringCapabilities; + this.componentData.get(id)!.capabilities = stringCapabilities; } /** @@ -98,15 +91,15 @@ export class ComponentManager { * @param id The ID of the component to remove. */ removeComponent(id: string) { - const componentInfo = this.componentData_[id]; + const componentInfo = this.componentData.get(id); if (!componentInfo) { return; } for (let i = 0; i < componentInfo.capabilities.length; i++) { const capability = String(componentInfo.capabilities[i]).toLowerCase(); - arrayUtils.removeElem(this.capabilityToComponentIds_[capability], id); + arrayUtils.removeElem(this.capabilityToComponentIds.get(capability)!, id); } - delete this.componentData_[id]; + this.componentData.delete(id); } /** @@ -126,8 +119,8 @@ export class ComponentManager { return; } capability = String(capability).toLowerCase(); - this.componentData_[id].capabilities.push(capability); - this.capabilityToComponentIds_[capability].push(id); + this.componentData.get(id)?.capabilities.push(capability); + this.capabilityToComponentIds.get(capability)?.push(id); } /** @@ -148,8 +141,8 @@ export class ComponentManager { return; } capability = String(capability).toLowerCase(); - arrayUtils.removeElem(this.componentData_[id].capabilities, capability); - arrayUtils.removeElem(this.capabilityToComponentIds_[capability], id); + arrayUtils.removeElem(this.componentData.get(id)!.capabilities, capability); + arrayUtils.removeElem(this.capabilityToComponentIds.get(capability)!, id); } /** @@ -160,7 +153,8 @@ export class ComponentManager { */ hasCapability(id: string, capability: string|Capability): boolean { capability = String(capability).toLowerCase(); - return this.componentData_[id].capabilities.indexOf(capability) !== -1; + return this.componentData.has(id) && + this.componentData.get(id)!.capabilities.indexOf(capability) !== -1; } /** @@ -169,7 +163,7 @@ export class ComponentManager { * @return The component with the given name or undefined if not found. */ getComponent(id: string): IComponent|undefined { - return this.componentData_[id] && this.componentData_[id].component; + return this.componentData.get(id)?.component; } /** @@ -180,16 +174,15 @@ export class ComponentManager { */ getComponents(capability: string|Capability, sorted: boolean): T[] { capability = String(capability).toLowerCase(); - const componentIds = this.capabilityToComponentIds_[capability]; + const componentIds = this.capabilityToComponentIds.get(capability); if (!componentIds) { return []; } const components: AnyDuringMigration[] = []; if (sorted) { const componentDataList: AnyDuringMigration[] = []; - const componentData = this.componentData_; - componentIds.forEach(function(id) { - componentDataList.push(componentData[id]); + componentIds.forEach((id) => { + componentDataList.push(this.componentData.get(id)); }); componentDataList.sort(function(a, b) { return a.weight - b.weight; @@ -198,9 +191,8 @@ export class ComponentManager { components.push(ComponentDatum.component); }); } else { - const componentData = this.componentData_; - componentIds.forEach(function(id) { - components.push(componentData[id].component); + componentIds.forEach((id) => { + components.push(this.componentData.get(id)!.component); }); } return components; diff --git a/core/contextmenu_registry.ts b/core/contextmenu_registry.ts index a963eda80..fc0fe3ef6 100644 --- a/core/contextmenu_registry.ts +++ b/core/contextmenu_registry.ts @@ -27,8 +27,8 @@ import type {WorkspaceSvg} from './workspace_svg.js'; */ export class ContextMenuRegistry { static registry: ContextMenuRegistry; - // TODO(b/109816955): remove '!', see go/strict-prop-init-fix. - private registry_!: {[key: string]: RegistryItem}; + /** Registry of all registered RegistryItems, keyed by ID. */ + private registry_ = new Map(); /** Resets the existing singleton instance of ContextMenuRegistry. */ constructor() { @@ -37,8 +37,7 @@ export class ContextMenuRegistry { /** Clear and recreate the registry. */ reset() { - /** Registry of all registered RegistryItems, keyed by ID. */ - this.registry_ = Object.create(null); + this.registry_.clear(); } /** @@ -47,10 +46,10 @@ export class ContextMenuRegistry { * @throws {Error} if an item with the given ID already exists. */ register(item: RegistryItem) { - if (this.registry_[item.id]) { + if (this.registry_.has(item.id)) { throw Error('Menu item with ID "' + item.id + '" is already registered.'); } - this.registry_[item.id] = item; + this.registry_.set(item.id, item); } /** @@ -59,10 +58,10 @@ export class ContextMenuRegistry { * @throws {Error} if an item with the given ID does not exist. */ unregister(id: string) { - if (!this.registry_[id]) { + if (!this.registry_.has(id)) { throw new Error('Menu item with ID "' + id + '" not found.'); } - delete this.registry_[id]; + this.registry_.delete(id); } /** @@ -70,7 +69,7 @@ export class ContextMenuRegistry { * @return RegistryItem or null if not found */ getItem(id: string): RegistryItem|null { - return this.registry_[id] || null; + return this.registry_.get(id) ?? null; } /** @@ -86,9 +85,7 @@ export class ContextMenuRegistry { getContextMenuOptions(scopeType: ScopeType, scope: Scope): ContextMenuOption[] { const menuOptions: AnyDuringMigration[] = []; - const registry = this.registry_; - Object.keys(registry).forEach(function(id) { - const item = registry[id]; + for (const item of this.registry_.values()) { if (scopeType === item.scopeType) { const precondition = item.preconditionFn(scope); if (precondition !== 'hidden') { @@ -105,7 +102,7 @@ export class ContextMenuRegistry { menuOptions.push(menuOption); } } - }); + } menuOptions.sort(function(a, b) { return a.weight - b.weight; }); diff --git a/core/marker_manager.ts b/core/marker_manager.ts index b3a24b968..a0a3c8640 100644 --- a/core/marker_manager.ts +++ b/core/marker_manager.ts @@ -33,7 +33,9 @@ export class MarkerManager { /** The cursor's SVG element. */ private cursorSvg_: SVGElement|null = null; - private markers_: {[key: string]: Marker}; + + /** The map of markers for the workspace. */ + private markers = new Map(); /** The marker's SVG element. */ private markerSvg_: SVGElement|null = null; @@ -42,10 +44,7 @@ export class MarkerManager { * @param workspace The workspace for the marker manager. * @internal */ - constructor(private readonly workspace: WorkspaceSvg) { - /** The map of markers for the workspace. */ - this.markers_ = Object.create(null); - } + constructor(private readonly workspace: WorkspaceSvg) {} /** * Register the marker by adding it to the map of markers. @@ -53,13 +52,13 @@ export class MarkerManager { * @param marker The marker to register. */ registerMarker(id: string, marker: Marker) { - if (this.markers_[id]) { + if (this.markers.has(id)) { this.unregisterMarker(id); } marker.setDrawer( this.workspace.getRenderer().makeMarkerDrawer(this.workspace, marker)); this.setMarkerSvg(marker.getDrawer().createDom()); - this.markers_[id] = marker; + this.markers.set(id, marker); } /** @@ -67,10 +66,10 @@ export class MarkerManager { * @param id The ID of the marker to unregister. */ unregisterMarker(id: string) { - const marker = this.markers_[id]; + const marker = this.markers.get(id); if (marker) { marker.dispose(); - delete this.markers_[id]; + this.markers.delete(id); } else { throw Error( 'Marker with ID ' + id + ' does not exist. ' + @@ -93,7 +92,7 @@ export class MarkerManager { * exists. */ getMarker(id: string): Marker|null { - return this.markers_[id] || null; + return this.markers.get(id) || null; } /** @@ -169,13 +168,11 @@ export class MarkerManager { * @internal */ dispose() { - const markerIds = Object.keys(this.markers_); + const markerIds = Object.keys(this.markers); for (let i = 0, markerId; markerId = markerIds[i]; i++) { this.unregisterMarker(markerId); } - // AnyDuringMigration because: Type 'null' is not assignable to type '{ - // [key: string]: Marker; }'. - this.markers_ = null as AnyDuringMigration; + this.markers.clear(); if (this.cursor_) { this.cursor_.dispose(); this.cursor_ = null; diff --git a/core/names.ts b/core/names.ts index b8e2eb023..b6faf8abd 100644 --- a/core/names.ts +++ b/core/names.ts @@ -32,9 +32,18 @@ import type {Workspace} from './workspace.js'; export class Names { static DEVELOPER_VARIABLE_TYPE: NameType; private readonly variablePrefix_: string; - private readonly reservedDict_: AnyDuringMigration; - private db_: {[key: string]: {[key: string]: string}}; - private dbReverse_: {[key: string]: boolean}; + + /** A set of reserved words. */ + private readonly reservedWords: Set; + + /** + * A map from type (e.g. name, procedure) to maps from names to generated + * names. + */ + private readonly db = new Map>(); + + /** A set of used names to avoid collisions. */ + private readonly dbReverse = new Set(); /** * The variable map from the workspace, containing Blockly variable models. @@ -42,42 +51,25 @@ export class Names { private variableMap_: VariableMap|null = null; /** - * @param reservedWords A comma-separated string of words that are illegal for - * use as names in a language (e.g. 'new,if,this,...'). + * @param reservedWordsList A comma-separated string of words that are illegal + * for use as names in a language (e.g. 'new,if,this,...'). * @param opt_variablePrefix Some languages need a '$' or a namespace before * all variable names (but not procedure names). */ - constructor(reservedWords: string, opt_variablePrefix?: string) { + constructor(reservedWordsList: string, opt_variablePrefix?: string) { /** The prefix to attach to variable names in generated code. */ this.variablePrefix_ = opt_variablePrefix || ''; - /** A dictionary of reserved words. */ - this.reservedDict_ = Object.create(null); - - /** - * A map from type (e.g. name, procedure) to maps from names to generated - * names. - */ - this.db_ = Object.create(null); - - /** A map from used names to booleans to avoid collisions. */ - this.dbReverse_ = Object.create(null); - - if (reservedWords) { - const splitWords = reservedWords.split(','); - for (let i = 0; i < splitWords.length; i++) { - this.reservedDict_[splitWords[i]] = true; - } - } - this.reset(); + this.reservedWords = + new Set(reservedWordsList ? reservedWordsList.split(',') : []); } /** * Empty the database and start from scratch. The reserved words are kept. */ reset() { - this.db_ = Object.create(null); - this.dbReverse_ = Object.create(null); + this.db.clear(); + this.dbReverse.clear(); this.variableMap_ = null; } @@ -156,15 +148,15 @@ export class Names { type === NameType.VARIABLE || type === NameType.DEVELOPER_VARIABLE; const prefix = isVar ? this.variablePrefix_ : ''; - if (!(type in this.db_)) { - this.db_[type] = Object.create(null); + if (!this.db.has(type)) { + this.db.set(type, new Map()); } - const typeDb = this.db_[type]; - if (normalizedName in typeDb) { - return prefix + typeDb[normalizedName]; + const typeDb = this.db.get(type); + if (typeDb!.has(normalizedName)) { + return prefix + typeDb!.get(normalizedName); } const safeName = this.getDistinctName(name, type); - typeDb[normalizedName] = safeName.substr(prefix.length); + typeDb!.set(normalizedName, safeName.substr(prefix.length)); return safeName; } @@ -175,8 +167,8 @@ export class Names { * @return A list of Blockly entity names (no constraints). */ getUserNames(type: NameType|string): string[] { - const typeDb = this.db_[type] || {}; - return Object.keys(typeDb); + const userNames = this.db.get(type)?.keys(); + return userNames ? Array.from(userNames) : []; } /** @@ -192,15 +184,15 @@ export class Names { getDistinctName(name: string, type: NameType|string): string { let safeName = this.safeName_(name); let i = ''; - while (this.dbReverse_[safeName + i] || - safeName + i in this.reservedDict_) { + while (this.dbReverse.has(safeName + i) || + this.reservedWords.has(safeName + i)) { // Collision with existing name. Create a unique name. // AnyDuringMigration because: Type 'string | 2' is not assignable to // type 'string'. i = (i ? i + 1 : 2) as AnyDuringMigration; } safeName += i; - this.dbReverse_[safeName] = true; + this.dbReverse.add(safeName); const isVar = type === NameType.VARIABLE || type === NameType.DEVELOPER_VARIABLE; const prefix = isVar ? this.variablePrefix_ : ''; diff --git a/core/renderers/zelos/path_object.ts b/core/renderers/zelos/path_object.ts index 6e8098565..da2b9964f 100644 --- a/core/renderers/zelos/path_object.ts +++ b/core/renderers/zelos/path_object.ts @@ -37,18 +37,17 @@ import type {ConstantProvider} from './constants.js'; export class PathObject extends BasePathObject { /** The selected path of the block. */ private svgPathSelected_: SVGElement|null = null; - private readonly outlines_: {[key: string]: SVGElement}; + + /** The outline paths on the block. */ + private readonly outlines = new Map(); /** * A set used to determine which outlines were used during a draw pass. The * set is initialized with a reference to all the outlines in - * `this.outlines_`. Every time we use an outline during the draw pass, the + * `this.outlines`. Every time we use an outline during the draw pass, the * reference is removed from this set. */ - // AnyDuringMigration because: Type 'null' is not assignable to type '{ [key: - // string]: number; }'. - private remainingOutlines_: {[key: string]: number} = - null as AnyDuringMigration; + private remainingOutlines = new Set(); /** * The type of block's output connection shape. This is set when a block @@ -70,9 +69,6 @@ export class PathObject extends BasePathObject { super(root, style, constants); this.constants = constants; - - /** The outline paths on the block. */ - this.outlines_ = Object.create(null); } override setPath(pathString: string) { @@ -91,16 +87,16 @@ export class PathObject extends BasePathObject { } // Apply colour to outlines. - for (const key in this.outlines_) { - this.outlines_[key].setAttribute('fill', this.style.colourTertiary); + for (const outline of this.outlines.values()) { + outline.setAttribute('fill', this.style.colourTertiary); } } override flipRTL() { super.flipRTL(); // Mirror each input outline path. - for (const key in this.outlines_) { - this.outlines_[key].setAttribute('transform', 'scale(-1 1)'); + for (const outline of this.outlines.values()) { + outline.setAttribute('transform', 'scale(-1 1)'); } } @@ -151,11 +147,9 @@ export class PathObject extends BasePathObject { * @internal */ beginDrawing() { - this.remainingOutlines_ = Object.create(null); - for (const key in this.outlines_) { - // The value set here isn't used anywhere, we are just using the - // object as a Set data structure. - this.remainingOutlines_[key] = 1; + this.remainingOutlines.clear() + for (const key of this.outlines.keys()) { + this.remainingOutlines.add(key); } } @@ -166,14 +160,12 @@ export class PathObject extends BasePathObject { endDrawing() { // Go through all remaining outlines that were not used this draw pass, and // remove them. - if (this.remainingOutlines_) { - for (const key in this.remainingOutlines_) { + if (this.remainingOutlines.size) { + for (const key of this.remainingOutlines) { this.removeOutlinePath_(key); } } - // AnyDuringMigration because: Type 'null' is not assignable to type '{ - // [key: string]: number; }'. - this.remainingOutlines_ = null as AnyDuringMigration; + this.remainingOutlines.clear(); } /** @@ -195,20 +187,21 @@ export class PathObject extends BasePathObject { * @return The SVG outline path. */ private getOutlinePath_(name: string): SVGElement { - if (!this.outlines_[name]) { - this.outlines_[name] = dom.createSvgElement( - Svg.PATH, { - 'class': 'blocklyOutlinePath', // IE doesn't like paths without the - // data definition, set empty - // default - 'd': '', - }, - this.svgRoot); + if (!this.outlines.has(name)) { + this.outlines.set( + name, + dom.createSvgElement( + Svg.PATH, { + 'class': + 'blocklyOutlinePath', // IE doesn't like paths without the + // data definition, set empty + // default + 'd': '', + }, + this.svgRoot)); } - if (this.remainingOutlines_) { - delete this.remainingOutlines_[name]; - } - return this.outlines_[name]; + this.remainingOutlines.delete(name); + return this.outlines.get(name)!; } /** @@ -216,7 +209,7 @@ export class PathObject extends BasePathObject { * @param name The input name. */ private removeOutlinePath_(name: string) { - this.outlines_[name].parentNode!.removeChild(this.outlines_[name]); - delete this.outlines_[name]; + this.outlines.get(name)?.parentNode?.removeChild(this.outlines.get(name)!); + this.outlines.delete(name); } } diff --git a/core/shortcut_registry.ts b/core/shortcut_registry.ts index c1be1962c..6a7ad1625 100644 --- a/core/shortcut_registry.ts +++ b/core/shortcut_registry.ts @@ -29,24 +29,23 @@ import type {Workspace} from './workspace.js'; * @alias Blockly.ShortcutRegistry */ export class ShortcutRegistry { - static registry: AnyDuringMigration; - // TODO(b/109816955): remove '!', see go/strict-prop-init-fix. - private registry_!: {[key: string]: KeyboardShortcut}; - // TODO(b/109816955): remove '!', see go/strict-prop-init-fix. - private keyMap_!: {[key: string]: string[]}; + static readonly registry = new ShortcutRegistry(); + + /** Registry of all keyboard shortcuts, keyed by name of shortcut. */ + private shortcuts = new Map(); + + /** Map of key codes to an array of shortcut names. */ + private keyMap = new Map(); /** Resets the existing ShortcutRegistry singleton. */ - constructor() { + private constructor() { this.reset(); } /** Clear and recreate the registry and keyMap. */ reset() { - /** Registry of all keyboard shortcuts, keyed by name of shortcut. */ - this.registry_ = Object.create(null); - - /** Map of key codes to an array of shortcut names. */ - this.keyMap_ = Object.create(null); + this.shortcuts.clear(); + this.keyMap.clear(); } /** @@ -57,12 +56,12 @@ export class ShortcutRegistry { * @throws {Error} if a shortcut with the same name already exists. */ register(shortcut: KeyboardShortcut, opt_allowOverrides?: boolean) { - const registeredShortcut = this.registry_[shortcut.name]; + const registeredShortcut = this.shortcuts.get(shortcut.name); if (registeredShortcut && !opt_allowOverrides) { throw new Error( 'Shortcut with name "' + shortcut.name + '" already exists.'); } - this.registry_[shortcut.name] = shortcut; + this.shortcuts.set(shortcut.name, shortcut); const keyCodes = shortcut.keyCodes; if (keyCodes && keyCodes.length > 0) { @@ -80,7 +79,7 @@ export class ShortcutRegistry { * @return True if an item was unregistered, false otherwise. */ unregister(shortcutName: string): boolean { - const shortcut = this.registry_[shortcutName]; + const shortcut = this.shortcuts.get(shortcutName); if (!shortcut) { console.warn( @@ -90,7 +89,7 @@ export class ShortcutRegistry { this.removeAllKeyMappings(shortcutName); - delete this.registry_[shortcutName]; + this.shortcuts.delete(shortcutName); return true; } @@ -109,7 +108,7 @@ export class ShortcutRegistry { keyCode: string|number|KeyCodes, shortcutName: string, opt_allowCollision?: boolean) { keyCode = String(keyCode); - const shortcutNames = this.keyMap_[keyCode]; + const shortcutNames = this.keyMap.get(keyCode); if (shortcutNames && !opt_allowCollision) { throw new Error( 'Shortcut with name "' + shortcutName + '" collides with shortcuts ' + @@ -117,7 +116,7 @@ export class ShortcutRegistry { } else if (shortcutNames && opt_allowCollision) { shortcutNames.unshift(shortcutName); } else { - this.keyMap_[keyCode] = [shortcutName]; + this.keyMap.set(keyCode, [shortcutName]); } } @@ -134,12 +133,14 @@ export class ShortcutRegistry { */ removeKeyMapping(keyCode: string, shortcutName: string, opt_quiet?: boolean): boolean { - const shortcutNames = this.keyMap_[keyCode]; + const shortcutNames = this.keyMap.get(keyCode); - if (!shortcutNames && !opt_quiet) { - console.warn( - 'No keyboard shortcut with name "' + shortcutName + - '" registered with key code "' + keyCode + '"'); + if (!shortcutNames) { + if (!opt_quiet) { + console.warn( + 'No keyboard shortcut with name "' + shortcutName + + '" registered with key code "' + keyCode + '"'); + } return false; } @@ -147,7 +148,7 @@ export class ShortcutRegistry { if (shortcutIdx > -1) { shortcutNames.splice(shortcutIdx, 1); if (shortcutNames.length === 0) { - delete this.keyMap_[keyCode]; + this.keyMap.delete(keyCode); } return true; } @@ -166,7 +167,7 @@ export class ShortcutRegistry { * @param shortcutName The name of the shortcut to remove from the key map. */ removeAllKeyMappings(shortcutName: string) { - for (const keyCode in this.keyMap_) { + for (const keyCode of this.keyMap.keys()) { this.removeKeyMapping(keyCode, shortcutName, true); } } @@ -174,18 +175,25 @@ export class ShortcutRegistry { /** * Sets the key map. Setting the key map will override any default key * mappings. - * @param keyMap The object with key code to shortcut names. + * @param newKeyMap The object with key code to shortcut names. */ - setKeyMap(keyMap: {[key: string]: string[]}) { - this.keyMap_ = keyMap; + setKeyMap(newKeyMap: {[key: string]: string[]}) { + this.keyMap.clear(); + for (const key in newKeyMap) { + this.keyMap.set(key, newKeyMap[key]); + } } /** * Gets the current key map. * @return The object holding key codes to ShortcutRegistry.KeyboardShortcut. */ - getKeyMap(): {[key: string]: KeyboardShortcut[]} { - return object.deepMerge(Object.create(null), this.keyMap_); + getKeyMap(): {[key: string]: string[]} { + const legacyKeyMap: {[key: string]: string[]} = Object.create(null); + for (const [key, value] of this.keyMap) { + legacyKeyMap[key] = value; + } + return legacyKeyMap; } /** @@ -193,7 +201,12 @@ export class ShortcutRegistry { * @return The registry of keyboard shortcuts. */ getRegistry(): {[key: string]: KeyboardShortcut} { - return object.deepMerge(Object.create(null), this.registry_); + const legacyRegistry: {[key: string]: KeyboardShortcut} = + Object.create(null); + for (const [key, value] of this.shortcuts) { + legacyRegistry[key] = value; + } + return object.deepMerge(Object.create(null), legacyRegistry); } /** @@ -209,10 +222,10 @@ export class ShortcutRegistry { return false; } for (let i = 0, shortcutName; shortcutName = shortcutNames[i]; i++) { - const shortcut = this.registry_[shortcutName]; - if (!shortcut.preconditionFn || shortcut.preconditionFn(workspace)) { + const shortcut = this.shortcuts.get(shortcutName); + if (!shortcut?.preconditionFn || shortcut?.preconditionFn(workspace)) { // If the key has been handled, stop processing shortcuts. - if (shortcut.callback && shortcut.callback(workspace, e, shortcut)) { + if (shortcut?.callback && shortcut?.callback(workspace, e, shortcut)) { return true; } } @@ -227,7 +240,7 @@ export class ShortcutRegistry { * Undefined if no shortcuts exist. */ getShortcutNamesByKeyCode(keyCode: string): string[]|undefined { - return this.keyMap_[keyCode] || []; + return this.keyMap.get(keyCode) || []; } /** @@ -238,8 +251,7 @@ export class ShortcutRegistry { */ getKeyCodesByShortcutName(shortcutName: string): string[] { const keys = []; - for (const keyCode in this.keyMap_) { - const shortcuts = this.keyMap_[keyCode]; + for (const [keyCode, shortcuts] of this.keyMap) { const shortcutIdx = shortcuts.indexOf(shortcutName); if (shortcutIdx > -1) { keys.push(keyCode); @@ -276,7 +288,7 @@ export class ShortcutRegistry { * @param modifiers List of modifiers to be used with the key. * @throws {Error} if the modifier is not in the valid modifiers list. */ - private checkModifiers_(modifiers: string[]) { + private checkModifiers_(modifiers: KeyCodes[]) { const validModifiers = object.values(ShortcutRegistry.modifierKeys); for (let i = 0, modifier; modifier = modifiers[i]; i++) { if (validModifiers.indexOf(modifier) < 0) { @@ -292,7 +304,7 @@ export class ShortcutRegistry { * valid modifiers can be found in the ShortcutRegistry.modifierKeys. * @return The serialized key code for the given modifiers and key. */ - createSerializedKey(keyCode: number, modifiers: string[]|null): string { + createSerializedKey(keyCode: number, modifiers: KeyCodes[]|null): string { let serializedKey = ''; if (modifiers) { @@ -341,7 +353,3 @@ export type KeyboardShortcut = ShortcutRegistry.KeyboardShortcut; // No need to export ShorcutRegistry.modifierKeys from the module at this time // because (1) it wasn't automatically converted by the automatic migration // script, (2) the name doesn't follow the styleguide. - -// Creates and assigns the singleton instance. -const registry = new ShortcutRegistry(); -ShortcutRegistry.registry = registry; diff --git a/core/theme_manager.ts b/core/theme_manager.ts index 3ef40f5b5..56f902596 100644 --- a/core/theme_manager.ts +++ b/core/theme_manager.ts @@ -31,7 +31,7 @@ import type {WorkspaceSvg} from './workspace_svg.js'; export class ThemeManager { /** A list of workspaces that are subscribed to this theme. */ private subscribedWorkspaces_: Workspace[] = []; - private componentDB_: {[key: string]: Component[]}; + private componentDB = new Map(); owner_: AnyDuringMigration; /** @@ -39,10 +39,7 @@ export class ThemeManager { * @param theme The workspace theme. * @internal */ - constructor(private readonly workspace: WorkspaceSvg, private theme: Theme) { - /** A map of subscribed UI components, keyed by component name. */ - this.componentDB_ = Object.create(null); - } + constructor(private readonly workspace: WorkspaceSvg, private theme: Theme) {} /** * Get the workspace theme. @@ -76,9 +73,8 @@ export class ThemeManager { } // Refresh all registered Blockly UI components. - for (let i = 0, keys = Object.keys(this.componentDB_), key; key = keys[i]; - i++) { - for (let j = 0, component; component = this.componentDB_[key][j]; j++) { + for (const [key, componentList] of this.componentDB) { + for (const component of componentList) { const element = component.element; const propertyName = component.propertyName; const style = this.theme && this.theme.getComponentStyle(key); @@ -125,12 +121,12 @@ export class ThemeManager { * @internal */ subscribe(element: Element, componentName: string, propertyName: string) { - if (!this.componentDB_[componentName]) { - this.componentDB_[componentName] = []; + if (!this.componentDB.has(componentName)) { + this.componentDB.set(componentName, []); } // Add the element to our component map. - this.componentDB_[componentName].push({element, propertyName}); + this.componentDB.get(componentName)!.push({element, propertyName}); // Initialize the element with its corresponding theme style. const style = this.theme && this.theme.getComponentStyle(componentName); @@ -149,17 +145,15 @@ export class ThemeManager { return; } // Go through all component, and remove any references to this element. - const componentNames = Object.keys(this.componentDB_); - for (let c = 0, componentName; componentName = componentNames[c]; c++) { - const elements = this.componentDB_[componentName]; + for (const [componentName, elements] of this.componentDB) { for (let i = elements.length - 1; i >= 0; i--) { if (elements[i].element === element) { elements.splice(i, 1); } } // Clean up the component map entry if the list is empty. - if (!this.componentDB_[componentName].length) { - delete this.componentDB_[componentName]; + if (!elements.length) { + this.componentDB.delete(componentName); } } } @@ -177,9 +171,7 @@ export class ThemeManager { // AnyDuringMigration because: Type 'null' is not assignable to type // 'Workspace[]'. this.subscribedWorkspaces_ = null as AnyDuringMigration; - // AnyDuringMigration because: Type 'null' is not assignable to type '{ - // [key: string]: Component[]; }'. - this.componentDB_ = null as AnyDuringMigration; + this.componentDB.clear(); } } diff --git a/core/touch_gesture.ts b/core/touch_gesture.ts index 21de9fcf1..b2c07ece9 100644 --- a/core/touch_gesture.ts +++ b/core/touch_gesture.ts @@ -42,7 +42,9 @@ const ZOOM_OUT_MULTIPLIER = 6; export class TouchGesture extends Gesture { /** Boolean for whether or not this gesture is a multi-touch gesture. */ private isMultiTouch_ = false; - private cachedPoints_: {[key: string]: Coordinate}; + + /** A map of cached points used for tracking multi-touch gestures. */ + private cachedPoints = new Map(); /** * This is the ratio between the starting distance between the touch points @@ -67,18 +69,6 @@ export class TouchGesture extends Gesture { override onMoveWrapper_: AnyDuringMigration; override onUpWrapper_: AnyDuringMigration; - /** - * @param e The event that kicked off this gesture. - * @param creatorWorkspace The workspace that created this gesture and has a - * reference to it. - */ - constructor(e: Event, creatorWorkspace: WorkspaceSvg) { - super(e, creatorWorkspace); - - /** A map of cached points used for tracking multi-touch gestures. */ - this.cachedPoints_ = Object.create(null); - } - /** * Start a gesture: update the workspace to indicate that a gesture is in * progress and bind mousemove and mouseup handlers. @@ -213,14 +203,12 @@ export class TouchGesture extends Gesture { handleTouchStart(e: Event) { const pointerId = Touch.getTouchIdentifierFromEvent(e); // store the pointerId in the current list of pointers - // AnyDuringMigration because: Type 'Coordinate | null' is not assignable - // to type 'Coordinate'. - this.cachedPoints_[pointerId] = this.getTouchPoint(e) as AnyDuringMigration; - const pointers = Object.keys(this.cachedPoints_); + this.cachedPoints.set(pointerId, this.getTouchPoint(e)); + const pointers = Array.from(this.cachedPoints.keys()); // If two pointers are down, store info if (pointers.length === 2) { - const point0 = (this.cachedPoints_[pointers[0]]); - const point1 = (this.cachedPoints_[pointers[1]]); + const point0 = (this.cachedPoints.get(pointers[0]))!; + const point1 = (this.cachedPoints.get(pointers[1]))!; this.startDistance_ = Coordinate.distance(point0, point1); this.isMultiTouch_ = true; e.preventDefault(); @@ -236,12 +224,9 @@ export class TouchGesture extends Gesture { handleTouchMove(e: MouseEvent) { const pointerId = Touch.getTouchIdentifierFromEvent(e); // Update the cache - // AnyDuringMigration because: Type 'Coordinate | null' is not assignable - // to type 'Coordinate'. - this.cachedPoints_[pointerId] = this.getTouchPoint(e) as AnyDuringMigration; + this.cachedPoints.set(pointerId, this.getTouchPoint(e)); - const pointers = Object.keys(this.cachedPoints_); - if (this.isPinchZoomEnabled_ && pointers.length === 2) { + if (this.isPinchZoomEnabled_ && this.cachedPoints.size === 2) { this.handlePinch_(e); } else { super.handleMove(e); @@ -253,10 +238,10 @@ export class TouchGesture extends Gesture { * @param e A touch move, or pointer move event. */ private handlePinch_(e: MouseEvent) { - const pointers = Object.keys(this.cachedPoints_); + const pointers = Array.from(this.cachedPoints.keys()); // Calculate the distance between the two pointers - const point0 = (this.cachedPoints_[pointers[0]]); - const point1 = (this.cachedPoints_[pointers[1]]); + const point0 = (this.cachedPoints.get(pointers[0]))!; + const point1 = (this.cachedPoints.get(pointers[1]))!; const moveDistance = Coordinate.distance(point0, point1); const scale = moveDistance / this.startDistance_; @@ -280,11 +265,11 @@ export class TouchGesture extends Gesture { */ handleTouchEnd(e: Event) { const pointerId = Touch.getTouchIdentifierFromEvent(e); - if (this.cachedPoints_[pointerId]) { - delete this.cachedPoints_[pointerId]; + if (this.cachedPoints.has(pointerId)) { + this.cachedPoints.delete(pointerId); } - if (Object.keys(this.cachedPoints_).length < 2) { - this.cachedPoints_ = Object.create(null); + if (this.cachedPoints.size < 2) { + this.cachedPoints.clear(); this.previousScale_ = 0; } } diff --git a/core/variable_map.ts b/core/variable_map.ts index 5c7bec694..b8644bb76 100644 --- a/core/variable_map.ts +++ b/core/variable_map.ts @@ -38,21 +38,19 @@ import type {Workspace} from './workspace.js'; * @alias Blockly.VariableMap */ export class VariableMap { - private variableMap_: {[key: string]: VariableModel[]}; + /** + * A map from variable type to list of variable names. The lists contain + * all of the named variables in the workspace, including variables that are + * not currently in use. + */ + private variableMap = new Map(); /** @param workspace The workspace this map belongs to. */ - constructor(public workspace: Workspace) { - /** - * A map from variable type to list of variable names. The lists contain - * all of the named variables in the workspace, including variables that are - * not currently in use. - */ - this.variableMap_ = Object.create(null); - } + constructor(public workspace: Workspace) {} /** Clear the variable map. */ clear() { - this.variableMap_ = Object.create(null); + this.variableMap.clear(); } /* Begin functions for renaming variables. */ /** @@ -141,7 +139,7 @@ export class VariableMap { // Finally delete the original variable, which is now unreferenced. eventUtils.fire(new (eventUtils.get(eventUtils.VAR_DELETE))!(variable)); // And remove it from the list. - arrayUtils.removeElem(this.variableMap_[type], variable); + arrayUtils.removeElem(this.variableMap.get(type)!, variable); } /* End functions for renaming variables. */ /** @@ -174,14 +172,14 @@ export class VariableMap { const type = opt_type || ''; variable = new VariableModel(this.workspace, name, type, id); - const variables = this.variableMap_[type] || []; + const variables = this.variableMap.get(type) || []; variables.push(variable); // Delete the list of variables of this type, and re-add it so that // the most recent addition is at the end. // This is used so the toolbox's set block is set to the most recent // variable. - delete this.variableMap_[type]; - this.variableMap_[type] = variables; + this.variableMap.delete(type); + this.variableMap.set(type, variables); return variable; } @@ -192,13 +190,16 @@ export class VariableMap { */ deleteVariable(variable: VariableModel) { const variableId = variable.getId(); - const variableList = this.variableMap_[variable.type]; - for (let i = 0; i < variableList.length; i++) { - const tempVar = variableList[i]; - if (tempVar.getId() === variableId) { - variableList.splice(i, 1); - eventUtils.fire(new (eventUtils.get(eventUtils.VAR_DELETE))!(variable)); - return; + const variableList = this.variableMap.get(variable.type); + if (variableList) { + for (let i = 0; i < variableList.length; i++) { + const tempVar = variableList[i]; + if (tempVar.getId() === variableId) { + variableList.splice(i, 1); + eventUtils.fire(new (eventUtils.get(eventUtils.VAR_DELETE))! + (variable)); + return; + } } } } @@ -280,7 +281,7 @@ export class VariableMap { */ getVariable(name: string, opt_type?: string|null): VariableModel|null { const type = opt_type || ''; - const list = this.variableMap_[type]; + const list = this.variableMap.get(type); if (list) { for (let j = 0, variable; variable = list[j]; j++) { if (Names.equals(variable.name, name)) { @@ -297,10 +298,8 @@ export class VariableMap { * @return The variable with the given ID. */ getVariableById(id: string): VariableModel|null { - const keys = Object.keys(this.variableMap_); - for (let i = 0; i < keys.length; i++) { - const key = keys[i]; - for (let j = 0, variable; variable = this.variableMap_[key][j]; j++) { + for (const [key, variables] of this.variableMap) { + for (const variable of variables) { if (variable.getId() === id) { return variable; } @@ -318,7 +317,7 @@ export class VariableMap { */ getVariablesOfType(type: string|null): VariableModel[] { type = type || ''; - const variableList = this.variableMap_[type]; + const variableList = this.variableMap.get(type); if (variableList) { return variableList.slice(); } @@ -335,22 +334,16 @@ export class VariableMap { * @internal */ getVariableTypes(ws: Workspace|null): string[] { - const variableMap = {}; - Object.assign(variableMap, this.variableMap_); + const variableTypes = new Set(this.variableMap.keys()); if (ws && ws.getPotentialVariableMap()) { - Object.assign(variableMap, ws.getPotentialVariableMap()!.variableMap_); - } - const types = Object.keys(variableMap); - let hasEmpty = false; - for (let i = 0; i < types.length; i++) { - if (types[i] === '') { - hasEmpty = true; + for (const key of ws.getPotentialVariableMap()!.variableMap.keys()) { + variableTypes.add(key); } } - if (!hasEmpty) { - types.push(''); + if (!variableTypes.has('')) { + variableTypes.add(''); } - return types; + return Array.from(variableTypes.values()); } /** @@ -358,9 +351,9 @@ export class VariableMap { * @return List of variable models. */ getAllVariables(): VariableModel[] { - let allVariables: AnyDuringMigration[] = []; - for (const key in this.variableMap_) { - allVariables = allVariables.concat(this.variableMap_[key]); + let allVariables: VariableModel[] = []; + for (const variables of this.variableMap.values()) { + allVariables = allVariables.concat(variables); } return allVariables; } @@ -370,14 +363,9 @@ export class VariableMap { * @return All of the variable names of all types. */ getAllVariableNames(): string[] { - const allNames = []; - for (const key in this.variableMap_) { - const variables = this.variableMap_[key]; - for (let i = 0, variable; variable = variables[i]; i++) { - allNames.push(variable.name); - } - } - return allNames; + return Array.from(this.variableMap.values()) + .flat() + .map(variable => variable.name); } /** diff --git a/core/variables.ts b/core/variables.ts index cd15ccc9f..a2df3bcd4 100644 --- a/core/variables.ts +++ b/core/variables.ts @@ -46,8 +46,8 @@ export const CATEGORY_NAME = 'VARIABLE'; */ export function allUsedVarModels(ws: Workspace): VariableModel[] { const blocks = ws.getAllBlocks(false); - const variableHash = Object.create(null); - // Iterate through every block and add each variable to the hash. + const variables = new Set(); + // Iterate through every block and add each variable to the set. for (let i = 0; i < blocks.length; i++) { const blockVariables = blocks[i].getVarModels(); if (blockVariables) { @@ -55,17 +55,13 @@ export function allUsedVarModels(ws: Workspace): VariableModel[] { const variable = blockVariables[j]; const id = variable.getId(); if (id) { - variableHash[id] = variable; + variables.add(variable); } } } } - // Flatten the hash into a list. - const variableList = []; - for (const id in variableHash) { - variableList.push(variableHash[id]); - } - return variableList; + // Convert the set into a list. + return Array.from(variables.values()); } const ALL_DEVELOPER_VARS_WARNINGS_BY_BLOCK_TYPE: {[key: string]: boolean} = {}; @@ -83,7 +79,7 @@ const ALL_DEVELOPER_VARS_WARNINGS_BY_BLOCK_TYPE: {[key: string]: boolean} = {}; */ export function allDeveloperVariables(workspace: Workspace): string[] { const blocks = workspace.getAllBlocks(false); - const variableHash = Object.create(null); + const variables = new Set(); for (let i = 0, block; block = blocks[i]; i++) { let getDeveloperVariables = block.getDeveloperVariables; if (!getDeveloperVariables && @@ -101,12 +97,12 @@ export function allDeveloperVariables(workspace: Workspace): string[] { if (getDeveloperVariables) { const devVars = getDeveloperVariables(); for (let j = 0; j < devVars.length; j++) { - variableHash[devVars[j]] = true; + variables.add(devVars[j]); } } } - // Flatten the hash into a list. - return Object.keys(variableHash); + // Convert the set into a list. + return Array.from(variables.values()); } /** diff --git a/core/workspace.ts b/core/workspace.ts index fca1ce771..ca614dcb5 100644 --- a/core/workspace.ts +++ b/core/workspace.ts @@ -102,12 +102,12 @@ export class Workspace implements IASTNodeLocation { private readonly topBlocks_: Block[] = []; private readonly topComments_: WorkspaceComment[] = []; - private readonly commentDB_: AnyDuringMigration; + private readonly commentDB = new Map(); private readonly listeners_: Function[] = []; protected undoStack_: Abstract[] = []; protected redoStack_: Abstract[] = []; - private readonly blockDB_: AnyDuringMigration; - private readonly typedBlocksDB_: AnyDuringMigration; + private readonly blockDB = new Map(); + private readonly typedBlocksDB = new Map(); private variableMap_: VariableMap; /** @@ -135,9 +135,6 @@ export class Workspace implements IASTNodeLocation { * An object that encapsulates logic for safety, type, and dragging checks. */ this.connectionChecker = new connectionCheckerClass!(this); - this.commentDB_ = Object.create(null); - this.blockDB_ = Object.create(null); - this.typedBlocksDB_ = Object.create(null); /** * A map from variable type to list of variable names. The lists contain @@ -235,10 +232,10 @@ export class Workspace implements IASTNodeLocation { * @param block Block to add. */ addTypedBlock(block: Block) { - if (!this.typedBlocksDB_[block.type]) { - this.typedBlocksDB_[block.type] = []; + if (!this.typedBlocksDB.has(block.type)) { + this.typedBlocksDB.set(block.type, []); } - this.typedBlocksDB_[block.type].push(block); + this.typedBlocksDB.get(block.type)!.push(block); } /** @@ -246,9 +243,9 @@ export class Workspace implements IASTNodeLocation { * @param block Block to remove. */ removeTypedBlock(block: Block) { - arrayUtils.removeElem(this.typedBlocksDB_[block.type], block); - if (!this.typedBlocksDB_[block.type].length) { - delete this.typedBlocksDB_[block.type]; + arrayUtils.removeElem(this.typedBlocksDB.get(block.type)!, block); + if (!this.typedBlocksDB.get(block.type)!.length) { + this.typedBlocksDB.delete(block.type); } } @@ -260,11 +257,11 @@ export class Workspace implements IASTNodeLocation { * @return The blocks of the given type. */ getBlocksByType(type: string, ordered: boolean): Block[] { - if (!this.typedBlocksDB_[type]) { + if (!this.typedBlocksDB.has(type)) { return []; } - const blocks = this.typedBlocksDB_[type].slice(0); - if (ordered && blocks.length > 1) { + const blocks = this.typedBlocksDB.get(type)!.slice(0); + if (ordered && blocks && blocks.length > 1) { // AnyDuringMigration because: Property 'offset' does not exist on type // '(a: Block | WorkspaceComment, b: Block | WorkspaceComment) => number'. (this.sortObjects_ as AnyDuringMigration).offset = @@ -293,12 +290,12 @@ export class Workspace implements IASTNodeLocation { // Note: If the comment database starts to hold block comments, this may // need to move to a separate function. - if (this.commentDB_[comment.id]) { + if (this.commentDB.has(comment.id)) { console.warn( 'Overriding an existing comment on this workspace, with id "' + comment.id + '"'); } - this.commentDB_[comment.id] = comment; + this.commentDB.set(comment.id, comment); } /** @@ -314,7 +311,7 @@ export class Workspace implements IASTNodeLocation { } // Note: If the comment database starts to hold block comments, this may // need to move to a separate function. - delete this.commentDB_[comment.id]; + this.commentDB.delete(comment.id); } /** @@ -700,7 +697,7 @@ export class Workspace implements IASTNodeLocation { * @return The sought after block, or null if not found. */ getBlockById(id: string): Block|null { - return this.blockDB_[id] || null; + return this.blockDB.get(id) || null; } /** @@ -710,7 +707,7 @@ export class Workspace implements IASTNodeLocation { * @internal */ setBlockById(id: string, block: Block) { - this.blockDB_[id] = block; + this.blockDB.set(id, block); } /** @@ -719,7 +716,7 @@ export class Workspace implements IASTNodeLocation { * @internal */ removeBlockById(id: string) { - delete this.blockDB_[id]; + this.blockDB.delete(id); } /** @@ -729,7 +726,7 @@ export class Workspace implements IASTNodeLocation { * @internal */ getCommentById(id: string): WorkspaceComment|null { - return this.commentDB_[id] || null; + return this.commentDB.get(id) ?? null; } /** diff --git a/core/workspace_audio.ts b/core/workspace_audio.ts index 8c9147b86..6fe43e3d9 100644 --- a/core/workspace_audio.ts +++ b/core/workspace_audio.ts @@ -32,7 +32,8 @@ const SOUND_LIMIT = 100; * @alias Blockly.WorkspaceAudio */ export class WorkspaceAudio { - private SOUNDS_: AnyDuringMigration; + /** Database of pre-loaded sounds. */ + private sounds = new Map(); /** Time that the last sound was played. */ // AnyDuringMigration because: Type 'null' is not assignable to type 'Date'. @@ -42,10 +43,7 @@ export class WorkspaceAudio { * @param parentWorkspace The parent of the workspace this audio object * belongs to, or null. */ - constructor(private parentWorkspace: WorkspaceSvg) { - /** Database of pre-loaded sounds. */ - this.SOUNDS_ = Object.create(null); - } + constructor(private parentWorkspace: WorkspaceSvg) {} /** * Dispose of this audio manager. @@ -55,7 +53,7 @@ export class WorkspaceAudio { // AnyDuringMigration because: Type 'null' is not assignable to type // 'WorkspaceSvg'. this.parentWorkspace = null as AnyDuringMigration; - this.SOUNDS_ = null; + this.sounds.clear(); } /** @@ -88,7 +86,7 @@ export class WorkspaceAudio { } } if (sound) { - this.SOUNDS_[name] = sound; + this.sounds.set(name, sound); } } @@ -97,8 +95,7 @@ export class WorkspaceAudio { * @internal */ preload() { - for (const name in this.SOUNDS_) { - const sound = this.SOUNDS_[name]; + for (const sound of this.sounds.values()) { sound.volume = 0.01; const playPromise = sound.play(); // Edge does not return a promise, so we need to check. @@ -131,7 +128,7 @@ export class WorkspaceAudio { * @param opt_volume Volume of sound (0-1). */ play(name: string, opt_volume?: number) { - const sound = this.SOUNDS_[name]; + const sound = this.sounds.get(name); if (sound) { // Don't play one sound on top of another. const now = new Date(); @@ -147,7 +144,7 @@ export class WorkspaceAudio { // node which must be deleted and recreated for each new audio tag. mySound = sound; } else { - mySound = sound.cloneNode(); + mySound = sound.cloneNode() as HTMLAudioElement; } mySound.volume = opt_volume === undefined ? 1 : opt_volume; mySound.play(); diff --git a/tests/mocha/shortcut_registry_test.js b/tests/mocha/shortcut_registry_test.js index c41fa1910..dea01e22a 100644 --- a/tests/mocha/shortcut_registry_test.js +++ b/tests/mocha/shortcut_registry_test.js @@ -25,14 +25,14 @@ suite('Keyboard Shortcut Registry Test', function() { test('Registering a shortcut', function() { const testShortcut = {'name': 'test_shortcut'}; this.registry.register(testShortcut, true); - const shortcut = this.registry.registry_['test_shortcut']; + const shortcut = this.registry.getRegistry()['test_shortcut']; chai.assert.equal(shortcut.name, 'test_shortcut'); }); test('Registers shortcut with same name', function() { const registry = this.registry; const testShortcut = {'name': 'test_shortcut'}; - registry.registry_['test_shortcut'] = [testShortcut]; + registry.register(testShortcut); const shouldThrow = function() { registry.register(testShortcut); @@ -51,13 +51,13 @@ suite('Keyboard Shortcut Registry Test', function() { 'callback': function() {}, }; - registry.registry_['test_shortcut'] = [testShortcut]; + registry.register(testShortcut); const shouldNotThrow = function() { registry.register(otherShortcut, true); }; chai.assert.doesNotThrow(shouldNotThrow); - chai.assert.exists(registry.registry_['test_shortcut'].callback); + chai.assert.exists(registry.getRegistry()['test_shortcut'].callback); }); test('Registering a shortcut with keycodes', function() { const shiftA = this.registry.createSerializedKey( @@ -67,9 +67,9 @@ suite('Keyboard Shortcut Registry Test', function() { 'keyCodes': ['65', 66, shiftA], }; this.registry.register(testShortcut, true); - chai.assert.lengthOf(this.registry.keyMap_[shiftA], 1); - chai.assert.lengthOf(this.registry.keyMap_['65'], 1); - chai.assert.lengthOf(this.registry.keyMap_['66'], 1); + chai.assert.lengthOf(this.registry.getKeyMap()[shiftA], 1); + chai.assert.lengthOf(this.registry.getKeyMap()['65'], 1); + chai.assert.lengthOf(this.registry.getKeyMap()['66'], 1); }); test('Registering a shortcut with allowCollision', function() { const testShortcut = { @@ -93,40 +93,43 @@ suite('Keyboard Shortcut Registry Test', function() { suite('Unregistering', function() { test('Unregistering a shortcut', function() { const testShortcut = {'name': 'test_shortcut'}; - this.registry.registry_['test'] = [testShortcut]; - chai.assert.isOk(this.registry.registry_['test']); - this.registry.unregister('test', 'test_shortcut'); - chai.assert.isUndefined(this.registry.registry_['test']); + this.registry.register(testShortcut); + chai.assert.isOk(this.registry.getRegistry()['test_shortcut']); + this.registry.unregister('test_shortcut'); + chai.assert.isUndefined(this.registry.getRegistry()['test_shortcut']); }); test('Unregistering a nonexistent shortcut', function() { const consoleStub = sinon.stub(console, 'warn'); - chai.assert.isUndefined(this.registry.registry_['test']); + chai.assert.isUndefined(this.registry.getRegistry['test']); const registry = this.registry; - chai.assert.isFalse(registry.unregister('test', 'test_shortcut')); + chai.assert.isFalse(registry.unregister('test')); sinon.assert.calledOnceWithExactly(consoleStub, 'Keyboard shortcut with name "test" not found.'); }); test('Unregistering a shortcut with key mappings', function() { const testShortcut = {'name': 'test_shortcut'}; - this.registry.keyMap_['keyCode'] = ['test_shortcut']; - this.registry.registry_['test_shortcut'] = testShortcut; + this.registry.register(testShortcut); + this.registry.addKeyMapping('keyCode', 'test_shortcut'); this.registry.unregister('test_shortcut'); - const shortcut = this.registry.registry_['test']; - const keyMappings = this.registry.keyMap_['keyCode']; + const shortcut = this.registry.getRegistry()['test_shortcut']; + const keyMappings = this.registry.getKeyMap()['keyCode']; chai.assert.isUndefined(shortcut); chai.assert.isUndefined(keyMappings); }); test('Unregistering a shortcut with colliding key mappings', function() { const testShortcut = {'name': 'test_shortcut'}; - this.registry.keyMap_['keyCode'] = ['test_shortcut', 'other_shortcutt']; - this.registry.registry_['test_shortcut'] = testShortcut; + const otherShortcut = {'name': 'other_shortcut'}; + this.registry.register(testShortcut); + this.registry.register(otherShortcut); + this.registry.addKeyMapping('keyCode', 'test_shortcut'); + this.registry.addKeyMapping('keyCode', 'other_shortcut', true); this.registry.unregister('test_shortcut'); - const shortcut = this.registry.registry_['test']; - const keyMappings = this.registry.keyMap_['keyCode']; + const shortcut = this.registry.getRegistry()['test_shortcut']; + const keyMappings = this.registry.getKeyMap()['keyCode']; chai.assert.lengthOf(keyMappings, 1); chai.assert.isUndefined(shortcut); }); @@ -134,28 +137,35 @@ suite('Keyboard Shortcut Registry Test', function() { suite('addKeyMapping', function() { test('Adds a key mapping', function() { - this.registry.registry_['test_shortcut'] = {'name': 'test_shortcut'}; + const testShortcut = {'name': 'test_shortcut'}; + this.registry.register(testShortcut); this.registry.addKeyMapping('keyCode', 'test_shortcut'); - const shortcutNames = this.registry.keyMap_['keyCode']; + const shortcutNames = this.registry.getKeyMap()['keyCode']; chai.assert.lengthOf(shortcutNames, 1); chai.assert.equal(shortcutNames[0], 'test_shortcut'); }); test('Adds a colliding key mapping - opt_allowCollision=true', function() { - this.registry.registry_['test_shortcut'] = {'name': 'test_shortcut'}; - this.registry.keyMap_['keyCode'] = ['test_shortcut_2']; + const testShortcut = {'name': 'test_shortcut'}; + const testShortcut2 = {'name': 'test_shortcut_2'}; + this.registry.register(testShortcut); + this.registry.register(testShortcut2); + this.registry.addKeyMapping('keyCode', 'test_shortcut_2'); this.registry.addKeyMapping('keyCode', 'test_shortcut', true); - const shortcutNames = this.registry.keyMap_['keyCode']; + const shortcutNames = this.registry.getKeyMap()['keyCode']; chai.assert.lengthOf(shortcutNames, 2); chai.assert.equal(shortcutNames[0], 'test_shortcut'); chai.assert.equal(shortcutNames[1], 'test_shortcut_2'); }); test('Adds a colliding key mapping - opt_allowCollision=false', function() { - this.registry.registry_['test_shortcut'] = {'name': 'test_shortcut'}; - this.registry.keyMap_['keyCode'] = ['test_shortcut_2']; + const testShortcut = {'name': 'test_shortcut'}; + const testShortcut2 = {'name': 'test_shortcut_2'}; + this.registry.register(testShortcut); + this.registry.register(testShortcut2); + this.registry.addKeyMapping('keyCode', 'test_shortcut_2'); const registry = this.registry; const shouldThrow = function() { @@ -169,29 +179,36 @@ suite('Keyboard Shortcut Registry Test', function() { suite('removeKeyMapping', function() { test('Removes a key mapping', function() { - this.registry.registry_['test_shortcut'] = {'name': 'test_shortcut'}; - this.registry.keyMap_['keyCode'] = ['test_shortcut', 'test_shortcut_2']; + const testShortcut = {'name': 'test_shortcut'}; + const testShortcut2 = {'name': 'test_shortcut_2'}; + this.registry.register(testShortcut); + this.registry.register(testShortcut2); + this.registry.addKeyMapping('keyCode', 'test_shortcut_2'); + this.registry.addKeyMapping('keyCode', 'test_shortcut', true); const isRemoved = this.registry.removeKeyMapping('keyCode', 'test_shortcut'); - const shortcutNames = this.registry.keyMap_['keyCode']; + const shortcutNames = this.registry.getKeyMap()['keyCode']; chai.assert.lengthOf(shortcutNames, 1); chai.assert.equal(shortcutNames[0], 'test_shortcut_2'); chai.assert.isTrue(isRemoved); }); test('Removes last key mapping for a key', function() { - this.registry.registry_['test_shortcut'] = {'name': 'test_shortcut'}; - this.registry.keyMap_['keyCode'] = ['test_shortcut']; + const testShortcut = {'name': 'test_shortcut'}; + this.registry.register(testShortcut); + this.registry.addKeyMapping('keyCode', 'test_shortcut'); this.registry.removeKeyMapping('keyCode', 'test_shortcut'); - const shortcutNames = this.registry.keyMap_['keyCode']; + const shortcutNames = this.registry.getKeyMap()['keyCode']; chai.assert.isUndefined(shortcutNames); }); test('Removes a key map that does not exist opt_quiet=false', function() { const consoleStub = sinon.stub(console, 'warn'); - this.registry.keyMap_['keyCode'] = ['test_shortcut_2']; + const testShortcut = {'name': 'test_shortcut_2'}; + this.registry.register(testShortcut); + this.registry.addKeyMapping('keyCode', 'test_shortcut_2'); const isRemoved = this.registry.removeKeyMapping('keyCode', 'test_shortcut'); @@ -219,30 +236,30 @@ suite('Keyboard Shortcut Registry Test', function() { suite('Setters/Getters', function() { test('Sets the key map', function() { this.registry.setKeyMap({'keyCode': ['test_shortcut']}); - chai.assert.lengthOf(Object.keys(this.registry.keyMap_), 1); - chai.assert.equal(this.registry.keyMap_['keyCode'][0], 'test_shortcut'); + chai.assert.equal(Object.keys(this.registry.getKeyMap()).length, 1); + chai.assert.equal(this.registry.getKeyMap()['keyCode'][0], 'test_shortcut'); }); test('Gets a copy of the key map', function() { - this.registry.keyMap_['keyCode'] = ['a']; + this.registry.setKeyMap({'keyCode': ['a']}); const keyMapCopy = this.registry.getKeyMap(); keyMapCopy['keyCode'] = ['b']; - chai.assert.equal(this.registry.keyMap_['keyCode'][0], 'a'); + chai.assert.equal(this.registry.getKeyMap()['keyCode'][0], 'a'); }); test('Gets a copy of the registry', function() { - this.registry.registry_['shortcutName'] = {'name': 'shortcutName'}; + const shortcut = {'name': 'shortcutName'}; + this.registry.register(shortcut); const registrycopy = this.registry.getRegistry(); registrycopy['shortcutName']['name'] = 'shortcutName1'; chai.assert.equal( - this.registry.registry_['shortcutName']['name'], 'shortcutName'); + this.registry.getRegistry()['shortcutName']['name'], 'shortcutName'); }); test('Gets keyboard shortcuts from a key code', function() { - this.registry.keyMap_['keyCode'] = ['shortcutName']; + this.registry.setKeyMap({'keyCode': ['shortcutName']}); const shortcutNames = this.registry.getShortcutNamesByKeyCode('keyCode'); chai.assert.equal(shortcutNames[0], 'shortcutName'); }); test('Gets keycodes by shortcut name', function() { - this.registry.keyMap_['keyCode'] = ['shortcutName']; - this.registry.keyMap_['keyCode1'] = ['shortcutName']; + this.registry.setKeyMap({'keyCode': ['shortcutName'], 'keyCode1': ['shortcutName']}); const shortcutNames = this.registry.getKeyCodesByShortcutName('shortcutName'); chai.assert.lengthOf(shortcutNames, 2); diff --git a/tests/mocha/test_helpers/workspace.js b/tests/mocha/test_helpers/workspace.js index 0500a29da..a2f973107 100644 --- a/tests/mocha/test_helpers/workspace.js +++ b/tests/mocha/test_helpers/workspace.js @@ -69,8 +69,7 @@ export function testAWorkspace() { this.workspace.clear(); chai.assert.equal(this.workspace.topBlocks_.length, 0); - const varMapLength = - Object.keys(this.workspace.variableMap_.variableMap_).length; + const varMapLength = this.workspace.variableMap_.variableMap.size; chai.assert.equal(varMapLength, 0); }); @@ -80,8 +79,7 @@ export function testAWorkspace() { this.workspace.clear(); chai.assert.equal(this.workspace.topBlocks_.length, 0); - const varMapLength = - Object.keys(this.workspace.variableMap_.variableMap_).length; + const varMapLength = this.workspace.variableMap_.variableMap.size; chai.assert.equal(varMapLength, 0); }); }); @@ -840,7 +838,7 @@ export function testAWorkspace() { ' ' + ''; - testUndoConnect.call(this, xml, 1, 2, (parent, child) => { + testUndoConnect.call(this, xml, '1', '2', (parent, child) => { parent.nextConnection.connect(child.previousConnection); }); }); @@ -852,7 +850,7 @@ export function testAWorkspace() { ' ' + ''; - testUndoConnect.call(this, xml, 1, 2, (parent, child) => { + testUndoConnect.call(this, xml, '1', '2', (parent, child) => { parent.getInput('INPUT').connection.connect(child.outputConnection); }); }); @@ -864,7 +862,7 @@ export function testAWorkspace() { ' ' + ''; - testUndoConnect.call(this, xml, 1, 2, (parent, child) => { + testUndoConnect.call(this, xml, '1', '2', (parent, child) => { parent.getInput('STATEMENT').connection .connect(child.previousConnection); }); @@ -881,7 +879,7 @@ export function testAWorkspace() { ' ' + ''; - testUndoConnect.call(this, xml, 1, 2, (parent, child) => { + testUndoConnect.call(this, xml, '1', '2', (parent, child) => { parent.nextConnection.connect(child.previousConnection); }); }); @@ -897,7 +895,7 @@ export function testAWorkspace() { ' ' + ''; - testUndoConnect.call(this, xml, 1, 2, (parent, child) => { + testUndoConnect.call(this, xml, '1', '2', (parent, child) => { parent.getInput('INPUT').connection.connect(child.outputConnection); }); }); @@ -913,7 +911,7 @@ export function testAWorkspace() { ' ' + ''; - testUndoConnect.call(this, xml, 1, 2, (parent, child) => { + testUndoConnect.call(this, xml, '1', '2', (parent, child) => { parent.getInput('STATEMENT').connection .connect(child.previousConnection); }); @@ -930,7 +928,7 @@ export function testAWorkspace() { ' ' + ''; - testUndoConnect.call(this, xml, 1, 2, (parent, child) => { + testUndoConnect.call(this, xml, '1', '2', (parent, child) => { parent.nextConnection.connect(child.previousConnection); }); }); @@ -946,7 +944,7 @@ export function testAWorkspace() { ' ' + ''; - testUndoConnect.call(this, xml, 1, 2, (parent, child) => { + testUndoConnect.call(this, xml, '1', '2', (parent, child) => { parent.getInput('INPUT').connection.connect(child.outputConnection); }); }); @@ -962,7 +960,7 @@ export function testAWorkspace() { ' ' + ''; - testUndoConnect.call(this, xml, 1, 2, (parent, child) => { + testUndoConnect.call(this, xml, '1', '2', (parent, child) => { parent.getInput('STATEMENT').connection .connect(child.previousConnection); }); @@ -1034,7 +1032,7 @@ export function testAWorkspace() { ' ' + ' ' + ''; - testUndoDisconnect.call(this, xml, 2); + testUndoDisconnect.call(this, xml, '2'); }); test('Row', function() { @@ -1046,7 +1044,7 @@ export function testAWorkspace() { ' ' + ' ' + ''; - testUndoDisconnect.call(this, xml, 2); + testUndoDisconnect.call(this, xml, '2'); }); test('Statement', function() { @@ -1058,7 +1056,7 @@ export function testAWorkspace() { ' ' + ' ' + ''; - testUndoDisconnect.call(this, xml, 2); + testUndoDisconnect.call(this, xml, '2'); }); test('Stack w/ child', function() { @@ -1074,7 +1072,7 @@ export function testAWorkspace() { ' ' + ' ' + ''; - testUndoDisconnect.call(this, xml, 2); + testUndoDisconnect.call(this, xml, '2'); }); test('Row w/ child', function() { @@ -1090,7 +1088,7 @@ export function testAWorkspace() { ' ' + ' ' + ''; - testUndoDisconnect.call(this, xml, 2); + testUndoDisconnect.call(this, xml, '2'); }); test('Statement w/ child', function() { @@ -1106,7 +1104,7 @@ export function testAWorkspace() { ' ' + ' ' + ''; - testUndoDisconnect.call(this, xml, 2); + testUndoDisconnect.call(this, xml, '2'); }); test('Stack w/ shadow', function() { @@ -1121,7 +1119,7 @@ export function testAWorkspace() { ' ' + ' ' + ''; - testUndoDisconnect.call(this, xml, 2); + testUndoDisconnect.call(this, xml, '2'); chai.assert.equal(this.workspace.getAllBlocks().length, 2, 'expected there to only be 2 blocks on the workspace ' + '(check for shadows)'); @@ -1137,7 +1135,7 @@ export function testAWorkspace() { ' ' + ' ' + ''; - testUndoDisconnect.call(this, xml, 2); + testUndoDisconnect.call(this, xml, '2'); chai.assert.equal(this.workspace.getAllBlocks().length, 2, 'expected there to only be 2 blocks on the workspace ' + '(check for shadows)'); @@ -1153,7 +1151,7 @@ export function testAWorkspace() { ' ' + ' ' + ''; - testUndoDisconnect.call(this, xml, 2); + testUndoDisconnect.call(this, xml, '2'); }); }); diff --git a/tests/mocha/variable_map_test.js b/tests/mocha/variable_map_test.js index c6a4701fc..a8bec4a1f 100644 --- a/tests/mocha/variable_map_test.js +++ b/tests/mocha/variable_map_test.js @@ -28,21 +28,21 @@ suite('Variable Map', function() { }); test('Already exists', function() { - // Expect that when the variable already exists, the variableMap_ is unchanged. + // Expect that when the variable already exists, the variableMap is unchanged. this.variableMap.createVariable('name1', 'type1', 'id1'); // Assert there is only one variable in the this.variableMap. - let keys = Object.keys(this.variableMap.variableMap_); + let keys = Array.from(this.variableMap.variableMap.keys()); chai.assert.equal(keys.length, 1); - let varMapLength = this.variableMap.variableMap_[keys[0]].length; + let varMapLength = this.variableMap.variableMap.get(keys[0]).length; chai.assert.equal(varMapLength, 1); this.variableMap.createVariable('name1', 'type1'); assertVariableValues(this.variableMap, 'name1', 'type1', 'id1'); - // Check that the size of the variableMap_ did not change. - keys = Object.keys(this.variableMap.variableMap_); + // Check that the size of the variableMap did not change. + keys = Array.from(this.variableMap.variableMap.keys()); chai.assert.equal(keys.length, 1); - varMapLength = this.variableMap.variableMap_[keys[0]].length; + varMapLength = this.variableMap.variableMap.get(keys[0]).length; chai.assert.equal(varMapLength, 1); }); @@ -52,16 +52,16 @@ suite('Variable Map', function() { this.variableMap.createVariable('name1', 'type1', 'id1'); // Assert there is only one variable in the this.variableMap. - let keys = Object.keys(this.variableMap.variableMap_); + let keys = Array.from(this.variableMap.variableMap.keys()); chai.assert.equal(keys.length, 1); - const varMapLength = this.variableMap.variableMap_[keys[0]].length; + const varMapLength = this.variableMap.variableMap.get(keys[0]).length; chai.assert.equal(varMapLength, 1); this.variableMap.createVariable('name1', 'type2', 'id2'); assertVariableValues(this.variableMap, 'name1', 'type1', 'id1'); assertVariableValues(this.variableMap, 'name1', 'type2', 'id2'); - // Check that the size of the variableMap_ did change. - keys = Object.keys(this.variableMap.variableMap_); + // Check that the size of the variableMap did change. + keys = Array.from(this.variableMap.variableMap.keys()); chai.assert.equal(keys.length, 2); }); diff --git a/tests/mocha/workspace_comment_test.js b/tests/mocha/workspace_comment_test.js index e4ec13f58..0ea07b88b 100644 --- a/tests/mocha/workspace_comment_test.js +++ b/tests/mocha/workspace_comment_test.js @@ -29,7 +29,7 @@ suite('Workspace comment', function() { const comment = new Blockly.WorkspaceComment( this.workspace, 'comment text', 0, 0, 'comment id'); chai.assert.equal(this.workspace.getTopComments(true).length, 1); - chai.assert.equal(this.workspace.commentDB_['comment id'], comment); + chai.assert.equal(this.workspace.commentDB.get('comment id'), comment); }); test('After clear empty workspace', function() { @@ -42,7 +42,7 @@ suite('Workspace comment', function() { this.workspace, 'comment text', 0, 0, 'comment id'); this.workspace.clear(); chai.assert.equal(this.workspace.getTopComments(true).length, 0); - chai.assert.isFalse('comment id' in this.workspace.commentDB_); + chai.assert.isFalse(this.workspace.commentDB.has('comment id')); }); test('After dispose', function() { @@ -50,7 +50,7 @@ suite('Workspace comment', function() { this.workspace, 'comment text', 0, 0, 'comment id'); comment.dispose(); chai.assert.equal(this.workspace.getTopComments(true).length, 0); - chai.assert.isFalse('comment id' in this.workspace.commentDB_); + chai.assert.isFalse(this.workspace.commentDB.has('comment id')); }); }); @@ -63,7 +63,7 @@ suite('Workspace comment', function() { const comment = new Blockly.WorkspaceComment( this.workspace, 'comment text', 0, 0, 'comment id'); chai.assert.equal(this.workspace.getTopComments(false).length, 1); - chai.assert.equal(this.workspace.commentDB_['comment id'], comment); + chai.assert.equal(this.workspace.commentDB.get('comment id'), comment); }); test('After clear empty workspace', function() { @@ -76,7 +76,7 @@ suite('Workspace comment', function() { this.workspace, 'comment text', 0, 0, 'comment id'); this.workspace.clear(); chai.assert.equal(this.workspace.getTopComments(false).length, 0); - chai.assert.isFalse('comment id' in this.workspace.commentDB_); + chai.assert.isFalse(this.workspace.commentDB.has('comment id')); }); test('After dispose', function() { @@ -84,7 +84,7 @@ suite('Workspace comment', function() { this.workspace, 'comment text', 0, 0, 'comment id'); comment.dispose(); chai.assert.equal(this.workspace.getTopComments(false).length, 0); - chai.assert.isFalse('comment id' in this.workspace.commentDB_); + chai.assert.isFalse(this.workspace.commentDB.has('comment id')); }); });