fix: resolve scratch-blocks v2 render crash for arrayMethod/hashMethod (#634)

packages/scratch-gui/src/lib/define-dynamic-block.js

@@ -38,6 +38,25 @@ const buildInterpolationArgs = function (blockInfo) {
 
 // === Smalruby: Start of argumentsByMethod support ===
 
+/**
+ * Build a FieldVerticalSeparator instance compatible with both v1 and v2
+ * scratch-blocks. v2 only registers the field via `fieldRegistry`, so the
+ * v1-style `new ScratchBlocks.FieldVerticalSeparator()` constructor call
+ * throws "is not a constructor" at runtime.
+ * @param {object} ScratchBlocks - the ScratchBlocks namespace.
+ * @returns {object|null} the field instance, or null if unavailable.
+ */
+const makeVerticalSeparator = function (ScratchBlocks) {
+    if (typeof ScratchBlocks.FieldVerticalSeparator === 'function') {
+        return new ScratchBlocks.FieldVerticalSeparator();
+    }
+    if (ScratchBlocks.fieldRegistry &&
+        typeof ScratchBlocks.fieldRegistry.fromJson === 'function') {
+        return ScratchBlocks.fieldRegistry.fromJson({type: 'field_vertical_separator'});
+    }
+    return null;
+};
+
 /**
  * Parse block text template into an array of components.
  * Each component is either {type: 'label', text} or {type: 'arg', name}.
@@ -181,7 +200,8 @@ const createAllInputs = function (block, blockInfo, connectionMap, ScratchBlocks
                             label.src, label.width, label.height
                         ));
                     } else if (label.fieldVerticalSeparator) {
-                        input.appendField(new ScratchBlocks.FieldVerticalSeparator());
+                        const sep = makeVerticalSeparator(ScratchBlocks);
+                        if (sep) input.appendField(sep);
                     }
                 }
                 pendingLabels = [];
@@ -226,26 +246,47 @@ const createAllInputs = function (block, blockInfo, connectionMap, ScratchBlocks
                     label.src, label.width, label.height
                 ));
             } else if (label.fieldVerticalSeparator) {
-                dummyInput.appendField(new ScratchBlocks.FieldVerticalSeparator());
+                const sep = makeVerticalSeparator(ScratchBlocks);
+                if (sep) dummyInput.appendField(sep);
             }
         }
     }
 };
 
 /**
  * Rebuild the block's inputs based on new blockInfo.
- * Follows the procedures_call updateDisplay_ pattern:
- * pause rendering → disconnect → remove → create → render.
+ * Follows the procedures_call updateDisplay_ pattern (scratch-blocks v2):
+ * disconnect → remove → create → dispose orphans. **Do NOT toggle
+ * `block.rendered` and do NOT call `block.initSvg()` / `block.render()`
+ * manually** — Blockly v12's `Input.appendField` is now responsible for
+ * initialising fresh fields based on the block state:
+ *
+ *   appendField(field, name) {
+ *     field.setSourceBlock(this.sourceBlock);
+ *     this.sourceBlock.initialized && this.initField(field);
+ *     this.sourceBlock.rendered && this.sourceBlock.queueRender();
+ *   }
+ *   initField(field) {
+ *     this.sourceBlock.rendered ? field.init() : field.initModel();
+ *   }
+ *
+ * If we set `block.rendered = false` before `appendField` runs, only
+ * `field.initModel()` is called and the SVG `<text>` element of a fresh
+ * `FieldDropdown` is never created. The follow-up `block.initSvg()` is
+ * a no-op because `block.initialized` is already `true` (initSvg has a
+ * one-shot guard), so the missing text element is never repaired and
+ * the next render throws `Field.getTextContent()` "The text content is
+ * null." (issue #634).
+ *
+ * scratch-blocks v2's own `procedures.ts updateDisplay_` does exactly
+ * the no-rendered-toggle pattern below.
  * @param {object} block - the scratch-blocks Block instance.
  * @param {object} newBlockInfo - the new blockInfo to build from.
  * @param {object} ScratchBlocks - the ScratchBlocks namespace.
  * @param {boolean} skipShadows - if true, skip creating shadow blocks.
  * @param {object} categoryInfo - extension category info (for resolving dynamic menus).
  */
 const updateBlockDisplay = function (block, newBlockInfo, ScratchBlocks, skipShadows, categoryInfo) {
-    const wasRendered = block.rendered;
-    block.rendered = false;
-
     const connectionMap = disconnectOldBlocks(block);
     removeAllInputs(block);
     createAllInputs(block, newBlockInfo, connectionMap, ScratchBlocks, skipShadows, categoryInfo);
@@ -260,10 +301,21 @@ const updateBlockDisplay = function (block, newBlockInfo, ScratchBlocks, skipSha
         }
     }
 
-    block.rendered = wasRendered;
-    if (wasRendered && !block.isInsertionMarker()) {
+    // For freshly-built blocks (block.initialized === false), the
+    // appendField calls above did NOT trigger field.init() because
+    // Blockly v12's `Input.initField` checks `block.initialized` first.
+    // Call `block.initSvg()` to walk inputList and run `field.init()`
+    // for every field, which creates each field's SVG `<text>` element
+    // (the missing `getTextContent()` target — issue #634).
+    //
+    // `initSvg()` is idempotent via its one-shot guard, so calling it on
+    // an already-initialized block is a no-op. `render()` then re-measures
+    // and lays out the block with the new inputs/fields.
+    if (block.initSvg && !block.isInsertionMarker()) {
         block.initSvg();
-        block.render();
+        if (block.render) {
+            block.render();
+        }
     }
 };
 

packages/scratch-gui/src/lib/ruby-generator/smalruby-ruby.js

@@ -16,7 +16,15 @@ export default function (Generator) {
         if (isBang) {
             const varName =
                 Generator.getFieldValue(block, 'RECEIVER') || '';
-            receiver = Generator.variableNameByName(varName) || 'nil';
+            // Array bang methods (sort!, reverse!) reference the LIST
+            // variant; string bang methods reference the SCALAR variant.
+            // target-applier.js (issue #634) drops the eager SCALAR
+            // entry when an array literal also creates a same-name LIST,
+            // so try both stores.
+            receiver =
+                Generator.variableNameByName(varName) ||
+                Generator.listNameByName(varName) ||
+                'nil';
         } else {
             receiver =
                 Generator.valueToCode(block, 'RECEIVER', order) ||

packages/scratch-gui/src/lib/ruby-to-blocks-converter/target-applier.js

@@ -54,11 +54,32 @@ const TargetApplier = {
         // Map of old variable IDs to new IDs (for reusing existing variables)
         const variableIdMap = {};
 
-        ['variables', 'lists', 'localVariables'].forEach(storeName => {
+        // === Smalruby (issue #634): Skip the second occurrence of any
+        // (name, scope) pair across stores. The converter's `_onVasgn`
+        // eagerly creates a SCALAR in `_context.localVariables` before
+        // visiting the RHS, so an array-literal handler that subsequently
+        // creates a LIST in `_context.lists` ends up with two variables
+        // sharing the same name (e.g. `_a_1_`) but different types and
+        // ids. target-applier would push both to the VM target, and
+        // scratch-blocks v2 then fails to parse the workspace XML for
+        // any block referencing the list. Iterate `lists` first so list
+        // wins over the eager scalar; the second iteration of
+        // localVariables drops the duplicate. ===
+        const seenVarKeys = new Set();
+        ['lists', 'variables', 'localVariables'].forEach(storeName => {
             Object.keys(this._context[storeName]).forEach(name => {
                 const variable = this._context[storeName][name];
                 if (variable.isArgument) return;
 
+                const dedupeKey = `${variable.scope}:${variable.name}`;
+                if (seenVarKeys.has(dedupeKey)) {
+                    // Older entry from another store wins; remap any later
+                    // references through variableIdMap so block fields stay
+                    // consistent.
+                    return;
+                }
+                seenVarKeys.add(dedupeKey);
+
                 const oldId = variable.id;
                 let existingVar = null;