fix: resolve scratch-blocks v2 render crash for arrayMethod/hashMethod (#634)#635
Merged
takaokouji merged 4 commits intoMay 4, 2026
Conversation
…` literals Issue #634 (under feat/upstream-merge-2026-05). Three independent v2 incompatibilities surfaced together when converting `a = [1, 2, 3]; puts(a.max)` and similar Ruby code: 1. **`ScratchBlocks.FieldVerticalSeparator` removed in v2.** scratch-blocks v2 no longer exports the class on the namespace; it is registered via `fieldRegistry`. The old `new ScratchBlocks.FieldVerticalSeparator()` call in `defineDynamicBlock.createAllInputs` threw mid-flight in `domToMutation`, which made the surrounding XML parser drop the rest of the block's `<next>` chain — every Ruby-converted script that touched an extension block with an icon ended up as detached top-level blocks. Add a v2-compatible `makeVerticalSeparator` helper that falls back to `fieldRegistry.fromJson({type: 'field_vertical_separator'})`. 2. **Eager scalar created by `_onVasgn`.** `assignments.js _onVasgn` calls `_lookupOrCreateVariable(varName)` *before* visiting the RHS, which registers `a` as a SCALAR in `_context.localVariables`. The array-literal converter then calls `_lookupOrCreateList(a)` to get the list, but the eager scalar is left behind — target-applier creates *both* `_a_1_` (list) and `_a_1_` (scalar) on the VM target, and scratch-blocks v2 fails to render any block referencing the list. The array-literal handler now drops the eager scalar before creating the list. 3. **Cross-store re-creation at top level.** At the top level the converter has no scope tracking (`_getCurrentScope() === null`), so a subsequent `_lookupOrCreateVariable('a')` call (e.g. from `visitLocalVariableReadNode` for `a.max`) takes the falls-through creation path and registers a fresh scalar with the same transformed name. Add a check in `_lookupOrCreateVariableOrList` to reuse an existing entry from the *other* store (lists ↔ localVariables) when the requested store is empty for that transformed name. Together these keep exactly one canonical variable per local name across the converter's data flow, which is required for scratch-blocks v2 to parse the workspace XML the VM emits. Note: rendering of `smalrubyRuby_arrayMethod` / `smalrubyRuby_hashMethod` blocks themselves is still pending — `Field.getTextContent()` returns null during render even with a clean variable graph. That is a separate v2 incompat tracked in #634 and will be addressed in follow-up commits.
6 tasks
…h on v2 Issue #634 — final fix. Smalruby's `defineDynamicBlock.updateBlockDisplay` followed a v1-style pattern that toggled `block.rendered = false` around `appendField` and re-issued `block.initSvg()` + `block.render()` at the end: const wasRendered = block.rendered; block.rendered = false; ... createAllInputs(block, ...); // → appendField(new FieldDropdown(...)) ... block.rendered = wasRendered; if (wasRendered) { block.initSvg(); block.render(); } In Blockly v12 (the runtime under scratch-blocks v2.1.19) this dance silently drops field initialisation: Input.appendField(field, name) { field.setSourceBlock(this.sourceBlock); this.sourceBlock.initialized && this.initField(field); this.sourceBlock.rendered && this.sourceBlock.queueRender(); } Input.initField(field) { this.sourceBlock.rendered ? field.init() : field.initModel(); } For a block constructed via `domToMutation`: * `block.initialized` is `false` until `initSvg()` runs the one-shot guard. * `block.rendered` is `true` (BlockSvg constructor default). Smalruby was forcing `rendered = false` during `appendField`, so: * `appendField` saw `initialized=false` → skipped `initField` entirely → field's SVG `<text>` element never created. * The follow-up `block.initSvg()` is gated by `initialized`, but because `initialized` was still `false`, that path is fine — *except* my code flipped `rendered` back to `true` before it. With `rendered=true`, `initSvg`'s field-init would run `field.init()` (which creates the SVG text). Either path *should* work. * In practice, neither path triggers consistently across the XML-parser-driven domToMutation flow, leaving the field's `textContent_` null. The next `block.render()` invokes the block renderer, which calls `Field.getSize` → `Field.render_` → `Field.getTextContent`, and the missing text element throws "The text content is null." Stack: at Field.getTextContent at Field.render_ at Field.getSize at new BlockRenderRow at BlockRenderer.createRows_ at BlockRenderer.measure at Block.render The error aborts the parser mid-tree, drops the rest of the block's `<next>` chain, and leaves Smalruby's flyout/workspace unable to render any script that references `smalrubyRuby_arrayMethod`, `smalrubyRuby_hashMethod`, or `smalrubyRuby_stringMethod`. Fix: stop toggling `block.rendered`. Let `appendField` see the genuine state (`initialized=false`, `rendered=true`) so it correctly delegates field initialisation to the subsequent `block.initSvg()` call we make at the end. `block.initSvg()` walks `inputList` and calls `field.init()` for every field, which creates each field's SVG `<text>` element. We also keep `block.render()` to re-measure the block with its new inputs. This mirrors what scratch-blocks v2's own `procedures.ts updateDisplay_` does — that's the working blueprint for the same pattern. After the fix: * `puts(a.max)` round-trips correctly (`a = [1, 2, 3]` block chain followed by `Array (a) . max` connected to `戻り値 と 1 秒言う`). * `Field.getTextContent` no longer throws. * Toolbox refresh no longer fails. Verified manually with Playwright on `?tab=ruby&ruby_version=2`.
takaokouji
changed the title
… = [...]` literals" This reverts commit cf7dc65.
… on v2
Three coordinated fixes for the array/hash method flyout crash on
scratch-blocks v2:
1. define-dynamic-block.js: scratch-blocks v2 only registers
FieldVerticalSeparator via fieldRegistry; the v1-style
`new ScratchBlocks.FieldVerticalSeparator()` constructor call now
throws "is not a constructor" at runtime. Add a `makeVerticalSeparator`
helper that prefers the v1 constructor when present and falls back to
`fieldRegistry.fromJson({type: 'field_vertical_separator'})` for v2.
2. target-applier.js: the converter eagerly creates a SCALAR entry in
`_context.localVariables` for `t = [...]` before visiting the RHS.
The array-literal handler then creates a LIST entry in `_context.lists`
under the same transformed name. Pushing both to the VM target makes
scratch-blocks v2 fail to parse the workspace XML for any block that
references the list. Iterate `lists` first and dedupe by
`(scope, name)` so the LIST wins and the eager SCALAR is dropped.
3. smalruby-ruby.js (Ruby generator): the bang-method generator
(`sort!`, `reverse!`) reads RECEIVER as a name and looks it up via
`variableNameByName(name)` (defaults to SCALAR_TYPE). With the SCALAR
dropped at target-applier boundary, the lookup returns null and the
round-trip emits `nil.sort!`. Fall back to `listNameByName` so array
bang methods resolve through the surviving LIST entry.
Closes #634
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Issue #634 の 完全修正。Ruby converter で
a = [1, 2, 3]; puts(a.max)のようなコードが正しくブロック描画されなかった問題を 3 つの v2 incompat fix で解消。Root cause
scratch-blocks v2 (= Blockly v12) で
Input.appendFieldのフィールド初期化フローが変わり、Smalruby 側のdefineDynamicBlock.updateBlockDisplayがblock.rendered = falseを前置していたためFieldDropdownの SVG<text>要素が作られず、Field.getTextContent()が null でBlock.render()が throw → workspace の XML 解析が中断されていた。(詳細は #634 の resolution comment 参照)
Changes
1.
define-dynamic-block.js—updateBlockDisplayの v2 対応block.rendered = falseの toggle を削除し、scratch-blocks v2procedures.ts updateDisplay_と同じパターンに修正。これでappendFieldが正しくfield.init()を delegate し、SVG<text>要素が初期化される。2.
define-dynamic-block.js—FieldVerticalSeparatorv2 namespace fixscratch-blocks v2 では
ScratchBlocks.FieldVerticalSeparatorが namespace export されていない (fieldRegistryのみで提供される)。makeVerticalSeparator(ScratchBlocks)ヘルパーで v1/v2 両対応。3. Ruby converter — 配列リテラル変換時の重複変数解消
_onVasgn(assignments.js) が_lookupOrCreateVariableを eager に呼んで scalar 変数を作り、その後配列リテラル handler が_lookupOrCreateListで list 変数を作るため、同名の_a_1_が VM target に 2 つ登録され、scratch-blocks v2 が同名変数の workspace XML を解析失敗していた。_context.localVariablesから削除_lookupOrCreateVariableOrListで、要求 type の store が空でも反対側 store にエントリがあれば再利用 (top-level scope では_getCurrentScope() === nullで create path に落ちるためのフォールバック)Test plan
puts(a.max)がArray (a) . maxブロックとして正しく描画される_a_1_変数が VM target に 1 つだけ存在 (type: list)Field.getTextContent()が null を返さないPR #630 (parent) で実施した #634 以外の Ruby/array 系テストへの regression は要確認 (CI 待ち)。
Outstanding
a.sort,h.keys,a.join(",")等) が動作することを再確認関連
feat/upstream-merge-2026-05)🤖 Generated with Claude Code