Skip to content

perf: optimize blockList iteration with for...of instead of for...in#5304

Merged
walterbender merged 4 commits intosugarlabs:masterfrom
Ashutoshx7:perf/optimize-blocklist-iteration
Feb 14, 2026
Merged

perf: optimize blockList iteration with for...of instead of for...in#5304
walterbender merged 4 commits intosugarlabs:masterfrom
Ashutoshx7:perf/optimize-blocklist-iteration

Conversation

@Ashutoshx7
Copy link
Contributor

Summary

This PR optimizes array iteration in js/blocks.js by replacing for...in loops with for...of loops.

Problem

The codebase was using for...in to iterate over this.blockList, which is an array. This is an anti-pattern because:

  • for...in iterates over all enumerable properties, including inherited ones
  • for...in is slower than for...of for arrays
  • for...of is the correct modern JavaScript pattern for array iteration

Solution

Replaced 23 instances of for...in loops with proper array iteration:

  • Used for...of when only the element is needed
  • Used for...of with .entries() when both index and element are needed

Functions optimized:

  • setBlockScale (2 loops)
  • bottomMostBlock, toggleCollapsibles (3 loops)
  • updateBlockPositions, bringToTop, checkBounds
  • moveAllBlocksExcept, _searchForArgFlow
  • changeDisabledStatus, unhighlightAll, hide, show
  • findUniqueActionName, findUniqueCustomName, findUniqueTemperamentName
  • _findDrumURLs, renameBoxes, renameStoreinBoxes
  • renameStorein2Boxes, renameNamedboxes

Testing

  • ✅ Comprehensive browser testing confirmed all functionality works

Impact

  • Performance: Faster array iteration
  • Correctness: Proper array iteration pattern
  • No breaking changes: All functionality preserved

Replace for...in loops with for...of for array iteration in blocks.js.
This is a performance optimization because:
- for...in iterates over enumerable properties including inherited ones
- for...in is slower than for...of for arrays
- for...of is the correct modern pattern for array iteration

Optimized 23 functions including:
- setBlockScale, bottomMostBlock, toggleCollapsibles
- updateBlockPositions, bringToTop, checkBounds
- moveAllBlocksExcept, _searchForArgFlow
- changeDisabledStatus, unhighlightAll, hide, show
- findUniqueActionName, findUniqueCustomName
- findUniqueTemperamentName, _findDrumURLs
- renameBoxes, renameStoreinBoxes, renameStorein2Boxes
- renameNamedboxes

All 2532 tests pass and browser testing confirms full functionality.
@github-actions
Copy link
Contributor

✅ All Jest tests passed! This PR is ready to merge.

@github-actions
Copy link
Contributor

✅ All Jest tests passed! This PR is ready to merge.

@github-actions
Copy link
Contributor

✅ All Jest tests passed! This PR is ready to merge.

@Ashutoshx7
Copy link
Contributor Author

@walterbender resolved merged conflict

@walterbender walterbender merged commit b472296 into sugarlabs:master Feb 14, 2026
6 checks passed
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