feat: make the variable map return early for noops (#6674)

* chore: add variable map tests

* chore: move firing create events into the variable map

* feat: add early returning in the case of noop renames

* chore: format
This commit is contained in:
Beka Westberg
2022-12-02 08:10:41 -08:00
committed by GitHub
parent 67275e4bba
commit fc2c10d0d0
3 changed files with 203 additions and 3 deletions

View File

@@ -59,6 +59,7 @@ export class VariableMap {
* @internal
*/
renameVariable(variable: VariableModel, newName: string) {
if (variable.name === newName) return;
const type = variable.type;
const conflictVar = this.getVariable(newName, type);
const blocks = this.workspace.getAllBlocks(false);
@@ -184,6 +185,8 @@ export class VariableMap {
this.variableMap.delete(type);
this.variableMap.set(type, variables);
eventUtils.fire(new (eventUtils.get(eventUtils.VAR_CREATE))(variable));
return variable;
}
/* Begin functions for variable deletion. */

View File

@@ -15,7 +15,6 @@ goog.declareModuleId('Blockly.VariableModel');
// Unused import preserved for side-effects. Remove if unneeded.
import './events/events_var_create.js';
import * as eventUtils from './events/utils.js';
import * as idGenerator from './utils/idgenerator.js';
import type {Workspace} from './workspace.js';
@@ -58,8 +57,6 @@ export class VariableModel {
* UUID.
*/
this.id_ = opt_id || idGenerator.genUid();
eventUtils.fire(new (eventUtils.get(eventUtils.VAR_CREATE))(this));
}
/** @returns The ID for the variable. */

View File

@@ -8,6 +8,7 @@ goog.declareModuleId('Blockly.test.variableMap');
import {assertVariableValues} from './test_helpers/variables.js';
import {createGenUidStubWithReturns, sharedTestSetup, sharedTestTeardown} from './test_helpers/setup_teardown.js';
import {assertEventFired, assertEventNotFired, createChangeListenerSpy} from './test_helpers/events.js';
suite('Variable Map', function() {
@@ -253,4 +254,203 @@ suite('Variable Map', function() {
chai.assert.deepEqual(resultArray, []);
});
});
suite('event firing', function() {
setup(function() {
this.eventSpy = createChangeListenerSpy(this.workspace);
});
teardown(function() {
this.workspace.removeChangeListener(this.eventSpy);
});
suite('variable create events', function() {
test('create events are fired when a variable is created', function() {
this.variableMap.createVariable('test name', 'test type', 'test id');
assertEventFired(
this.eventSpy,
Blockly.Events.VarCreate,
{
varType: 'test type',
varName: 'test name',
varId: 'test id',
},
this.workspace.id);
});
test(
'create events are not fired if a variable is already exists',
function() {
this.variableMap.createVariable('test name', 'test type', 'test id');
this.eventSpy.resetHistory();
this.variableMap.createVariable('test name', 'test type', 'test id');
assertEventNotFired(
this.eventSpy,
Blockly.Events.VarCreate,
{},
this.workspace.id);
});
});
suite('variable delete events', function() {
suite('deleting with a variable', function() {
test('delete events are fired when a variable is deleted', function() {
const variable =
this.variableMap.createVariable('test name', 'test type', 'test id');
this.variableMap.deleteVariable(variable);
assertEventFired(
this.eventSpy,
Blockly.Events.VarDelete,
{
varType: 'test type',
varName: 'test name',
varId: 'test id',
},
this.workspace.id);
});
test(
'delete events are not fired when a variable does not exist',
function() {
const variable =
new Blockly.VariableModel(
this.workspace, 'test name', 'test type', 'test id');
this.variableMap.deleteVariable(variable);
assertEventNotFired(
this.eventSpy,
Blockly.Events.VarDelete,
{},
this.workspace.id);
});
});
suite('deleting by ID', function() {
test(
'delete events are fired when a variable is deleted',
function() {
this.variableMap.createVariable('test name', 'test type', 'test id');
this.variableMap.deleteVariableById('test id');
assertEventFired(
this.eventSpy,
Blockly.Events.VarDelete,
{
varType: 'test type',
varName: 'test name',
varId: 'test id',
},
this.workspace.id);
});
test(
'delete events are not fired when a variable does not exist',
function() {
this.variableMap.deleteVariableById('test id');
assertEventNotFired(
this.eventSpy,
Blockly.Events.VarDelete,
{},
this.workspace.id);
});
});
});
suite('variable rename events', function() {
suite('renaming with variable', function() {
test('rename events are fired when a variable is renamed', function() {
const variable =
this.variableMap.createVariable(
'test name', 'test type', 'test id');
this.variableMap.renameVariable(variable, 'new test name');
assertEventFired(
this.eventSpy,
Blockly.Events.VarRename,
{
oldName: 'test name',
newName: 'new test name',
varId: 'test id',
},
this.workspace.id);
});
test(
'rename events are not fired if the variable name already matches',
function() {
const variable =
this.variableMap.createVariable(
'test name', 'test type', 'test id');
this.variableMap.renameVariable(variable, 'test name');
assertEventNotFired(
this.eventSpy,
Blockly.Events.VarRename,
{},
this.workspace.id);
});
test(
'rename events are not fired if the variable does not exist',
function() {
const variable =
new Blockly.VariableModel(
'test name', 'test type', 'test id');
this.variableMap.renameVariable(variable, 'test name');
assertEventNotFired(
this.eventSpy,
Blockly.Events.VarRename,
{},
this.workspace.id);
});
});
suite('renaming by ID', function() {
test('rename events are fired when a variable is renamed', function() {
this.variableMap.createVariable('test name', 'test type', 'test id');
this.variableMap.renameVariableById('test id', 'new test name');
assertEventFired(
this.eventSpy,
Blockly.Events.VarRename,
{
oldName: 'test name',
newName: 'new test name',
varId: 'test id',
},
this.workspace.id);
});
test(
'rename events are not fired if the variable name already matches',
function() {
this.variableMap.createVariable(
'test name', 'test type', 'test id');
this.variableMap.renameVariableById('test id', 'test name');
assertEventNotFired(
this.eventSpy,
Blockly.Events.VarRename,
{},
this.workspace.id);
});
test(
'renaming throws if the variable does not exist',
function() {
// Not sure why this throws when the other one doesn't but might
// as well test it.
chai.assert.throws(() => {
this.variableMap.renameVariableById('test id', 'test name');
}, `Tried to rename a variable that didn't exist`);
});
});
});
});
});