Skip to content

#7391 circular blocks#7789

Open
dhruvpatil972 wants to merge 2 commits into
sugarlabs:masterfrom
dhruvpatil972:#7391-circular-blocks
Open

#7391 circular blocks#7789
dhruvpatil972 wants to merge 2 commits into
sugarlabs:masterfrom
dhruvpatil972:#7391-circular-blocks

Conversation

@dhruvpatil972

@dhruvpatil972 dhruvpatil972 commented Jul 11, 2026 •
edited
Loading

Copy link
Copy Markdown

Summary

Implements Directed Graph cycle detection inside loadNewBlocks in js/blocks.js using DFS. This prevents multi-block circular connection loops (e.g. A -> B -> A or A -> B -> C -> A) from causing RangeError (Maximum call stack size exceeded) crashes during project loading.

Fixes #7391

PR Category

  • Bug Fix
  • Feature
  • Performance
  • Tests
  • Documentation
  • CI/CD

Changes

1. Cycle Detection in Blocks

  • Implemented DFS-based cycle detection in loadNewBlocks (js/blocks.js).
  • Restructured edges to follow only outgoing/child connections (indices 1 and above, ignoring index 0 parent links) to prevent false-positives on normal connected stacks.

2. Unit & E2E Tests

  • Added Jest unit tests in js/__tests__/blocks.test.js validating self-loops and multi-block cycles.
  • Added Cypress E2E browser tests in cypress/e2e/circular-blocks.cy.js to verify rejection in a real browser page.

Verification

  • npm test — Passed all 6,748 tests across 190 suites ✅
  • npx cypress run --spec "cypress/e2e/circular-blocks.cy.js" — Passed ✅
  • npm run lint — Passed cleanly ✅

@github-actions github-actions Bot added bug fix Fixes a bug or incorrect behavior tests Adds or updates test coverage size/M Medium: 50-249 lines changed area/javascript Changes to JS source files area/tests Changes to test files feature Adds new functionality performance Improves performance (load time, memory, rendering) documentation Updates to docs, comments, or README ci Changes to CI/CD workflows and automation labels Jul 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@omsuneri omsuneri Awaiting requested review from omsuneri omsuneri is a code owner
@Ashutoshx7 Ashutoshx7 Awaiting requested review from Ashutoshx7 Ashutoshx7 is a code owner
@walterbender walterbender Awaiting requested review from walterbender walterbender is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

area/javascript Changes to JS source files area/tests Changes to test files bug fix Fixes a bug or incorrect behavior ci Changes to CI/CD workflows and automation documentation Updates to docs, comments, or README feature Adds new functionality performance Improves performance (load time, memory, rendering) size/M Medium: 50-249 lines changed tests Adds or updates test coverage

Projects

@MusicBlocks Issues and PRs
Status: Todo

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

loadNewBlocks only detects direct self-loops — multi-block circular connections can trigger recursive stack overflow during project load

1 participant

@dhruvpatil972