From 8a44dff4e82fdf74d8f0ba0ccb628e8ff8e9c4ff Mon Sep 17 00:00:00 2001 From: Aaron Dodson Date: Tue, 28 Feb 2023 14:38:17 -0800 Subject: [PATCH] refactor: Remove more uses of AnyDuringMigration (#6863) * refactor: Remove uses of AnyDuringMigration from xml.ts. * refactor: Remove uses of AnyDuringMigration from block_svg.ts. * refactor: Remove uses of AnyDuringMigration from theme_manager.ts. * refactor: Remove uses of AnyDuringMigration from names.ts. * refactor: Remove uses of AnyDuringMigration from field_angle.ts. * refactor: Remove some uses of AnyDuringMigration from block.ts. * refactor: Make safename construction more readable. * fix: Use a default size for block comments. * fix: Clarify test of shadow block disposal. * fix: Update assert in workspace_svg_test to use isDeadOrDying(). --- core/block.ts | 72 +++++++++-------------- core/block_svg.ts | 48 ++-------------- core/connection.ts | 7 ++- core/events/events_block_move.ts | 4 +- core/field_angle.ts | 20 ++----- core/inject.ts | 4 +- core/keyboard_nav/ast_node.ts | 4 +- core/names.ts | 12 ++-- core/theme_manager.ts | 25 +++----- core/xml.ts | 96 +++++++++++-------------------- tests/mocha/workspace_svg_test.js | 8 +-- 11 files changed, 98 insertions(+), 202 deletions(-) diff --git a/core/block.ts b/core/block.ts index 34c862119..40cf48117 100644 --- a/core/block.ts +++ b/core/block.ts @@ -57,7 +57,7 @@ export class Block implements IASTNodeLocation, IDeletable { * changes. This is usually only called from the constructor, the block type * initializer function, or an extension initializer function. */ - onchange?: ((p1: Abstract) => AnyDuringMigration)|null; + onchange?: ((p1: Abstract) => void)|null; /** The language-neutral ID given to the collapsed input. */ static readonly COLLAPSED_INPUT_NAME: string = constants.COLLAPSED_INPUT_NAME; @@ -91,39 +91,38 @@ export class Block implements IASTNodeLocation, IDeletable { protected styleName_ = ''; /** An optional method called during initialization. */ - init?: (() => AnyDuringMigration)|null = undefined; + init?: (() => void); /** An optional method called during disposal. */ - destroy?: (() => void) = undefined; + destroy?: (() => void); /** * An optional serialization method for defining how to serialize the * mutation state to XML. This must be coupled with defining * `domToMutation`. */ - mutationToDom?: ((...p1: AnyDuringMigration[]) => Element)|null = undefined; + mutationToDom?: (...p1: AnyDuringMigration[]) => Element; /** * An optional deserialization method for defining how to deserialize the * mutation state from XML. This must be coupled with defining * `mutationToDom`. */ - domToMutation?: ((p1: Element) => AnyDuringMigration)|null = undefined; + domToMutation?: (p1: Element) => void; /** * An optional serialization method for defining how to serialize the * block's extra state (eg mutation state) to something JSON compatible. * This must be coupled with defining `loadExtraState`. */ - saveExtraState?: (() => AnyDuringMigration)|null = undefined; + saveExtraState?: () => AnyDuringMigration; /** * An optional serialization method for defining how to deserialize the * block's extra state (eg mutation state) from something JSON compatible. * This must be coupled with defining `saveExtraState`. */ - loadExtraState?: - ((p1: AnyDuringMigration) => AnyDuringMigration)|null = undefined; + loadExtraState?: (p1: AnyDuringMigration) => void; /** * An optional property for suppressing adding STATEMENT_PREFIX and @@ -137,31 +136,25 @@ export class Block implements IASTNodeLocation, IDeletable { * shown to the user, but are declared as global variables in the generated * code. */ - getDeveloperVariables?: (() => string[]) = undefined; + getDeveloperVariables?: () => string[]; /** * An optional function that reconfigures the block based on the contents of * the mutator dialog. */ - compose?: ((p1: Block) => void) = undefined; + compose?: (p1: Block) => void; /** * An optional function that populates the mutator's dialog with * this block's components. */ - decompose?: ((p1: Workspace) => Block) = undefined; + decompose?: (p1: Workspace) => Block; id: string; - // AnyDuringMigration because: Type 'null' is not assignable to type - // 'Connection'. - outputConnection: Connection = null as AnyDuringMigration; - // AnyDuringMigration because: Type 'null' is not assignable to type - // 'Connection'. - nextConnection: Connection = null as AnyDuringMigration; - // AnyDuringMigration because: Type 'null' is not assignable to type - // 'Connection'. - previousConnection: Connection = null as AnyDuringMigration; + outputConnection: Connection|null = null; + nextConnection: Connection|null = null; + previousConnection: Connection|null = null; inputList: Input[] = []; - inputsInline?: boolean = undefined; + inputsInline?: boolean; private disabled = false; tooltip: Tooltip.TipInfo = ''; contextMenu = true; @@ -203,19 +196,17 @@ export class Block implements IASTNodeLocation, IDeletable { protected isInsertionMarker_ = false; /** Name of the type of hat. */ - hat?: string = undefined; + hat?: string; rendered: boolean|null = null; /** * String for block help, or function that returns a URL. Null for no help. */ - // AnyDuringMigration because: Type 'null' is not assignable to type 'string - // | Function'. - helpUrl: string|Function = null as AnyDuringMigration; + helpUrl: string|Function|null = null; /** A bound callback function to use when the parent workspace changes. */ - private onchangeWrapper_: ((p1: Abstract) => AnyDuringMigration)|null = null; + private onchangeWrapper_: ((p1: Abstract) => void)|null = null; /** * A count of statement inputs on the block. @@ -422,7 +413,7 @@ export class Block implements IASTNodeLocation, IDeletable { */ private unplugFromRow_(opt_healStack?: boolean) { let parentConnection = null; - if (this.outputConnection.isConnected()) { + if (this.outputConnection?.isConnected()) { parentConnection = this.outputConnection.targetConnection; // Disconnect from any superior block. this.outputConnection.disconnect(); @@ -487,7 +478,7 @@ export class Block implements IASTNodeLocation, IDeletable { */ private unplugFromStack_(opt_healStack?: boolean) { let previousTarget = null; - if (this.previousConnection.isConnected()) { + if (this.previousConnection?.isConnected()) { // Remember the connection that any next statements need to connect to. previousTarget = this.previousConnection.targetConnection; // Detach this block from the parent's tree. @@ -496,7 +487,7 @@ export class Block implements IASTNodeLocation, IDeletable { const nextBlock = this.getNextBlock(); if (opt_healStack && nextBlock && !nextBlock.isShadow()) { // Disconnect the next statement. - const nextTarget = this.nextConnection.targetConnection; + const nextTarget = this.nextConnection?.targetConnection ?? null; nextTarget?.disconnect(); if (previousTarget && this.workspace.connectionChecker.canConnect( @@ -1052,7 +1043,7 @@ export class Block implements IASTNodeLocation, IDeletable { * @param onchangeFn The callback to call when the block's workspace changes. * @throws {Error} if onchangeFn is not falsey and not a function. */ - setOnChange(onchangeFn: (p1: Abstract) => AnyDuringMigration) { + setOnChange(onchangeFn: (p1: Abstract) => void) { if (onchangeFn && typeof onchangeFn !== 'function') { throw Error('onchange must be a function.'); } @@ -1219,9 +1210,7 @@ export class Block implements IASTNodeLocation, IDeletable { 'connection.'); } this.previousConnection.dispose(); - // AnyDuringMigration because: Type 'null' is not assignable to type - // 'Connection'. - this.previousConnection = null as AnyDuringMigration; + this.previousConnection = null; } } } @@ -1251,9 +1240,7 @@ export class Block implements IASTNodeLocation, IDeletable { 'connection.'); } this.nextConnection.dispose(); - // AnyDuringMigration because: Type 'null' is not assignable to type - // 'Connection'. - this.nextConnection = null as AnyDuringMigration; + this.nextConnection = null; } } } @@ -1282,9 +1269,7 @@ export class Block implements IASTNodeLocation, IDeletable { 'Must disconnect output value before removing connection.'); } this.outputConnection.dispose(); - // AnyDuringMigration because: Type 'null' is not assignable to type - // 'Connection'. - this.outputConnection = null as AnyDuringMigration; + this.outputConnection = null; } } } @@ -1962,10 +1947,7 @@ export class Block implements IASTNodeLocation, IDeletable { if (type === inputTypes.STATEMENT) { this.statementInputCount++; } - // AnyDuringMigration because: Argument of type 'Connection | null' is not - // assignable to parameter of type 'Connection'. - const input = - new Input(type, name, this, (connection as AnyDuringMigration)); + const input = new Input(type, name, this, connection); // Append input to list. this.inputList.push(input); return input; @@ -2109,9 +2091,7 @@ export class Block implements IASTNodeLocation, IDeletable { eventUtils.fire(new (eventUtils.get(eventUtils.BLOCK_CHANGE))( this, 'comment', null, this.commentModel.text, text)); this.commentModel.text = text; - // AnyDuringMigration because: Type 'string | null' is not assignable to - // type 'string | Comment'. - this.comment = text as AnyDuringMigration; // For backwards compatibility. + this.comment = text; // For backwards compatibility. } /** diff --git a/core/block_svg.ts b/core/block_svg.ts index 6d84c6d3f..344b34399 100644 --- a/core/block_svg.ts +++ b/core/block_svg.ts @@ -77,12 +77,11 @@ export class BlockSvg extends Block implements IASTNodeLocationSvg, * the block. */ static readonly COLLAPSED_WARNING_ID = 'TEMP_COLLAPSED_WARNING_'; - override decompose?: ((p1: Workspace) => BlockSvg); + override decompose?: (p1: Workspace) => BlockSvg; // override compose?: ((p1: BlockSvg) => void)|null; - saveConnections?: ((p1: BlockSvg) => AnyDuringMigration); + saveConnections?: (p1: BlockSvg) => void; customContextMenu?: - ((p1: Array) => - AnyDuringMigration)|null; + (p1: Array) => void; /** * An property used internally to reference the block's rendering debugger. @@ -153,34 +152,6 @@ export class BlockSvg extends Block implements IASTNodeLocationSvg, constructor(workspace: WorkspaceSvg, prototypeName: string, opt_id?: string) { super(workspace, prototypeName, opt_id); this.workspace = workspace; - - /** - * An optional method called when a mutator dialog is first opened. - * This function must create and initialize a top-level block for the - * mutator dialog, and return it. This function should also populate this - * top-level block with any sub-blocks which are appropriate. This method - * must also be coupled with defining a `compose` method for the default - * mutation dialog button and UI to appear. - */ - this.decompose = this.decompose; - - /** - * An optional method called when a mutator dialog saves its content. - * This function is called to modify the original block according to new - * settings. This method must also be coupled with defining a `decompose` - * method for the default mutation dialog button and UI to appear. - */ - this.compose = this.compose; - - /** - * An optional method called by the default mutator UI which gives the block - * a chance to save information about what child blocks are connected to - * what mutated connections. - */ - this.saveConnections = this.saveConnections; - - /** An optional method for defining custom block context menu items. */ - this.customContextMenu = this.customContextMenu; this.svgGroup_ = dom.createSvgElement(Svg.G, {}); /** A block style object. */ @@ -191,7 +162,7 @@ export class BlockSvg extends Block implements IASTNodeLocationSvg, workspace.getRenderer().makePathObject(this.svgGroup_, this.style); const svgPath = this.pathObject.svgPath; - (svgPath as AnyDuringMigration).tooltip = this; + (svgPath as any).tooltip = this; Tooltip.bindMouseEvents(svgPath); // Expose this block's ID on its top-level SVG group. @@ -639,11 +610,8 @@ export class BlockSvg extends Block implements IASTNodeLocationSvg, if (this.workspace.options.readOnly || !this.contextMenu) { return null; } - // AnyDuringMigration because: Argument of type '{ block: this; }' is not - // assignable to parameter of type 'Scope'. const menuOptions = ContextMenuRegistry.registry.getContextMenuOptions( - ContextMenuRegistry.ScopeType.BLOCK, - {block: this} as AnyDuringMigration); + ContextMenuRegistry.ScopeType.BLOCK, {block: this}); // Allow the block to add or modify menuOptions. if (this.customContextMenu) { @@ -839,10 +807,6 @@ export class BlockSvg extends Block implements IASTNodeLocationSvg, dom.removeNode(this.svgGroup_); blockWorkspace.resizeContents(); - // Sever JavaScript to DOM connections. - // AnyDuringMigration because: Type 'null' is not assignable to type - // 'SVGGElement'. - this.svgGroup_ = null as AnyDuringMigration; dom.stopTextWidthCache(); } @@ -944,8 +908,6 @@ export class BlockSvg extends Block implements IASTNodeLocationSvg, * @param text The text, or null to delete. */ override setCommentText(text: string|null) { - // AnyDuringMigration because: Property 'get' does not exist on type - // '(name: string) => void'. if (this.commentModel.text === text) { return; } diff --git a/core/connection.ts b/core/connection.ts index f5dc0a6bb..76536aa8b 100644 --- a/core/connection.ts +++ b/core/connection.ts @@ -127,8 +127,9 @@ export class Connection implements IASTNodeLocationWithBlock { if (orphan) { const orphanConnection = this.type === INPUT ? orphan.outputConnection : orphan.previousConnection; + if (!orphanConnection) return; const connection = Connection.getConnectionForOrphanedConnection( - childBlock, (orphanConnection)); + childBlock, orphanConnection); if (connection) { orphanConnection.connect(connection); } else { @@ -676,11 +677,11 @@ function getSingleConnection(block: Block, orphanBlock: Block): Connection| null { let foundConnection = null; const output = orphanBlock.outputConnection; - const typeChecker = output.getConnectionChecker(); + const typeChecker = output?.getConnectionChecker(); for (let i = 0, input; input = block.inputList[i]; i++) { const connection = input.connection; - if (connection && typeChecker.canConnect(output, connection, false)) { + if (connection && typeChecker?.canConnect(output, connection, false)) { if (foundConnection) { return null; // More than one connection. } diff --git a/core/events/events_block_move.ts b/core/events/events_block_move.ts index 0034c2e0b..e886a06ac 100644 --- a/core/events/events_block_move.ts +++ b/core/events/events_block_move.ts @@ -261,7 +261,7 @@ export class BlockMove extends BlockBase { blockConnection = block.previousConnection; } let parentConnection; - const connectionType = blockConnection.type; + const connectionType = blockConnection?.type; if (inputName) { const input = parentBlock!.getInput(inputName); if (input) { @@ -270,7 +270,7 @@ export class BlockMove extends BlockBase { } else if (connectionType === ConnectionType.PREVIOUS_STATEMENT) { parentConnection = parentBlock!.nextConnection; } - if (parentConnection) { + if (parentConnection && blockConnection) { blockConnection.connect(parentConnection); } else { console.warn('Can\'t connect to non-existent input: ' + inputName); diff --git a/core/field_angle.ts b/core/field_angle.ts index be00a958e..31c910920 100644 --- a/core/field_angle.ts +++ b/core/field_angle.ts @@ -98,9 +98,7 @@ export class FieldAngle extends FieldInput { line_: SVGLineElement|null = null; /** The degree symbol for this field. */ - // AnyDuringMigration because: Type 'null' is not assignable to type - // 'SVGTSpanElement'. - protected symbol_: SVGTSpanElement = null as AnyDuringMigration; + protected symbol_: SVGTSpanElement|null = null; /** Wrapper click event data. */ private clickWrapper_: browserEvents.Data|null = null; @@ -213,10 +211,7 @@ export class FieldAngle extends FieldInput { this.sourceBlock_.style.colourTertiary); } - // AnyDuringMigration because: Argument of type 'this' is not assignable to - // parameter of type 'Field'. - dropDownDiv.showPositionedByField( - this as AnyDuringMigration, this.dropdownDispose_.bind(this)); + dropDownDiv.showPositionedByField(this, this.dropdownDispose_.bind(this)); this.updateGraph_(); } @@ -387,12 +382,8 @@ export class FieldAngle extends FieldInput { ' 0 ', largeFlag, ' ', clockwiseFlag, ' ', x2, ',', y2, ' z'); } this.gauge_.setAttribute('d', path.join('')); - // AnyDuringMigration because: Argument of type 'number' is not assignable - // to parameter of type 'string'. - this.line_!.setAttribute('x2', x2 as AnyDuringMigration); - // AnyDuringMigration because: Argument of type 'number' is not assignable - // to parameter of type 'string'. - this.line_!.setAttribute('y2', y2 as AnyDuringMigration); + this.line_?.setAttribute('x2', `${x2}`); + this.line_?.setAttribute('y2', `${y2}`); } /** @@ -435,8 +426,7 @@ export class FieldAngle extends FieldInput { * @param opt_newValue The input value. * @returns A valid angle, or null if invalid. */ - protected override doClassValidation_(opt_newValue?: AnyDuringMigration): - number|null { + protected override doClassValidation_(opt_newValue?: any): number|null { const value = Number(opt_newValue); if (isNaN(value) || !isFinite(value)) { return null; diff --git a/core/inject.ts b/core/inject.ts index 1f7d81cbf..dcf62ac0c 100644 --- a/core/inject.ts +++ b/core/inject.ts @@ -94,7 +94,7 @@ export function inject( * @param options Dictionary of options. * @returns Newly created SVG image. */ -function createDom(container: Element, options: Options): Element { +function createDom(container: Element, options: Options): SVGElement { // Sadly browsers (Chrome vs Firefox) are currently inconsistent in laying // out content in RTL mode. Therefore Blockly forces the use of LTR, // then manually positions content in RTL as needed. @@ -146,7 +146,7 @@ function createDom(container: Element, options: Options): Element { * @param options Dictionary of options. * @returns Newly created main workspace. */ -function createMainWorkspace(svg: Element, options: Options): WorkspaceSvg { +function createMainWorkspace(svg: SVGElement, options: Options): WorkspaceSvg { options.parentWorkspace = null; const mainWorkspace = new WorkspaceSvg(options); const wsOptions = mainWorkspace.options; diff --git a/core/keyboard_nav/ast_node.ts b/core/keyboard_nav/ast_node.ts index b3e18d299..1319c4b57 100644 --- a/core/keyboard_nav/ast_node.ts +++ b/core/keyboard_nav/ast_node.ts @@ -419,6 +419,7 @@ export class ASTNode { case ASTNode.types.BLOCK: { const block = this.location_ as Block; const nextConnection = block.nextConnection; + if (!nextConnection) return null; return ASTNode.createConnectionNode(nextConnection); } case ASTNode.types.PREVIOUS: { @@ -493,6 +494,7 @@ export class ASTNode { case ASTNode.types.BLOCK: { const block = this.location_ as Block; const topConnection = getParentConnection(block); + if (!topConnection) return null; return ASTNode.createConnectionNode(topConnection); } case ASTNode.types.PREVIOUS: { @@ -743,7 +745,7 @@ export type Params = ASTNode.Params; * @param block The block to find the parent connection on. * @returns The connection connecting to the parent of the block. */ -function getParentConnection(block: Block): Connection { +function getParentConnection(block: Block): Connection|null { let topConnection = block.outputConnection; if (!topConnection || block.previousConnection && block.previousConnection.isConnected()) { diff --git a/core/names.ts b/core/names.ts index c6602d825..0a7ca3888 100644 --- a/core/names.ts +++ b/core/names.ts @@ -184,15 +184,13 @@ export class Names { */ getDistinctName(name: string, type: NameType|string): string { let safeName = this.safeName_(name); - let i = ''; - while (this.dbReverse.has(safeName + i) || - this.reservedWords.has(safeName + i)) { + let i: number|null = null; + 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; + i = i ? i + 1 : 2; } - safeName += i; + safeName += (i ?? ''); this.dbReverse.add(safeName); const isVar = type === NameType.VARIABLE || type === NameType.DEVELOPER_VARIABLE; diff --git a/core/theme_manager.ts b/core/theme_manager.ts index 2052b7f61..b4f8fb7c8 100644 --- a/core/theme_manager.ts +++ b/core/theme_manager.ts @@ -27,7 +27,6 @@ export class ThemeManager { /** A list of workspaces that are subscribed to this theme. */ private subscribedWorkspaces_: Workspace[] = []; private componentDB = new Map(); - owner_: AnyDuringMigration; /** * @param workspace The main workspace. @@ -81,9 +80,7 @@ export class ThemeManager { const element = component.element; const propertyName = component.propertyName; const style = this.theme && this.theme.getComponentStyle(key); - // AnyDuringMigration because: Property 'style' does not exist on type - // 'Element'. - (element as AnyDuringMigration).style[propertyName] = style || ''; + element.style.setProperty(propertyName, style || ''); } } @@ -126,7 +123,9 @@ export class ThemeManager { * @param propertyName The inline style property name to update. * @internal */ - subscribe(element: Element, componentName: string, propertyName: string) { + subscribe( + element: HTMLElement|SVGElement, componentName: string, + propertyName: string) { if (!this.componentDB.has(componentName)) { this.componentDB.set(componentName, []); } @@ -136,9 +135,7 @@ export class ThemeManager { // Initialize the element with its corresponding theme style. const style = this.theme && this.theme.getComponentStyle(componentName); - // AnyDuringMigration because: Property 'style' does not exist on type - // 'Element'. - (element as AnyDuringMigration).style[propertyName] = style || ''; + element.style.setProperty(propertyName, style || ''); } /** @@ -147,7 +144,7 @@ export class ThemeManager { * @param element The element to unsubscribe. * @internal */ - unsubscribe(element: Element) { + unsubscribe(element: HTMLElement|SVGElement) { if (!element) { return; } @@ -172,13 +169,7 @@ export class ThemeManager { * @internal */ dispose() { - this.owner_ = null; - // AnyDuringMigration because: Type 'null' is not assignable to type - // 'Theme'. - this.theme = null as AnyDuringMigration; - // AnyDuringMigration because: Type 'null' is not assignable to type - // 'Workspace[]'. - this.subscribedWorkspaces_ = null as AnyDuringMigration; + this.subscribedWorkspaces_.length = 0; this.componentDB.clear(); } } @@ -186,7 +177,7 @@ export class ThemeManager { export namespace ThemeManager { /** The type for a Blockly UI Component. */ export interface Component { - element: Element; + element: HTMLElement|SVGElement; propertyName: string; } } diff --git a/core/xml.ts b/core/xml.ts index d3e50d3b0..b6f1753f5 100644 --- a/core/xml.ts +++ b/core/xml.ts @@ -103,14 +103,12 @@ export function blockToDomWithXY(block: Block, opt_noId?: boolean): Element| } const element = blockToDom(block, opt_noId); - const xy = block.getRelativeToSurfaceXY(); - // AnyDuringMigration because: Property 'setAttribute' does not exist on type - // 'Element | DocumentFragment'. - (element as AnyDuringMigration) - .setAttribute('x', Math.round(block.workspace.RTL ? width - xy.x : xy.x)); - // AnyDuringMigration because: Property 'setAttribute' does not exist on type - // 'Element | DocumentFragment'. - (element as AnyDuringMigration).setAttribute('y', Math.round(xy.y)); + if (isElement(element)) { + const xy = block.getRelativeToSurfaceXY(); + element.setAttribute( + 'x', `${Math.round(block.workspace.RTL ? width - xy.x : xy.x)}`); + element.setAttribute('y', `${Math.round(xy.y)}`); + } return element; } @@ -194,15 +192,9 @@ export function blockToDom(block: Block, opt_noId?: boolean): Element| const commentElement = utilsXml.createElement('comment'); commentElement.appendChild(utilsXml.createTextNode(commentText)); - // AnyDuringMigration because: Argument of type 'boolean' is not assignable - // to parameter of type 'string'. - commentElement.setAttribute('pinned', pinned as AnyDuringMigration); - // AnyDuringMigration because: Argument of type 'number' is not assignable - // to parameter of type 'string'. - commentElement.setAttribute('h', size.height as AnyDuringMigration); - // AnyDuringMigration because: Argument of type 'number' is not assignable - // to parameter of type 'string'. - commentElement.setAttribute('w', size.width as AnyDuringMigration); + commentElement.setAttribute('pinned', `${pinned}`); + commentElement.setAttribute('h', `${size.height}`); + commentElement.setAttribute('w', `${size.width}`); element.appendChild(commentElement); } @@ -402,9 +394,7 @@ export function clearWorkspaceAndLoadFromXml( xml: Element, workspace: WorkspaceSvg): string[] { workspace.setResizesEnabled(false); workspace.clear(); - // AnyDuringMigration because: Argument of type 'WorkspaceSvg' is not - // assignable to parameter of type 'Workspace'. - const blockIds = domToWorkspace(xml, workspace as AnyDuringMigration); + const blockIds = domToWorkspace(xml, workspace); workspace.setResizesEnabled(true); return blockIds; } @@ -447,16 +437,8 @@ export function domToWorkspace(xml: Element, workspace: Workspace): string[] { // to be moved to a nested destination in the next operation. const block = domToBlock(xmlChildElement, workspace); newBlockIds.push(block.id); - // AnyDuringMigration because: Argument of type 'string | null' is not - // assignable to parameter of type 'string'. - const blockX = xmlChildElement.hasAttribute('x') ? - parseInt(xmlChildElement.getAttribute('x') as AnyDuringMigration) : - 10; - // AnyDuringMigration because: Argument of type 'string | null' is not - // assignable to parameter of type 'string'. - const blockY = xmlChildElement.hasAttribute('y') ? - parseInt(xmlChildElement.getAttribute('y') as AnyDuringMigration) : - 10; + const blockX = parseInt(xmlChildElement.getAttribute('x') ?? '10', 10); + const blockY = parseInt(xmlChildElement.getAttribute('y') ?? '10', 10); if (!isNaN(blockX) && !isNaN(blockY)) { block.moveBy(workspace.RTL ? width - blockX : blockX, blockY); } @@ -597,8 +579,6 @@ export function domToBlock(xmlBlock: Element, workspace: Workspace): Block { eventUtils.enable(); } if (eventUtils.isEnabled()) { - // AnyDuringMigration because: Property 'get' does not exist on type - // '(name: string) => void'. const newVariables = Variables.getAddedVariables(workspace, variablesBeforeCreation); // Fire a VarCreate event for each (if any) new variable created. @@ -627,9 +607,8 @@ export function domToVariables(xmlVariables: Element, workspace: Workspace) { const id = xmlChild.getAttribute('id'); const name = xmlChild.textContent; - // AnyDuringMigration because: Argument of type 'string | null' is not - // assignable to parameter of type 'string'. - workspace.createVariable(name as AnyDuringMigration, type, id); + if (!name) return; + workspace.createVariable(name, type, id); } } @@ -731,12 +710,8 @@ function applyCommentTagNodes(xmlChildren: Element[], block: Block) { const xmlChild = xmlChildren[i]; const text = xmlChild.textContent; const pinned = xmlChild.getAttribute('pinned') === 'true'; - // AnyDuringMigration because: Argument of type 'string | null' is not - // assignable to parameter of type 'string'. - const width = parseInt(xmlChild.getAttribute('w') as AnyDuringMigration); - // AnyDuringMigration because: Argument of type 'string | null' is not - // assignable to parameter of type 'string'. - const height = parseInt(xmlChild.getAttribute('h') as AnyDuringMigration); + const width = parseInt(xmlChild.getAttribute('w') ?? '50', 10); + const height = parseInt(xmlChild.getAttribute('h') ?? '50', 10); block.setCommentText(text); block.commentModel.pinned = pinned; @@ -776,9 +751,11 @@ function applyFieldTagNodes(xmlChildren: Element[], block: Block) { for (let i = 0; i < xmlChildren.length; i++) { const xmlChild = xmlChildren[i]; const nodeName = xmlChild.getAttribute('name'); - // AnyDuringMigration because: Argument of type 'string | null' is not - // assignable to parameter of type 'string'. - domToField(block, nodeName as AnyDuringMigration, xmlChild); + if (!nodeName) { + console.warn(`Ignoring unnamed field in block ${block.type}`); + continue; + } + domToField(block, nodeName, xmlChild); } } @@ -790,24 +767,19 @@ function applyFieldTagNodes(xmlChildren: Element[], block: Block) { */ function findChildBlocks(xmlNode: Element): {childBlockElement: Element|null, childShadowElement: Element|null} { - const childBlockInfo = {childBlockElement: null, childShadowElement: null}; + let childBlockElement: Element|null = null; + let childShadowElement: Element|null = null; for (let i = 0; i < xmlNode.childNodes.length; i++) { const xmlChild = xmlNode.childNodes[i]; - if (xmlChild.nodeType === dom.NodeType.ELEMENT_NODE) { + if (isElement(xmlChild)) { if (xmlChild.nodeName.toLowerCase() === 'block') { - // AnyDuringMigration because: Type 'Element' is not assignable to type - // 'null'. - childBlockInfo.childBlockElement = - xmlChild as Element as AnyDuringMigration; + childBlockElement = xmlChild; } else if (xmlChild.nodeName.toLowerCase() === 'shadow') { - // AnyDuringMigration because: Type 'Element' is not assignable to type - // 'null'. - childBlockInfo.childShadowElement = - xmlChild as Element as AnyDuringMigration; + childShadowElement = xmlChild; } } } - return childBlockInfo; + return {childBlockElement, childShadowElement}; } /** * Applies input child nodes (value or statement) to the given block. @@ -823,9 +795,7 @@ function applyInputTagNodes( for (let i = 0; i < xmlChildren.length; i++) { const xmlChild = xmlChildren[i]; const nodeName = xmlChild.getAttribute('name'); - // AnyDuringMigration because: Argument of type 'string | null' is not - // assignable to parameter of type 'string'. - const input = block.getInput(nodeName as AnyDuringMigration); + const input = nodeName ? block.getInput(nodeName) : null; if (!input) { console.warn( 'Ignoring non-existent input ' + nodeName + ' in block ' + @@ -899,10 +869,8 @@ function domToBlockHeadless( if (!prototypeName) { throw TypeError('Block type unspecified: ' + xmlBlock.outerHTML); } - const id = xmlBlock.getAttribute('id'); - // AnyDuringMigration because: Argument of type 'string | null' is not - // assignable to parameter of type 'string | undefined'. - block = workspace.newBlock(prototypeName, id as AnyDuringMigration); + const id = xmlBlock.getAttribute('id') ?? undefined; + block = workspace.newBlock(prototypeName, id); // Preprocess childNodes so tags can be processed in a consistent order. const xmlChildNameMap = mapSupportedXmlTags(xmlBlock); @@ -1018,3 +986,7 @@ export function deleteNext(xmlBlock: Element|DocumentFragment) { } } } + +function isElement(node: Node): node is Element { + return node.nodeType === dom.NodeType.ELEMENT_NODE; +} diff --git a/tests/mocha/workspace_svg_test.js b/tests/mocha/workspace_svg_test.js index c606abbe3..d33da06bc 100644 --- a/tests/mocha/workspace_svg_test.js +++ b/tests/mocha/workspace_svg_test.js @@ -70,7 +70,7 @@ suite('WorkspaceSvg', function() { 'Block 2 position y'); }); - test('Replacing shadow disposes svg', function() { + test('Replacing shadow disposes of old shadow', function() { const dom = Blockly.utils.xml.textToDom( '' + '' + @@ -84,7 +84,7 @@ suite('WorkspaceSvg', function() { const blocks = this.workspace.getAllBlocks(false); chai.assert.equal(blocks.length, 2, 'Block count'); const shadowBlock = blocks[1]; - chai.assert.exists(shadowBlock.getSvgRoot()); + chai.assert.equal(false, shadowBlock.isDeadOrDying()); const block = this.workspace.newBlock('simple_test_block'); block.initSvg(); @@ -92,8 +92,8 @@ suite('WorkspaceSvg', function() { const inputConnection = this.workspace.getTopBlocks()[0].getInput('NAME').connection; inputConnection.connect(block.outputConnection); - chai.assert.exists(block.getSvgRoot()); - chai.assert.notExists(shadowBlock.getSvgRoot()); + chai.assert.equal(false, block.isDeadOrDying()); + chai.assert.equal(true, shadowBlock.isDeadOrDying()); }); suite('updateToolbox', function() {