From 76b5517008971954cfc36a2b6413c87b7b1423e1 Mon Sep 17 00:00:00 2001 From: Neil Fraser Date: Tue, 8 Jun 2021 06:03:14 -0700 Subject: [PATCH] Use null-prototype objects for maps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A {} has a bunch of names already defined on it (like ‘toString’). When using an object as a map with arbitrary keys, it should not inherit from Object.prototype. --- build.py | 2 +- core/component_manager.js | 8 ++++---- core/contextmenu_registry.js | 20 ++++++++----------- core/extensions.js | 2 +- core/names.js | 1 + core/procedures.js | 2 +- core/registry.js | 4 ++-- core/renderers/common/constants.js | 4 ++-- core/renderers/zelos/path_object.js | 20 ++++++++----------- core/toolbox/toolbox.js | 8 ++++---- core/trashcan.js | 7 +------ core/utils/dom.js | 2 +- core/utils/toolbox.js | 2 +- core/warning.js | 2 +- core/workspace_svg.js | 4 ++-- .../workspacefactory/wfactory_controller.js | 7 +++---- .../workspacefactory/wfactory_model.js | 6 +++--- generators/php/procedures.js | 2 +- generators/python/procedures.js | 1 - scripts/gulpfiles/build_tasks.js | 2 +- 20 files changed, 46 insertions(+), 60 deletions(-) diff --git a/build.py b/build.py index 48d1502c0..2f963ce38 100755 --- a/build.py +++ b/build.py @@ -135,7 +135,7 @@ this.BLOCKLY_BOOT = function(root) { f.write('\n') f.write('// Load Blockly.\n') - f.write('goog.require(\'Blockly.requires\')\n') + f.write('goog.require(\'Blockly.requires\');\n') f.write(""" delete root.BLOCKLY_DIR; diff --git a/core/component_manager.js b/core/component_manager.js index fff41a625..ddb2091b5 100644 --- a/core/component_manager.js +++ b/core/component_manager.js @@ -28,14 +28,14 @@ Blockly.ComponentManager = function() { * @type {!Object} * @private */ - this.componentData_ = {}; + this.componentData_ = Object.create(null); /** - * A map of capabilities to component ids. - * @type {!Object>} + * A map of capabilities to component IDs. + * @type {!Object>} * @private */ - this.capabilityToComponentIds_ = {}; + this.capabilityToComponentIds_ = Object.create(null); }; /** diff --git a/core/contextmenu_registry.js b/core/contextmenu_registry.js index 503e5e963..38cf08d3c 100644 --- a/core/contextmenu_registry.js +++ b/core/contextmenu_registry.js @@ -31,11 +31,11 @@ Blockly.ContextMenuRegistry = function() { Blockly.ContextMenuRegistry.registry = this; /** - * Registry of all registered RegistryItems, keyed by id. - * @type {!Object} + * Registry of all registered RegistryItems, keyed by ID. + * @type {!Object} * @private */ - this.registry_ = {}; + this.registry_ = Object.create(null); }; /** @@ -97,7 +97,7 @@ Blockly.ContextMenuRegistry.registry = null; */ Blockly.ContextMenuRegistry.prototype.register = function(item) { if (this.registry_[item.id]) { - throw Error('Menu item with id "' + item.id + '" is already registered.'); + throw Error('Menu item with ID "' + item.id + '" is already registered.'); } this.registry_[item.id] = item; }; @@ -108,11 +108,10 @@ Blockly.ContextMenuRegistry.prototype.register = function(item) { * @throws {Error} if an item with the given ID does not exist. */ Blockly.ContextMenuRegistry.prototype.unregister = function(id) { - if (this.registry_[id]) { - delete this.registry_[id]; - } else { - throw new Error('Menu item with id "' + id + '" not found.'); + if (!this.registry_[id]) { + throw new Error('Menu item with ID "' + id + '" not found.'); } + delete this.registry_[id]; }; /** @@ -120,10 +119,7 @@ Blockly.ContextMenuRegistry.prototype.unregister = function(id) { * @return {?Blockly.ContextMenuRegistry.RegistryItem} RegistryItem or null if not found */ Blockly.ContextMenuRegistry.prototype.getItem = function(id) { - if (this.registry_[id]) { - return this.registry_[id]; - } - return null; + return this.registry_[id] || null; }; /** diff --git a/core/extensions.js b/core/extensions.js index ac0d22262..cf459ead1 100644 --- a/core/extensions.js +++ b/core/extensions.js @@ -28,7 +28,7 @@ goog.requireType('Blockly.Block'); * The set of all registered extensions, keyed by extension name/id. * @private */ -Blockly.Extensions.ALL_ = {}; +Blockly.Extensions.ALL_ = Object.create(null); /** * Registers a new extension function. Extensions are functions that help diff --git a/core/names.js b/core/names.js index 3618a4603..60a0cfbed 100644 --- a/core/names.js +++ b/core/names.js @@ -228,5 +228,6 @@ Blockly.Names.prototype.safeName_ = function(name) { * @return {boolean} True if names are the same. */ Blockly.Names.equals = function(name1, name2) { + // name1.localeCompare(name2) is slower. return name1.toLowerCase() == name2.toLowerCase(); }; diff --git a/core/procedures.js b/core/procedures.js index 715496ca8..d88426f7d 100644 --- a/core/procedures.js +++ b/core/procedures.js @@ -404,7 +404,7 @@ Blockly.Procedures.getDefinition = function(name, workspace) { blocks[i]); var tuple = procedureBlock.getProcedureDef(); if (tuple && Blockly.Names.equals(tuple[0], name)) { - return blocks[i]; + return procedureBlock; } } } diff --git a/core/registry.js b/core/registry.js index a3348d27f..ae36458a5 100644 --- a/core/registry.js +++ b/core/registry.js @@ -33,7 +33,7 @@ goog.requireType('Blockly.ToolboxItem'); * * @type {Object>} */ -Blockly.registry.typeMap_ = {}; +Blockly.registry.typeMap_ = Object.create(null); /** * The string used to register the default class for a type of plugin. @@ -137,7 +137,7 @@ Blockly.registry.register = function( var typeRegistry = Blockly.registry.typeMap_[type]; // If the type registry has not been created, create it. if (!typeRegistry) { - typeRegistry = Blockly.registry.typeMap_[type] = {}; + typeRegistry = Blockly.registry.typeMap_[type] = Object.create(null); } // Validate that the given class has all the required properties. diff --git a/core/renderers/common/constants.js b/core/renderers/common/constants.js index 282d355d5..f9b0596c5 100644 --- a/core/renderers/common/constants.js +++ b/core/renderers/common/constants.js @@ -587,10 +587,10 @@ Blockly.blockRendering.ConstantProvider.prototype.setTheme = function( /** * The block styles map. - * @type {Object} + * @type {Object} * @package */ - this.blockStyles = {}; + this.blockStyles = Object.create(null); var blockStyles = theme.blockStyles; for (var key in blockStyles) { diff --git a/core/renderers/zelos/path_object.js b/core/renderers/zelos/path_object.js index 4362e8c2b..9a406e90b 100644 --- a/core/renderers/zelos/path_object.js +++ b/core/renderers/zelos/path_object.js @@ -45,17 +45,17 @@ Blockly.zelos.PathObject = function(root, style, constants) { /** * The selected path of the block. - * @type {SVGElement} + * @type {?SVGElement} * @private */ this.svgPathSelected_ = null; /** * The outline paths on the block. - * @type {!Object} + * @type {!Object} * @private */ - this.outlines_ = {}; + this.outlines_ = Object.create(null); /** * A set used to determine which outlines were used during a draw pass. The @@ -98,8 +98,7 @@ Blockly.zelos.PathObject.prototype.applyColour = function(block) { } // Apply colour to outlines. - for (var i = 0, keys = Object.keys(this.outlines_), - key; (key = keys[i]); i++) { + for (var key in this.outlines_) { this.outlines_[key].setAttribute('fill', this.style.colourTertiary); } }; @@ -110,8 +109,7 @@ Blockly.zelos.PathObject.prototype.applyColour = function(block) { Blockly.zelos.PathObject.prototype.flipRTL = function() { Blockly.zelos.PathObject.superClass_.flipRTL.call(this); // Mirror each input outline path. - for (var i = 0, keys = Object.keys(this.outlines_), - key; (key = keys[i]); i++) { + for (var key in this.outlines_) { this.outlines_[key].setAttribute('transform', 'scale(-1 1)'); } }; @@ -175,9 +173,8 @@ Blockly.zelos.PathObject.prototype.updateShapeForInputHighlight = function( * @package */ Blockly.zelos.PathObject.prototype.beginDrawing = function() { - this.remainingOutlines_ = {}; - for (var i = 0, keys = Object.keys(this.outlines_), - key; (key = keys[i]); i++) { + this.remainingOutlines_ = Object.create(null); + for (var 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; @@ -192,8 +189,7 @@ Blockly.zelos.PathObject.prototype.endDrawing = function() { // Go through all remaining outlines that were not used this draw pass, and // remove them. if (this.remainingOutlines_) { - for (var i = 0, keys = Object.keys(this.remainingOutlines_), - key; (key = keys[i]); i++) { + for (var key in this.remainingOutlines_) { this.removeOutlinePath_(key); } } diff --git a/core/toolbox/toolbox.js b/core/toolbox/toolbox.js index 9769b96c5..debd34013 100644 --- a/core/toolbox/toolbox.js +++ b/core/toolbox/toolbox.js @@ -125,10 +125,10 @@ Blockly.Toolbox = function(workspace) { /** * A map from toolbox item IDs to toolbox items. - * @type {!Object} + * @type {!Object} * @protected */ - this.contentMap_ = {}; + this.contentMap_ = Object.create(null); /** * Position of the toolbox and flyout relative to the workspace. @@ -393,7 +393,7 @@ Blockly.Toolbox.prototype.render = function(toolboxDef) { } } this.contents_ = []; - this.contentMap_ = {}; + this.contentMap_ = Object.create(null); this.renderContents_(toolboxDef['contents']); this.position(); }; @@ -537,7 +537,7 @@ Blockly.Toolbox.prototype.getClientRect = function() { * @public */ Blockly.Toolbox.prototype.getToolboxItemById = function(id) { - return this.contentMap_[id]; + return this.contentMap_[id] || null; }; /** diff --git a/core/trashcan.js b/core/trashcan.js index 49cc20762..cbcf89683 100644 --- a/core/trashcan.js +++ b/core/trashcan.js @@ -411,11 +411,7 @@ Blockly.Trashcan.prototype.openFlyout = function() { if (this.contentsIsOpen()) { return; } - - var xml = []; - for (var i = 0, text; (text = this.contents_[i]); i++) { - xml[i] = Blockly.Xml.textToDom(text); - } + var xml = this.contents_.map(Blockly.Xml.textToDom); this.flyout.show(xml); this.fireUiEvent_(true); }; @@ -427,7 +423,6 @@ Blockly.Trashcan.prototype.closeFlyout = function() { if (!this.contentsIsOpen()) { return; } - this.flyout.hide(); this.fireUiEvent_(false); }; diff --git a/core/utils/dom.js b/core/utils/dom.js index 9aa62807c..392f0094d 100644 --- a/core/utils/dom.js +++ b/core/utils/dom.js @@ -232,7 +232,7 @@ Blockly.utils.dom.setCssTransform = function(element, transform) { Blockly.utils.dom.startTextWidthCache = function() { Blockly.utils.dom.cacheReference_++; if (!Blockly.utils.dom.cacheWidths_) { - Blockly.utils.dom.cacheWidths_ = {}; + Blockly.utils.dom.cacheWidths_ = Object.create(null); } }; diff --git a/core/utils/toolbox.js b/core/utils/toolbox.js index bbce2ce39..1e745c0fa 100644 --- a/core/utils/toolbox.js +++ b/core/utils/toolbox.js @@ -353,7 +353,7 @@ Blockly.utils.toolbox.xmlToJsonArray_ = function(toolboxDef) { obj['contents'] = Blockly.utils.toolbox.xmlToJsonArray_(child); } - // Add xml attributes to object + // Add XML attributes to object Blockly.utils.toolbox.addAttributes_(child, obj); arr.push(obj); } diff --git a/core/warning.js b/core/warning.js index e60ceccda..9bbd9c91b 100644 --- a/core/warning.js +++ b/core/warning.js @@ -36,7 +36,7 @@ Blockly.Warning = function(block) { Blockly.Warning.superClass_.constructor.call(this, block); this.createIcon(); // The text_ object can contain multiple warnings. - this.text_ = {}; + this.text_ = Object.create(null); }; Blockly.utils.object.inherits(Blockly.Warning, Blockly.Icon); diff --git a/core/workspace_svg.js b/core/workspace_svg.js index 8179fc4f3..6797926c8 100644 --- a/core/workspace_svg.js +++ b/core/workspace_svg.js @@ -169,7 +169,7 @@ Blockly.WorkspaceSvg = function( * @type {!Object>} * @private */ - this.toolboxCategoryCallbacks_ = {}; + this.toolboxCategoryCallbacks_ = Object.create(null); /** * Map from function names to callbacks, for deciding what to do when a button @@ -177,7 +177,7 @@ Blockly.WorkspaceSvg = function( * @type {!Object} * @private */ - this.flyoutButtonCallbacks_ = {}; + this.flyoutButtonCallbacks_ = Object.create(null); if (Blockly.Variables && Blockly.Variables.flyoutCategory) { this.registerToolboxCategoryCallback(Blockly.VARIABLE_CATEGORY_NAME, diff --git a/demos/blockfactory/workspacefactory/wfactory_controller.js b/demos/blockfactory/workspacefactory/wfactory_controller.js index 387c0ee98..02f9c8cda 100644 --- a/demos/blockfactory/workspacefactory/wfactory_controller.js +++ b/demos/blockfactory/workspacefactory/wfactory_controller.js @@ -887,7 +887,6 @@ WorkspaceFactoryController.prototype.clearAll = function() { if (!confirm(msg)) { return; } - var hasCategories = this.model.hasElements(); this.model.clearToolboxList(); this.view.clearToolboxTabs(); this.model.savePreloadXml(Blockly.utils.xml.createElement('xml')); @@ -1209,9 +1208,9 @@ WorkspaceFactoryController.prototype.importBlocks = function(file, format) { // If an imported block type is already defined, check if the user wants // to override the current block definition. if (controller.model.hasDefinedBlockTypes(blockTypes)) { - var msg = 'An imported block uses the same name as a block ' - + 'already in your toolbox. Are you sure you want to override the ' - + 'currently defined block?'; + var msg = 'An imported block uses the same name as a block ' + + 'already in your toolbox. Are you sure you want to override the ' + + 'currently defined block?'; var continueAnyway = confirm(msg); BlocklyDevTools.Analytics.onWarning(msg); if (!continueAnyway) { diff --git a/demos/blockfactory/workspacefactory/wfactory_model.js b/demos/blockfactory/workspacefactory/wfactory_model.js index 0e9ad783b..c97e8d091 100644 --- a/demos/blockfactory/workspacefactory/wfactory_model.js +++ b/demos/blockfactory/workspacefactory/wfactory_model.js @@ -360,7 +360,7 @@ WorkspaceFactoryModel.prototype.addCustomTag = function(category, tag) { * @param {!Element} xml The XML to be saved. */ WorkspaceFactoryModel.prototype.savePreloadXml = function(xml) { - this.preloadXml = xml + this.preloadXml = xml; }; /** @@ -445,8 +445,8 @@ WorkspaceFactoryModel.prototype.updateLibBlockTypes = function(blockTypes) { * @return {boolean} True if blockType is defined, false otherwise. */ WorkspaceFactoryModel.prototype.isDefinedBlockType = function(blockType) { - var isStandardBlock = StandardCategories.coreBlockTypes.indexOf(blockType) - != -1; + var isStandardBlock = + StandardCategories.coreBlockTypes.indexOf(blockType) != -1; var isLibBlock = this.libBlockTypes.indexOf(blockType) != -1; var isImportedBlock = this.importedBlockTypes.indexOf(blockType) != -1; return (isStandardBlock || isLibBlock || isImportedBlock); diff --git a/generators/php/procedures.js b/generators/php/procedures.js index c14a72607..1b361ea9f 100644 --- a/generators/php/procedures.js +++ b/generators/php/procedures.js @@ -14,12 +14,12 @@ goog.provide('Blockly.PHP.procedures'); goog.require('Blockly.PHP'); + Blockly.PHP['procedures_defreturn'] = function(block) { // Define a procedure with a return value. // First, add a 'global' statement for every variable that is not shadowed by // a local parameter. var globals = []; - var varName; var workspace = block.workspace; var variables = Blockly.Variables.allUsedVarModels(workspace) || []; for (var i = 0, variable; variable = variables[i]; i++) { diff --git a/generators/python/procedures.js b/generators/python/procedures.js index 4288911d9..d5f46a8ac 100644 --- a/generators/python/procedures.js +++ b/generators/python/procedures.js @@ -20,7 +20,6 @@ Blockly.Python['procedures_defreturn'] = function(block) { // First, add a 'global' statement for every variable that is not shadowed by // a local parameter. var globals = []; - var varName; var workspace = block.workspace; var variables = Blockly.Variables.allUsedVarModels(workspace) || []; for (var i = 0, variable; variable = variables[i]; i++) { diff --git a/scripts/gulpfiles/build_tasks.js b/scripts/gulpfiles/build_tasks.js index 33a04fcee..6553e8872 100644 --- a/scripts/gulpfiles/build_tasks.js +++ b/scripts/gulpfiles/build_tasks.js @@ -398,7 +398,7 @@ return gulp.src(maybeAddClosureLibrary(['core/**/**/*.js'])) const requires = `goog.addDependency("base.js", [], []); // Load Blockly. -goog.require('Blockly.requires') +goog.require('Blockly.requires'); `; fs.writeFileSync('blockly_uncompressed.js', header +