Fix duplicate listener name collision between DuplicateBlock and Arpe…#5806
Merged
walterbender merged 2 commits intosugarlabs:masterfrom Mar 8, 2026
Merged
Conversation
…ggioBlock Signed-off-by: Ady0333 <adityashinde1525@gmail.com>
Contributor
Author
|
@vanshika2720 and @walterbender , please review this pr once. |
Contributor
|
✅ All Jest tests passed! This PR is ready to merge. |
Signed-off-by: Ady0333 <adityashinde1525@gmail.com>
Contributor
|
✅ All Jest tests passed! This PR is ready to merge. |
vanshika2720
approved these changes
Feb 20, 2026
parthdagia05
pushed a commit
to parthdagia05/musicblocks
that referenced
this pull request
Mar 15, 2026
sugarlabs#5806) * Fix duplicate listener name collision between DuplicateBlock and ArpeggioBlock Signed-off-by: Ady0333 <adityashinde1525@gmail.com> * Format FlowBlocks.js to pass Prettier check Signed-off-by: Ady0333 <adityashinde1525@gmail.com> --------- Signed-off-by: Ady0333 <adityashinde1525@gmail.com>
parthdagia05
pushed a commit
to parthdagia05/musicblocks
that referenced
this pull request
Mar 15, 2026
sugarlabs#5806) * Fix duplicate listener name collision between DuplicateBlock and ArpeggioBlock Signed-off-by: Ady0333 <adityashinde1525@gmail.com> * Format FlowBlocks.js to pass Prettier check Signed-off-by: Ady0333 <adityashinde1525@gmail.com> --------- Signed-off-by: Ady0333 <adityashinde1525@gmail.com>
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
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
This PR fixes a listener name collision between
DuplicateBlockandArpeggioBlockthat caused silent block-graph corruption when one was nested inside the other. Both blocks constructed their teardown listener name as"_duplicate_" + turtle, and sincesetTurtleListener()stores listeners in a flat dictionary keyed by this string, the inner block registration overwrote the outer block’s listener. When the outerDuplicateBlockclamp ended, its teardown never executed, leavingconnectionStore[turtle][duplicateBlk]populated and permanently corrupting the block graph across runs.Impact
When nesting
ArpeggioinsideDuplicate notes(or vice versa), the outer block’s cleanup was silently swallowed. This caused connection rewiring performed byDuplicateBlockto never be restored, leading to progressively broken playback behavior across runs. The corruption persisted until page reload and produced no visible runtime error, making it especially misleading in classroom usage.Steps to Reproduce
Start → Arpeggio (Cm) clamp → Duplicate notes (2×) clamp → Note 1/4.Observed: incorrect duplication behavior and progressive corruption across runs.
Expected: each clamp restores its original connections after execution.
Root Cause
Both blocks used the identical listener key:
Fix
Append blk to the listener name so each block instance has a unique teardown listener, matching the convention used by other clamp blocks.
Scope
Two-line change in:
js/blocks/FlowBlocks.jsjs/blocks/IntervalsBlocks.jsNo logic changes, no API changes, no new state introduced.
Verification
Nested
DuplicateandArpeggioblocks now clean up correctly.connectionStoreis restored after execution.Multiple runs do not accumulate corruption.
Lint and tests pass.