fix: override old blocks in dynamic connections (#988)

* fix: add deserializing standard XML mutators for if, list create, and text join

* fix: add jsdoc

* tests: add tests

* docs: update readme

* fix: lint

* fix: remove magic constants

plugins/block-dynamic-connection/README.md

@@ -16,11 +16,12 @@ import * as BlockDynamicConnection from '@blockly/block-dynamic-connection';
 ```
 
 ## API
-- ~~`overrideOldBlockDefinitions`: Replaces the Blockly default blocks with the
+- `overrideOldBlockDefinitions`: Replaces the Blockly default blocks with the
   dynamic connection blocks. This enables projects to use the dynamic block
-  plugin without changing existing XML.~~
-  Do not use this API unless you are already using it, because it does not
-  perform as intended. See #844 for context.
+  plugin without changing existing XML.
+  Note that if you enable this, you will **never** be able to switch back to
+  non-dynamic connections, because this changes the way mutations are
+  serialized.
 
 ## XML
 ```xml

plugins/block-dynamic-connection/src/dynamic_if.js

@@ -67,6 +67,19 @@ Blockly.Blocks['dynamic_if'] = {
    * @this {Blockly.Block}
    */
   domToMutation: function(xmlElement) {
+    if (xmlElement.getAttribute('inputs')) {
+      this.deserializeInputs_(xmlElement);
+    } else {
+      this.deserializeCounts_(xmlElement);
+    }
+  },
+
+  /**
+   * Parses XML based on the 'inputs' attribute (non-standard).
+   * @param {!Element} xmlElement XML storage element.
+   * @this {Blockly.Block}
+   */
+  deserializeInputs_: function(xmlElement) {
     const inputs = xmlElement.getAttribute('inputs');
     if (inputs) {
       const inputNumbers = inputs.split(',');
@@ -100,6 +113,27 @@ Blockly.Blocks['dynamic_if'] = {
     this.inputCounter = next;
   },
 
+  /**
+   * Parses XML based on the 'elseif' and 'else' attributes (standard).
+   * @param {!Element} xmlElement XML storage element.
+   * @this {Blockly.Block}
+   */
+  deserializeCounts_: function(xmlElement) {
+    const elseifCount = parseInt(xmlElement.getAttribute('elseif'), 10) || 0;
+    const elseCount = parseInt(xmlElement.getAttribute('else'), 10) || 0;
+    for (let i = 1; i <= elseifCount; i++) {
+      this.appendValueInput('IF' + i).setCheck('Boolean').appendField(
+          Blockly.Msg['CONTROLS_IF_MSG_ELSEIF']);
+      this.appendStatementInput('DO' + i).appendField(
+          Blockly.Msg['CONTROLS_IF_MSG_THEN']);
+    }
+    if (elseCount) {
+      this.appendStatementInput('ELSE').appendField(
+          Blockly.Msg['CONTROLS_IF_MSG_ELSE']);
+    }
+    this.inputCounter = elseifCount + 1;
+  },
+
   /**
    * Finds the index of a connection. Used to determine where in the block to
    * insert new inputs.

plugins/block-dynamic-connection/src/dynamic_list_create.js

@@ -57,6 +57,19 @@ Blockly.Blocks['dynamic_list_create'] = {
    * @this {Blockly.Block}
    */
   domToMutation: function(xmlElement) {
+    if (xmlElement.getAttribute('inputs')) {
+      this.deserializeInputs_(xmlElement);
+    } else {
+      this.deserializeCounts_(xmlElement);
+    }
+  },
+
+  /**
+   * Parses XML based on the 'inputs' attribute (non-standard).
+   * @param {!Element} xmlElement XML storage element.
+   * @this {Blockly.Block}
+   */
+  deserializeInputs_: function(xmlElement) {
     const items = xmlElement.getAttribute('inputs');
     if (items) {
       const inputNames = items.split(',');
@@ -69,6 +82,21 @@ Blockly.Blocks['dynamic_list_create'] = {
     this.inputCounter = next;
   },
 
+  /**
+   * Parses XML based on the 'items' attribute (standard).
+   * @param {!Element} xmlElement XML storage element.
+   * @this {Blockly.Block}
+   */
+  deserializeCounts_: function(xmlElement) {
+    const itemCount = Math.max(
+        parseInt(xmlElement.getAttribute('items'), 10), this.minInputs);
+    // Two inputs are added automatically.
+    for (let i = this.minInputs; i < itemCount; i++) {
+      this.appendValueInput('ADD' + i);
+    }
+    this.inputCounter = itemCount;
+  },
+
   /**
    * Check whether a new input should be added and determine where it should go.
    * @param {!Blockly.Connection} connection The connection that has a

plugins/block-dynamic-connection/src/dynamic_text_join.js

@@ -57,6 +57,19 @@ Blockly.Blocks['dynamic_text_join'] = {
    * @this {Blockly.Block}
    */
   domToMutation: function(xmlElement) {
+    if (xmlElement.getAttribute('inputs')) {
+      this.deserializeInputs_(xmlElement);
+    } else {
+      this.deserializeCounts_(xmlElement);
+    }
+  },
+
+  /**
+   * Parses XML based on the 'inputs' attribute (non-standard).
+   * @param {!Element} xmlElement XML storage element.
+   * @this {Blockly.Block}
+   */
+  deserializeInputs_: function(xmlElement) {
     const items = xmlElement.getAttribute('inputs');
     if (items) {
       const inputNames = items.split(',');
@@ -69,6 +82,21 @@ Blockly.Blocks['dynamic_text_join'] = {
     this.inputCounter = next;
   },
 
+  /**
+   * Parses XML based on the 'items' attribute (standard).
+   * @param {!Element} xmlElement XML storage element.
+   * @this {Blockly.Block}
+   */
+  deserializeCounts_: function(xmlElement) {
+    const itemCount = Math.max(
+        parseInt(xmlElement.getAttribute('items'), 10), this.minInputs);
+    // Two inputs are added automatically.
+    for (let i = this.minInputs; i < itemCount; i++) {
+      this.appendValueInput('ADD' + i);
+    }
+    this.inputCounter = itemCount;
+  },
+
   /**
    * Check whether a new input should be added and determine where it should go.
    * @param {!Blockly.Connection} connection The connection that has a

plugins/block-dynamic-connection/src/index.js

@@ -16,7 +16,7 @@ import './dynamic_text_join.js';
 import './dynamic_list_create.js';
 
 export const overrideOldBlockDefinitions = function() {
-  Blockly.Blocks['list_create'] = Blockly.Blocks['dynamic_list_create'];
+  Blockly.Blocks['lists_create_with'] = Blockly.Blocks['dynamic_list_create'];
   Blockly.Blocks['text_join'] = Blockly.Blocks['dynamic_text_join'];
   Blockly.Blocks['controls_if'] = Blockly.Blocks['dynamic_if'];
 };

plugins/block-dynamic-connection/test/dynamic_if.mocha.js

@@ -7,8 +7,7 @@
 const chai = require('chai');
 const {testHelpers} = require('@blockly/dev-tools');
 const Blockly = require('blockly/node');
-
-require('../src/index');
+const {overrideOldBlockDefinitions} = require('../src/index');
 
 const assert = chai.assert;
 
@@ -17,9 +16,10 @@ suite('If block', function() {
    * Asserts that the if block has the expected inputs.
    * @param {!Blockly.Block} block The block to check.
    * @param {!Array<string>} expectedInputs The expected inputs.
+   * @type {string=} The block type expected. Defaults to 'dynamic_if'.
    */
-  function assertBlockStructure(block, expectedInputs) {
-    assert.equal(block.type, 'dynamic_if');
+  function assertBlockStructure(block, expectedInputs, type = 'dynamic_if') {
+    assert.equal(block.type, type);
     assert.equal(block.inputList.length, expectedInputs.length);
     assert.isTrue(expectedInputs.length >= 2);
     for (let i = 0; i < expectedInputs.length; i++) {
@@ -53,6 +53,7 @@ suite('If block', function() {
 
   setup(function() {
     this.workspace = new Blockly.Workspace();
+    overrideOldBlockDefinitions();
   });
 
   teardown(function() {
@@ -223,6 +224,21 @@ suite('If block', function() {
             block, ['IF0', 'DO0', 'IF1', 'DO1', 'IF2', 'DO2', 'ELSE']);
       },
     },
+    {
+      title: 'standard/core XML is deserialized correctly',
+      xml:
+        '<block type="controls_if" id="1" x="38" y="63">' +
+        '  <mutation elseif="1" else= "1" ></mutation>' +
+        '</block>',
+      expectedXml:
+          '<block xmlns="https://developers.google.com/blockly/xml" ' +
+          'type="controls_if" id="1">\n' +
+          '  <mutation inputs="0,1" else="true" next="2"></mutation>\n</block>',
+      assertBlockStructure: (block) => {
+        assertBlockStructure(
+            block, ['IF0', 'DO0', 'IF1', 'DO1', 'ELSE'], 'controls_if');
+      },
+    },
   ];
   testHelpers.runSerializationTestSuite(testCases);
 });

plugins/block-dynamic-connection/test/dynamic_list_create.mocha.js

@@ -7,8 +7,7 @@
 const chai = require('chai');
 const {testHelpers} = require('@blockly/dev-tools');
 const Blockly = require('blockly/node');
-
-require('../src/index');
+const {overrideOldBlockDefinitions} = require('../src/index');
 
 const assert = chai.assert;
 
@@ -17,9 +16,12 @@ suite('List create block', function() {
    * Asserts that the list create block has the expected inputs.
    * @param {!Blockly.Block} block The block to check.
    * @param {!Array<string>} expectedInputs The expected inputs.
+   * @type {string=} The block type expected. Defaults to 'dynamic_list_create'.
    */
-  function assertBlockStructure(block, expectedInputs) {
-    assert.equal(block.type, 'dynamic_list_create');
+  function assertBlockStructure(
+      block, expectedInputs, type = 'dynamic_list_create'
+  ) {
+    assert.equal(block.type, type);
     assert.equal(block.inputList.length, expectedInputs.length);
     assert.isTrue(expectedInputs.length >= 2);
     for (let i = 0; i < expectedInputs.length; i++) {
@@ -41,6 +43,7 @@ suite('List create block', function() {
 
   setup(function() {
     this.workspace = new Blockly.Workspace();
+    overrideOldBlockDefinitions();
   });
 
   teardown(function() {
@@ -161,6 +164,35 @@ suite('List create block', function() {
         assertBlockStructure(block, ['ADD1', 'ADD5', 'ADD2', 'ADD4']);
       },
     },
+    {
+      title: 'standard/core XML is deserialized correctly',
+      xml:
+        '<block type="lists_create_with" id="1" x="63" y="113">' +
+        '  <mutation items="3"></mutation>' +
+        '</block>',
+      expectedXml:
+          '<block xmlns="https://developers.google.com/blockly/xml" ' +
+          'type="lists_create_with" id="1">\n' +
+          '  <mutation inputs="ADD0,ADD1,ADD2" next="3"></mutation>\n</block>',
+      assertBlockStructure: (block) => {
+        assertBlockStructure(
+            block, ['ADD0', 'ADD1', 'ADD2'], 'lists_create_with');
+      },
+    },
+    {
+      title: 'standard/core XML still maintains minimum inputs',
+      xml:
+        '<block type="lists_create_with" id="1" x="63" y="113">' +
+        '  <mutation items="0"></mutation>' +
+        '</block>',
+      expectedXml:
+          '<block xmlns="https://developers.google.com/blockly/xml" ' +
+          'type="lists_create_with" id="1">\n' +
+          '  <mutation inputs="ADD0,ADD1" next="2"></mutation>\n</block>',
+      assertBlockStructure: (block) => {
+        assertBlockStructure(block, ['ADD0', 'ADD1'], 'lists_create_with');
+      },
+    },
   ];
   testHelpers.runSerializationTestSuite(testCases);
 });

plugins/block-dynamic-connection/test/dynamic_text_join.mocha.js

@@ -7,8 +7,7 @@
 const chai = require('chai');
 const {testHelpers} = require('@blockly/dev-tools');
 const Blockly = require('blockly/node');
-
-require('../src/index');
+const {overrideOldBlockDefinitions} = require('../src/index');
 
 const assert = chai.assert;
 
@@ -17,9 +16,12 @@ suite('Text join block', function() {
    * Asserts that the text join block has the expected inputs.
    * @param {!Blockly.Block} block The block to check.
    * @param {!Array<string>} expectedInputs The expected inputs.
+   * @type {string=} The block type expected. Defaults to 'dynamic_text_join'.
    */
-  function assertBlockStructure(block, expectedInputs) {
-    assert.equal(block.type, 'dynamic_text_join');
+  function assertBlockStructure(
+      block, expectedInputs, type = 'dynamic_text_join'
+  ) {
+    assert.equal(block.type, type);
     assert.equal(block.inputList.length, expectedInputs.length);
     assert.isTrue(expectedInputs.length >= 2);
     for (let i = 0; i < expectedInputs.length; i++) {
@@ -41,6 +43,7 @@ suite('Text join block', function() {
 
   setup(function() {
     this.workspace = new Blockly.Workspace();
+    overrideOldBlockDefinitions();
   });
 
   teardown(function() {
@@ -161,6 +164,35 @@ suite('Text join block', function() {
         assertBlockStructure(block, ['ADD1', 'ADD5', 'ADD2', 'ADD4']);
       },
     },
+    {
+      title: 'standard/core XML is deserialized correctly',
+      xml:
+        '<block type="text_join" id="1" x="63" y="113">' +
+        '  <mutation items="3"></mutation>' +
+        '</block>',
+      expectedXml:
+          '<block xmlns="https://developers.google.com/blockly/xml" ' +
+          'type="text_join" id="1">\n' +
+          '  <mutation inputs="ADD0,ADD1,ADD2" next="3"></mutation>\n</block>',
+      assertBlockStructure: (block) => {
+        assertBlockStructure(
+            block, ['ADD0', 'ADD1', 'ADD2'], 'text_join');
+      },
+    },
+    {
+      title: 'standard/core XML still maintains minimum inputs',
+      xml:
+        '<block type="text_join" id="1" x="63" y="113">' +
+        '  <mutation items="0"></mutation>' +
+        '</block>',
+      expectedXml:
+          '<block xmlns="https://developers.google.com/blockly/xml" ' +
+          'type="text_join" id="1">\n' +
+          '  <mutation inputs="ADD0,ADD1" next="2"></mutation>\n</block>',
+      assertBlockStructure: (block) => {
+        assertBlockStructure(block, ['ADD0', 'ADD1'], 'text_join');
+      },
+    },
   ];
   testHelpers.runSerializationTestSuite(testCases);
 });

plugins/block-dynamic-connection/test/index.html

@@ -17,7 +17,7 @@
 
   <xml xmlns="https://developers.google.com/blockly/xml" id="toolbox" style="display: none">
     <block type="text_join"></block>
-    <block type="list_create"></block>
+    <block type="lists_create_with"></block>
     <block type="controls_if"></block>
     <block type="logic_boolean"><field name="BOOL">TRUE</field></block>
     <block type="text"><field name="TEXT">abc</field></block>