Skip to content

Warn conflicts with Blockly.Generator.prototype - #1603

Merged
AnmAtAnm merged 4 commits into
RaspberryPiFoundation:developfrom
AnmAtAnm:ban-block-names
Feb 12, 2018
Merged

Warn conflicts with Blockly.Generator.prototype#1603
AnmAtAnm merged 4 commits into
RaspberryPiFoundation:developfrom
AnmAtAnm:ban-block-names

Conversation

@AnmAtAnm

@AnmAtAnm AnmAtAnm commented Feb 12, 2018

Copy link
Copy Markdown
Contributor

Console warnings on block prototypeNames that conflict with Blockly.Generator.prototype

First pass at #1558

The basics

  • I branched from develop
  • My pull request is against develop
  • My code follows the style guide

The details

Resolves

Warning on conflicting Block names. Implementing a warning first, since fully banning type names is a breaking change.

Proposed Changes

Warn on conflict with Blockly.Generator.prototype members.

Reason for Changes

By convention, we use the Generator instance objects as generator function maps. This will conflict with prototype lookup for name collisions between block names and said members, and needs to be disallowed. However, this is a breaking changes, so emit a warning in this release.

Test Coverage

Locally edited Playground test blocks to have conflicting block instances. Saw the expected the error.

Also ran the build script with no bad effect on existing demos.

@AnmAtAnm AnmAtAnm self-assigned this Feb 12, 2018
Comment thread core/block.js Outdated
Blockly.Block = function(workspace, prototypeName, opt_id) {
if (typeof Blockly.Generator.prototype[prototypeName] !== 'undefined') {
console.warn('FUTURE ERROR: Block prototypeName "' + prototypeName
+ '`" conflicts with Blockly.Generator members. Registering Generators '

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's an extra ` here that doesn't match the rest of your quote.

@AnmAtAnm
AnmAtAnm merged commit dc7783e into RaspberryPiFoundation:develop Feb 12, 2018
@AnmAtAnm
AnmAtAnm deleted the ban-block-names branch February 15, 2018 00:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants