Skip to content

refactor(screenreader-trap): replace recursive util functions#245

Merged
ianmcburnie merged 1 commit intomasterfrom
refactor-screenreader-trap
Mar 31, 2026
Merged

refactor(screenreader-trap): replace recursive util functions#245
ianmcburnie merged 1 commit intomasterfrom
refactor-screenreader-trap

Conversation

@ianmcburnie
Copy link
Copy Markdown
Member

Summary

  • Rewrites three recursive functions in makeup-screenreader-trap/src/util.js as iterative while loops: getPreviousSiblings, getNextSiblings, and getAllAncestors
  • Eliminates the risk of stack overflow on deeply nested or large DOM trees
  • The getAllAncestors helper is removed — its logic is inlined directly into getAncestors, reducing indirection
  • No behaviour change; all 86 existing tests pass

Test plan

  • npm run test:unit -- packages/core/makeup-screenreader-trap passes (86 tests)
  • Manual smoke test: open the screenreader-trap demo and verify trap/untrap behaviour

…erative loops

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors the DOM traversal helpers in makeup-screenreader-trap to use iterative loops instead of recursion, preventing potential stack overflows on very deep DOM trees while preserving existing behavior.

Changes:

  • Replaced recursive sibling-walking utilities with iterative while loops (getPreviousSiblings, getNextSiblings).
  • Removed the getAllAncestors helper and inlined its logic into getAncestors using an iterative loop.
  • Updated built dist outputs and the docs bundle artifacts to reflect the source changes.

Reviewed changes

Copilot reviewed 1 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
packages/core/makeup-screenreader-trap/src/util.js Converts recursive DOM traversal helpers to iterative loops and inlines ancestor collection.
packages/core/makeup-screenreader-trap/dist/mjs/util.js Updates ESM build output for the iterative traversal changes.
packages/core/makeup-screenreader-trap/dist/cjs/util.js Updates CJS build output for the iterative traversal changes.
docs/core/makeup-screenreader-trap/index.min.js.map Regenerated sourcemap artifact reflecting updated bundle contents.
docs/core/makeup-screenreader-trap/index.min.js Regenerated docs bundle reflecting updated traversal implementation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@ianmcburnie ianmcburnie merged commit 23e4f00 into master Mar 31, 2026
5 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