Skip to content

feat: add destroy lifecycle hook to blocks #6678

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
BeksOmega merged 5 commits into from
Dec 5, 2022
Merged

feat: add destroy lifecycle hook to blocks #6678

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a1e2da5
chore: move healing tests
BeksOmega Dec 1, 2022
26de145
chore: add tests for calling destroy
BeksOmega Dec 1, 2022
1870886
feat: add calling the destroy callback
BeksOmega Dec 1, 2022
de6932c
chore: cleanup reversions
BeksOmega Dec 2, 2022
75d4faf
chore: fixup type of destroy
BeksOmega Dec 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions core/block.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,9 @@ export class Block implements IASTNodeLocation, IDeletable {
/** An optional method called during initialization. */
init?: (() => AnyDuringMigration)|null = undefined;

/** An optional method called during disposal. */
destroy?: (() => void) = undefined;

/**
* An optional serialization method for defining how to serialize the
* mutation state to XML. This must be coupled with defining
Expand Down Expand Up @@ -361,6 +364,9 @@ export class Block implements IASTNodeLocation, IDeletable {
}
} finally {
eventUtils.enable();
if (typeof this.destroy === 'function') {
this.destroy();
}
this.disposed = true;
}
}
Expand Down
284 changes: 181 additions & 103 deletions tests/mocha/block_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,12 @@ import {createDeprecationWarningStub} from './test_helpers/warnings.js';
import {createRenderedBlock} from './test_helpers/block_definitions.js';
import * as eventUtils from '../../build/src/core/events/utils.js';
import {sharedTestSetup, sharedTestTeardown, workspaceTeardown} from './test_helpers/setup_teardown.js';
import {createChangeListenerSpy, createMockEvent} from './test_helpers/events.js';


suite('Blocks', function() {
setup(function() {
sharedTestSetup.call(this, {fireEventsNow: false});
this.clock = sharedTestSetup.call(this, {fireEventsNow: false}).clock;
this.workspace = new Blockly.Workspace();
Blockly.defineBlocksWithJsonArray([
{
Expand Down Expand Up @@ -198,123 +199,200 @@ suite('Blocks', function() {
});
});
});
suite('Dispose', function() {
function assertDisposedNoheal(blocks) {
chai.assert.isFalse(blocks.A.disposed);
// A has nothing connected to it.
chai.assert.equal(blocks.A.getChildren().length, 0);
// B is disposed.
chai.assert.isTrue(blocks.B.disposed);
// And C is disposed.
chai.assert.isTrue(blocks.C.disposed);
}
function assertDisposedHealed(blocks) {
chai.assert.isFalse(blocks.A.disposed);
chai.assert.isFalse(blocks.C.disposed);
// A and C are connected.
chai.assert.equal(blocks.A.getChildren().length, 1);
chai.assert.equal(blocks.C.getParent(), blocks.A);
// B is disposed.
chai.assert.isTrue(blocks.B.disposed);
}
function assertDisposedHealFailed(blocks) {
chai.assert.isFalse(blocks.A.disposed);
chai.assert.isFalse(blocks.C.disposed);
// A has nothing connected to it.
chai.assert.equal(blocks.A.getChildren().length, 0);
// B is disposed.
chai.assert.isTrue(blocks.B.disposed);
// C is the top of its stack.
chai.assert.isNull(blocks.C.getParent());
}

suite('Row', function() {
suite('Disposal', function() {
suite('calling destroy', function() {
setup(function() {
this.blocks = createTestBlocks(this.workspace, true);
Blockly.Blocks['destroyable_block'] = {
init: function() { },
destroy: function() { },
};
this.block = this.workspace.newBlock('destroyable_block');
});

test('Don\'t heal', function() {
this.blocks.B.dispose(false);
assertDisposedNoheal(this.blocks);
});
test('Heal', function() {
this.blocks.B.dispose(true);
// Each block has only one input, and the types work.
assertDisposedHealed(this.blocks);
teardown(function() {
delete Blockly.Blocks['destroyable_block'];
});
test('Heal with bad checks', function() {
const blocks = this.blocks;

// A and C can't connect, but both can connect to B.
blocks.A.inputList[0].connection.setCheck('type1');
blocks.C.outputConnection.setCheck('type2');
test('destroy is called', function() {
const spy = sinon.spy(this.block, 'destroy');

// Each block has only one input, but the types don't work.
blocks.B.dispose(true);
assertDisposedHealFailed(blocks);
});
test('Parent has multiple inputs', function() {
const blocks = this.blocks;
// Add extra input to parent
blocks.A.appendValueInput("INPUT").setCheck(null);
blocks.B.dispose(true);
assertDisposedHealed(blocks);
this.block.dispose();

chai.assert.isTrue(spy.calledOnce, 'Expected destroy to be called.');
});
test('Middle has multiple inputs', function() {
const blocks = this.blocks;
// Add extra input to middle block
blocks.B.appendValueInput("INPUT").setCheck(null);
blocks.B.dispose(true);
assertDisposedHealed(blocks);

test('disposing is set before destroy', function() {
let disposing = null;
this.block.destroy = function() {
disposing = this.disposing;
};

this.block.dispose();

chai.assert.isTrue(
disposing,
'Expected disposing to be set to true before destroy is called.');
});
test('Child has multiple inputs', function() {
const blocks = this.blocks;
// Add extra input to child block
blocks.C.appendValueInput("INPUT").setCheck(null);
// Child block input count doesn't matter.
blocks.B.dispose(true);
assertDisposedHealed(blocks);

test('disposed is not set before destroy', function() {
let disposed = null;
this.block.destroy = function() {
disposed = this.disposed;
};

this.block.dispose();

chai.assert.isFalse(
disposed,
'Expected disposed to be false when destroy is called');
});
test('Child is shadow', function() {
const blocks = this.blocks;
blocks.C.setShadow(true);
blocks.B.dispose(true);
// Even though we're asking to heal, it will appear as if it has not
// healed because shadows always get destroyed.
assertDisposedNoheal(blocks);

test('events can be fired from destroy', function() {
const mockEvent = createMockEvent(this.workspace);
this.block.destroy = function() {
Blockly.Events.fire(mockEvent);
};
const spy = createChangeListenerSpy(this.workspace);

this.block.dispose();
this.clock.runAll();

chai.assert.isTrue(
spy.calledWith(mockEvent),
'Expected to be able to fire events from destroy');
});
});
suite('Stack', function() {
setup(function() {
this.blocks = createTestBlocks(this.workspace, false);
});

test('Don\'t heal', function() {
this.blocks.B.dispose();
assertDisposedNoheal(this.blocks);
});
test('Heal', function() {
this.blocks.B.dispose(true);
assertDisposedHealed(this.blocks);
});
test('Heal with bad checks', function() {
const blocks = this.blocks;
// A and C can't connect, but both can connect to B.
blocks.A.nextConnection.setCheck('type1');
blocks.C.previousConnection.setCheck('type2');
suite('stack/row healing', function() {
function assertDisposedNoheal(blocks) {
chai.assert.isFalse(blocks.A.disposed);
// A has nothing connected to it.
chai.assert.equal(blocks.A.getChildren().length, 0);
// B is disposed.
chai.assert.isTrue(blocks.B.disposed);
// And C is disposed.
chai.assert.isTrue(blocks.C.disposed);
}

// The types don't work.
blocks.B.dispose(true);
function assertDisposedHealed(blocks) {
chai.assert.isFalse(blocks.A.disposed);
chai.assert.isFalse(blocks.C.disposed);
// A and C are connected.
chai.assert.equal(blocks.A.getChildren().length, 1);
chai.assert.equal(blocks.C.getParent(), blocks.A);
// B is disposed.
chai.assert.isTrue(blocks.B.disposed);
}

function assertDisposedHealFailed(blocks) {
chai.assert.isFalse(blocks.A.disposed);
chai.assert.isFalse(blocks.C.disposed);
// A has nothing connected to it.
chai.assert.equal(blocks.A.getChildren().length, 0);
// B is disposed.
chai.assert.isTrue(blocks.B.disposed);
// C is the top of its stack.
chai.assert.isNull(blocks.C.getParent());
}

assertDisposedHealFailed(blocks);
suite('Row', function() {
setup(function() {
this.blocks = createTestBlocks(this.workspace, true);
});

test('Don\'t heal', function() {
this.blocks.B.dispose(false);
assertDisposedNoheal(this.blocks);
});

test('Heal', function() {
this.blocks.B.dispose(true);
// Each block has only one input, and the types work.
assertDisposedHealed(this.blocks);
});

test('Heal with bad checks', function() {
const blocks = this.blocks;

// A and C can't connect, but both can connect to B.
blocks.A.inputList[0].connection.setCheck('type1');
blocks.C.outputConnection.setCheck('type2');

// Each block has only one input, but the types don't work.
blocks.B.dispose(true);
assertDisposedHealFailed(blocks);
});

test('Parent has multiple inputs', function() {
const blocks = this.blocks;
// Add extra input to parent
blocks.A.appendValueInput("INPUT").setCheck(null);
blocks.B.dispose(true);
assertDisposedHealed(blocks);
});

test('Middle has multiple inputs', function() {
const blocks = this.blocks;
// Add extra input to middle block
blocks.B.appendValueInput("INPUT").setCheck(null);
blocks.B.dispose(true);
assertDisposedHealed(blocks);
});

test('Child has multiple inputs', function() {
const blocks = this.blocks;
// Add extra input to child block
blocks.C.appendValueInput("INPUT").setCheck(null);
// Child block input count doesn't matter.
blocks.B.dispose(true);
assertDisposedHealed(blocks);
});

test('Child is shadow', function() {
const blocks = this.blocks;
blocks.C.setShadow(true);
blocks.B.dispose(true);
// Even though we're asking to heal, it will appear as if it has not
// healed because shadows always get destroyed.
assertDisposedNoheal(blocks);
});
});
test('Child is shadow', function() {
const blocks = this.blocks;
blocks.C.setShadow(true);
blocks.B.dispose(true);
// Even though we're asking to heal, it will appear as if it has not
// healed because shadows always get destroyed.
assertDisposedNoheal(blocks);

suite('Stack', function() {
setup(function() {
this.blocks = createTestBlocks(this.workspace, false);
});

test('Don\'t heal', function() {
this.blocks.B.dispose();
assertDisposedNoheal(this.blocks);
});

test('Heal', function() {
this.blocks.B.dispose(true);
assertDisposedHealed(this.blocks);
});

test('Heal with bad checks', function() {
const blocks = this.blocks;
// A and C can't connect, but both can connect to B.
blocks.A.nextConnection.setCheck('type1');
blocks.C.previousConnection.setCheck('type2');

// The types don't work.
blocks.B.dispose(true);

assertDisposedHealFailed(blocks);
});

test('Child is shadow', function() {
const blocks = this.blocks;
blocks.C.setShadow(true);
blocks.B.dispose(true);
// Even though we're asking to heal, it will appear as if it has not
// healed because shadows always get destroyed.
assertDisposedNoheal(blocks);
});
});
});
});
Expand Down
12 changes: 12 additions & 0 deletions tests/mocha/test_helpers/events.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,18 @@ export function createChangeListenerSpy(workspace) {
return spy;
}

/**
* Creates a mock event for testing if arbitrary events get fired/received.
* @param {!Blockly.Workspace} workspace The workspace to create the mock in.
* @return {!Object} A mock event that can be fired via Blockly.Events.fire
*/
export function createMockEvent(workspace) {
return {
isNull: () => false,
workspaceId: workspace.id,
};
}

/**
* Asserts whether the given xml property has the expected property.
* @param {!Node} xmlValue The xml value to check.
Expand Down
5 changes: 5 additions & 0 deletions tests/mocha/test_helpers/setup_teardown.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,8 @@ function wrapDefineBlocksWithJsonArrayWithCleanup_(sharedCleanupObj) {
*
* @param {Object<string, boolean>} options Options to enable/disable setup
* of certain stubs.
* @return {{clock: *}} The fake clock (as part of an object to make refactoring
* easier).
*/
export function sharedTestSetup(options = {}) {
this.sharedSetupCalled_ = true;
Expand All @@ -122,6 +124,9 @@ export function sharedTestSetup(options = {}) {
this.blockTypesCleanup_ = this.sharedCleanup.blockTypesCleanup_;
this.messagesCleanup_ = this.sharedCleanup.messagesCleanup_;
wrapDefineBlocksWithJsonArrayWithCleanup_(this.sharedCleanup);
return {
clock: this.clock,
};
}

/**
Expand Down