Skip to content

Conversation

@seanmcguire12
Copy link
Member

@seanmcguire12 seanmcguire12 commented Dec 17, 2025

why

  • a11y/snapshot.ts has large inline stringified scripts for scroll offsets, active-element lookup, and XPath building. stringified JS is not good practice as it is difficult to debug and is ignored by linting
  • there was also duplicate selector resolution logic inside snapshot.ts which already exists inside locatorScripts dir

what changed

  • added lib/v3/dom/a11yScripts plus a new genA11yScripts.ts generator so those helpers are bundled once, exported as strings and injected as needed
  • added shared buildA11yInvocation & buildLocatorInvocation helpers that wrap the JS expressions with a guard that checks if the script already exists
  • changed a11y/snapshot.ts to consume the generated helpers for scroll offsets, deep-active resolution, and XPath serialization, and to call locator scripts for CSS/XPath queries instead of embedding custom JS strings.

test plan

  • existing tests

Summary by cubic

Replaced inline stringified JS in a11y/snapshot.ts with a generated a11y script bundle and small invocation helpers for cleaner, lintable code. Also routed CSS/XPath queries through existing locator scripts to remove duplicated logic.

  • Refactors
    • Added lib/v3/dom/a11yScripts and genA11yScripts.ts to bundle and export a11y helpers as strings.
    • Introduced buildA11yInvocation and buildLocatorInvocation to inject bundles and call helpers safely.
    • Updated snapshot.ts to use generated helpers for scroll offsets, deep-active element, bounding rect, and absolute XPath.
    • Switched CSS/XPath resolution to locator scripts instead of custom inlined JS.
    • Trimmed ~180 lines of inline JS from snapshot.ts; improved debuggability and lint coverage.
    • Updated build-dom-scripts to include a11y script generation.

Written for commit a799be8. Summary will update automatically on new commits.

@changeset-bot
Copy link

changeset-bot bot commented Dec 17, 2025

⚠️ No Changeset found

Latest commit: a799be8

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

No issues found across 6 files

@seanmcguire12
Copy link
Member Author

@greptileai

bundle: true,
format: "esm",
platform: "browser",
target: "es2020",
Copy link
Collaborator

Choose a reason for hiding this comment

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

we should add a note in case we upgrade our builds to ES2022 to remember to update here

@seanmcguire12 seanmcguire12 merged commit 5e33713 into main Dec 17, 2025
19 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.

3 participants