Skip to content

Commit

Permalink
fix: Made workspace non-nullable. (google#6300)
Browse files Browse the repository at this point in the history
* Made workspace non-nullable.

* chore: clang-format files.

* fix: Fixed incorrect block disposal check in connection.ts
  • Loading branch information
gonfunko authored Aug 2, 2022
1 parent 21d9069 commit 83a3e74
Show file tree
Hide file tree
Showing 27 changed files with 121 additions and 130 deletions.
46 changes: 20 additions & 26 deletions core/block.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,9 +221,7 @@ export class Block implements IASTNodeLocation, IDeletable {
type!: string;
// Record initial inline state.
inputsInlineDefault?: boolean;
// Setting this to null indicates that the block has been disposed. Must be
// nullable.
workspace: Workspace|null;
workspace: Workspace;

/**
* @param workspace The block's workspace.
Expand Down Expand Up @@ -317,7 +315,7 @@ export class Block implements IASTNodeLocation, IDeletable {
* @suppress {checkTypes}
*/
dispose(healStack: boolean) {
if (!this.workspace) {
if (this.disposed) {
// Already deleted.
return;
}
Expand All @@ -335,13 +333,10 @@ export class Block implements IASTNodeLocation, IDeletable {
try {
// This block is now at the top of the workspace.
// Remove this block from the workspace's list of top-most blocks.
if (this.workspace) {
this.workspace.removeTopBlock(this);
this.workspace.removeTypedBlock(this);
// Remove from block database.
this.workspace.removeBlockById(this.id);
this.workspace = null;
}
this.workspace.removeTopBlock(this);
this.workspace.removeTypedBlock(this);
// Remove from block database.
this.workspace.removeBlockById(this.id);

// First, dispose of all my children.
for (let i = this.childBlocks_.length - 1; i >= 0; i--) {
Expand Down Expand Up @@ -428,7 +423,7 @@ export class Block implements IASTNodeLocation, IDeletable {
// Disconnect the child block.
childConnection?.disconnect();
// Connect child to the parent if possible, otherwise bump away.
if (this.workspace!.connectionChecker.canConnect(
if (this.workspace.connectionChecker.canConnect(
childConnection, parentConnection, false)) {
parentConnection.connect(childConnection!);
} else {
Expand Down Expand Up @@ -481,7 +476,7 @@ export class Block implements IASTNodeLocation, IDeletable {
const nextTarget = this.nextConnection.targetConnection;
nextTarget?.disconnect();
if (previousTarget &&
this.workspace!.connectionChecker.canConnect(
this.workspace.connectionChecker.canConnect(
previousTarget, nextTarget, false)) {
// Attach the next statement to the previous statement.
previousTarget.connect(nextTarget!);
Expand Down Expand Up @@ -721,15 +716,15 @@ export class Block implements IASTNodeLocation, IDeletable {
} else {
// New parent must be non-null so remove this block from the workspace's
// list of top-most blocks.
this.workspace!.removeTopBlock(this);
this.workspace.removeTopBlock(this);
}

this.parentBlock_ = newParent;
if (newParent) {
// Add this block to the new parent's child list.
newParent.childBlocks_.push(this);
} else {
this.workspace!.addTopBlock(this);
this.workspace.addTopBlock(this);
}
}

Expand Down Expand Up @@ -759,8 +754,8 @@ export class Block implements IASTNodeLocation, IDeletable {
* @return True if deletable.
*/
isDeletable(): boolean {
return this.deletable_ && !this.isShadow_ &&
!(this.workspace && this.workspace.options.readOnly);
return this.deletable_ && !this.isShadow_ && !this.disposed &&
!this.workspace.options.readOnly;
}

/**
Expand All @@ -776,8 +771,8 @@ export class Block implements IASTNodeLocation, IDeletable {
* @return True if movable.
*/
isMovable(): boolean {
return this.movable_ && !this.isShadow_ &&
!(this.workspace && this.workspace.options.readOnly);
return this.movable_ && !this.isShadow_ && !this.disposed &&
!this.workspace.options.readOnly;
}

/**
Expand All @@ -796,10 +791,10 @@ export class Block implements IASTNodeLocation, IDeletable {
* @return True if duplicatable.
*/
isDuplicatable(): boolean {
if (!this.workspace!.hasBlockLimits()) {
if (!this.workspace.hasBlockLimits()) {
return true;
}
return this.workspace!.isCapacityAvailable(
return this.workspace.isCapacityAvailable(
common.getBlockTypeCounts(this, true));
}

Expand Down Expand Up @@ -843,8 +838,7 @@ export class Block implements IASTNodeLocation, IDeletable {
* @return True if editable.
*/
isEditable(): boolean {
return this.editable_ &&
!(this.workspace && this.workspace.options.readOnly);
return this.editable_ && !this.disposed && !this.workspace.options.readOnly;
}

/**
Expand Down Expand Up @@ -974,11 +968,11 @@ export class Block implements IASTNodeLocation, IDeletable {
throw Error('onchange must be a function.');
}
if (this.onchangeWrapper_) {
this.workspace!.removeChangeListener(this.onchangeWrapper_);
this.workspace.removeChangeListener(this.onchangeWrapper_);
}
this.onchange = onchangeFn;
this.onchangeWrapper_ = onchangeFn.bind(this);
this.workspace!.addChangeListener(this.onchangeWrapper_);
this.workspace.addChangeListener(this.onchangeWrapper_);
}

/**
Expand Down Expand Up @@ -1031,7 +1025,7 @@ export class Block implements IASTNodeLocation, IDeletable {
for (let j = 0, field; field = input.fieldRow[j]; j++) {
if (field.referencesVariables()) {
const model =
this.workspace!.getVariableById(field.getValue() as string);
this.workspace.getVariableById(field.getValue() as string);
// Check if the variable actually exists (and isn't just a potential
// variable).
if (model) {
Expand Down
8 changes: 4 additions & 4 deletions core/block_animations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ let disconnectGroup: Element = null as AnyDuringMigration;
* @internal
*/
export function disposeUiEffect(block: BlockSvg) {
const workspace = block.workspace!;
const workspace = block.workspace;
const svgGroup = block.getSvgRoot();
workspace.getAudioManager().play('delete');

Expand Down Expand Up @@ -100,7 +100,7 @@ function disposeUiStep(
* @internal
*/
export function connectionUiEffect(block: BlockSvg) {
const workspace = block.workspace!;
const workspace = block.workspace;
const scale = workspace.scale;
workspace.getAudioManager().play('click');
if (scale < 1) {
Expand Down Expand Up @@ -157,8 +157,8 @@ function connectionUiStep(ripple: SVGElement, start: Date, scale: number) {
* @internal
*/
export function disconnectUiEffect(block: BlockSvg) {
block.workspace!.getAudioManager().play('disconnect');
if (block.workspace!.scale < 1) {
block.workspace.getAudioManager().play('disconnect');
if (block.workspace.scale < 1) {
return; // Too small to care about visual effects.
}
// Horizontal distance for bottom of block to wiggle.
Expand Down
2 changes: 1 addition & 1 deletion core/block_dragger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ export class BlockDragger implements IBlockDragger {
// Blocks dragged directly from a flyout may need to be bumped into
// bounds.
bumpObjects.bumpIntoBounds(
this.draggingBlock_.workspace!,
this.draggingBlock_.workspace,
this.workspace_.getMetricsManager().getScrollMetrics(true),
this.draggingBlock_);
}
Expand Down
Loading

0 comments on commit 83a3e74

Please sign in to comment.