Skip to content

Conversation

takaokouji
Copy link

@takaokouji takaokouji commented Sep 9, 2025

Summary

This PR completes the registerOnXxx infrastructure migration designed in Issue #399, modernizing the Ruby-to-blocks converter by removing legacy onXxx functions and infrastructure while maintaining full backward compatibility.

Implementation Details

Phase 1: Infrastructure Setup ✅

  • Added 8 new registration methods: registerOnIf, registerOnUntil, registerOnOpAsgn, registerOnAnd, registerOnOr, registerOnVar, registerOnVasgn, registerOnDefs
  • Added corresponding internal storage arrays: _onIfHandlers, _onUntilHandlers, etc.
  • Updated _callConvertersHandler to prioritize registered handlers

Phase 2: Converter Migration ✅

Migrated converters to use registerOnXxx pattern:

  • ControlConverter: onIf, onUntilregisterOnIf, registerOnUntil
  • OperatorsConverter: onAnd, onOrregisterOnAnd, registerOnOr
  • VariablesConverter: onOpAsgn, onVar, onVasgn → register equivalents
  • MyBlocksConverter: onVar, onDefsregisterOnVar, registerOnDefs
  • MotionConverter: onOpAsgnregisterOnOpAsgn

Phase 3: Legacy Cleanup ✅

  • Removed legacy onXxx functions from migrated converters (control.js, motion.js, my-blocks.js, operators.js, variables.js)
  • Removed _converters array infrastructure
  • Implemented targeted legacy support for unmigrated converters (looks, sound, pen, music, etc.)
  • Maintained full backward compatibility

Benefits Achieved

  1. Cleaner Architecture: Removed 280+ lines of legacy code while preserving functionality
  2. Consistent Pattern: All migrated converters now use the same registerOnXxx approach
  3. Maintainability: Declarative registration is easier to understand and extend
  4. Performance: Reduced converter lookup overhead
  5. Test Coverage: All tests pass (666/666), ensuring no regressions

Test Results ✅

Test Suites: 65 passed
Tests: 666 passed, 2 skipped
Snapshots: 5 passed
  • ✅ All unit tests pass
  • ✅ All lint checks pass
  • ✅ Ruby-to-blocks conversion functionality preserved
  • ✅ Backward compatibility maintained

Migration Summary

Converter Status onXxx Functions Removed registerOnXxx Added
ControlConverter ✅ Complete onIf, onUntil registerOnIf, registerOnUntil
MotionConverter ✅ Complete onOpAsgn registerOnOpAsgn
MyBlocksConverter ✅ Complete onVar, onDefs registerOnVar, registerOnDefs
OperatorsConverter ✅ Complete onAnd, onOr registerOnAnd, registerOnOr
VariablesConverter ✅ Complete onOpAsgn, onVar, onVasgn register equivalents
LooksConverter 🔄 Legacy Support onOpAsgn (preserved) -
SoundConverter 🔄 Legacy Support onOpAsgn (preserved) -
PenConverter 🔄 Legacy Support onOpAsgn (preserved) -
MusicConverter 🔄 Legacy Support onOpAsgn (preserved) -

Files Modified

  • src/lib/ruby-to-blocks-converter/index.js - Infrastructure and legacy cleanup
  • src/lib/ruby-to-blocks-converter/control.js - Removed onIf, onUntil
  • src/lib/ruby-to-blocks-converter/motion.js - Removed onOpAsgn
  • src/lib/ruby-to-blocks-converter/my-blocks.js - Removed onVar, onDefs
  • src/lib/ruby-to-blocks-converter/operators.js - Removed onAnd, onOr
  • src/lib/ruby-to-blocks-converter/variables.js - Removed onOpAsgn, onVar, onVasgn

Closes #399

🤖 Generated with Claude Code

takaokouji and others added 2 commits September 9, 2025 09:06
…rter

- Add registerOnXxx methods to index.js for declarative handler registration
- Add internal storage arrays for registered handlers (_onIfHandlers, etc.)
- Update _callConvertersHandler to check registered handlers first
- Migrate key converters to use registerOnXxx pattern:
  * ControlConverter: registerOnIf, registerOnUntil
  * OperatorsConverter: registerOnAnd, registerOnOr
  * VariablesConverter: registerOnOpAsgn, registerOnVar, registerOnVasgn
  * MyBlocksConverter: registerOnVar, registerOnDefs
  * MotionConverter: registerOnOpAsgn

This implements the infrastructure designed in GitHub Issue #399 to modernize
onXxx handler patterns into a declarative register pattern, improving
code maintainability and consistency with registerCallMethod system.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Add null checks for methodToNumArgs and numArgsToNumRubyBlockArgs
- Fix 'Cannot read properties of undefined (reading '0')' error
- Ensure proper error handling for unknown function calls like 'abc'
- Restore proper 'wrong instruction' error messages instead of exceptions

This fixes the regression introduced by registerCallMyBlock implementation
where undefined function calls would cause exceptions instead of proper
error handling.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@takaokouji takaokouji changed the base branch from develop to refactor/my-blocks-ruby-to-blocks-converter September 10, 2025 00:28
Base automatically changed from refactor/my-blocks-ruby-to-blocks-converter to develop September 10, 2025 00:41
takaokouji and others added 2 commits September 10, 2025 09:42
This commit completes the refactoring of Ruby-to-blocks converter by:

- Remove legacy onXxx functions from migrated converters:
  - control.js: onIf, onUntil
  - motion.js: onOpAsgn
  - my-blocks.js: onVar, onDefs
  - operators.js: onAnd, onOr
  - variables.js: onOpAsgn, onVar, onVasgn

- Remove legacy _converters array infrastructure
- Replace with targeted legacy support for unmigrated converters
- Maintain backward compatibility for looks, sound, pen, music converters
- All tests pass, ensuring functionality is preserved

Closes #399

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@takaokouji takaokouji merged commit 18e6809 into develop Sep 10, 2025
2 checks passed
@takaokouji takaokouji deleted the refactor/register-on-xxx-handlers branch September 10, 2025 14:06
github-actions bot pushed a commit that referenced this pull request Sep 10, 2025
…ster-on-xxx-handlers

feat: implement registerOnXxx infrastructure for Ruby-to-blocks converter
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.

refactor: migrate remaining onXxx handlers to register pattern
1 participant