Skip to content

fix(core): keep safeJoin side-effect-free for DOM elements (#21353)#21356

Open
archievi wants to merge 2 commits into
getsentry:developfrom
archievi:fix/safejoin-element-side-effects-21353
Open

fix(core): keep safeJoin side-effect-free for DOM elements (#21353)#21356
archievi wants to merge 2 commits into
getsentry:developfrom
archievi:fix/safejoin-element-side-effects-21353

Conversation

@archievi
Copy link
Copy Markdown

@archievi archievi commented Jun 8, 2026

Closes #21353

  • Added a regression test.
  • Code lints (oxlint) and the affected test suite passes (vitest run packages/core/test/lib/utils/string.test.ts → 29/29).
  • Linked the issue.

Summary

In 10.55.0, safeJoin (packages/core/src/utils/string.ts) switched non-primitive serialization from String(value) to stringifyValue(...). For DOM elements, stringifyValue routes through the browser stringifier's htmlTreeAsString, which reads id / className / getAttribute and walks the ancestor chain — so capturing a console breadcrumb now invokes user-defined property getters and traverses the DOM on every console.log(element), even when no event is sent. (<= 10.54.0 was side-effect-free via String(value).)

This short-circuits DOM elements in safeJoin back to a side-effect-free String(value), while keeping stringifyValue for other non-primitive values (functions, plain objects, etc.). htmlTreeAsString is intentionally left intact — it is still used for the click/DOM breadcrumb path that legitimately needs the selector string.

Added a regression test asserting safeJoin does not invoke getters or the stringifier for elements (it fails on the current code and passes with this change).

…#21353)

In 10.55.0 safeJoin switched non-primitive serialization from String() to
stringifyValue(), which for DOM elements routes through the browser stringifier's
htmlTreeAsString and reads id/className/getAttribute, invoking user-defined getters
(and walking the DOM) during console breadcrumb capture. Short-circuit elements via
String() so breadcrumb serialization stays side-effect free. Adds a regression test.
@isaacs isaacs self-requested a review June 8, 2026 18:01
Copy link
Copy Markdown
Member

@isaacs isaacs left a comment

Choose a reason for hiding this comment

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

This clearly fixes the regression described, and restores the behavior very surgically without affecting the intended behavior that led to it. Good patch, thanks!

@isaacs isaacs enabled auto-merge (squash) June 8, 2026 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants