ref(browser): Extract browser-specific normalize code out of core#21172
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
size-limit report 📦
|
mydea
force-pushed
the
fn/move-convertToPlainObject
branch
2 times, most recently
from
9729b8a to
fbe2a21
Compare
mydea
changed the title
normalize code into registerable utilnormalize code out of core
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 02da5dd. Configure here.
mydea
force-pushed
the
fn/move-convertToPlainObject
branch
from
87b679e to
36cca78
Compare
Lms24
left a comment
Lms24
left a comment
There was a problem hiding this comment.
Nice! I think this makes things more targeted and cleaner.
One concern, but I think this is something we can address if it comes up: Us pulling out the Vue/React specific stuff into the respective framework SDKs is cleaner (fully agree!) but also means anyone using the default browser SDK (loader/CDN especially) on React/Vue apps won't get the specific functionality. Again, probably fine to fix if it becomes a real issue. The fix would be that we simply pull the added functionality into the browser-utils core version. We might need to eat the bundle size/minimal perf overhead increase at this point.
| * Vue ViewModels and React SyntheticEvents are not handled here — the Vue and React | ||
| * SDKs wrap this function in their `init` and add their own checks on top. | ||
| */ | ||
| export function normalizeStringifyValue(value: Exclude<unknown, string | number | boolean | null>): string | undefined { |
There was a problem hiding this comment.
l: I might be missing something obvious but can we hoist this up into @sentry/browser? I don't see it being imported by packages that don't depend on browser (as opposed to htmlTreeAsString)
There was a problem hiding this comment.
good point, this makes sense 👍
I agree, but I think in this case it is fine because mostly this is additive stuff (and a fix for a weird vue bug that may/may not be around still, sadly we don't really have integration/e2e tests where we know this is covered...), so it feels fine to say you only get this if using the proper SDK. E.g. for react, this will just be serialized a bit differently which is also not the end of the world I'd say :) |
Relocates `normalizeStringifyValue` (and its test) out of `@sentry-internal/browser-utils` into `@sentry/browser`, exports it from the browser package's public surface, and switches `@sentry/vue` and `@sentry/react` to import from `@sentry/browser` instead. Both framework packages no longer need a direct dependency on `@sentry-internal/browser-utils`, so that entry is dropped from their `package.json` files. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
mydea
requested review from
logaretm,
nicohrubec and
s1gr1d
and removed request for
a team
2 participants
Summary
Today,
@sentry/corecarries browser-specific code that runs (or just ships) in every runtime:window/document/HTMLElementcollapsing insidenormalize(), Vue ViewModel detection, React SyntheticEvent detection, andhtmlTreeAsString(DOM walk). This PR moves all of that into the packages that actually need it, leaving@sentry/coremore runtime-agnostic (there are still a bunch of other things, but step by step...), and exposes a single new hook for SDKs to plug runtime-specific rendering intonormalize().Mechanism
packages/core/src/utils/normalize.tsnow exposes:normalize()'s internalstringifyValueconsults the registered function before its runtime-agnostic fallbacks (NaN / function / symbol / bigint /[object ConstructorName]). Returning a string short-circuits; returningundefinedfalls through. Default state is no stringifier — server-only consumers never reach this code path.What moved
htmlTreeAsString@sentry-internal/browser-utils@deprecatedisElement@sentry-internal/browser-utils@deprecatedisSyntheticEvent@sentry/react(internal)@deprecatedisVueViewModel@sentry/vue(internal)@deprecatedgetVueInternalName@sentry/vue(internal)@deprecated@sentry-internal/browser-utilsalso gains a newnormalizeStringifyValuethat handleswindow→[Window],document→[Document], andHTMLElementinstances →[HTMLElement: <css-selector-path>](viahtmlTreeAsString). It explicitly does not handle Vue or React values — those are added by their respective SDKs.SDK wiring
@sentry/browserinit()callssetNormalizeStringifier(normalizeStringifyValue)(the browser-utils variant).@sentry/vueinit()runs afterbrowserInit, then registers a wrapper that checksisVueViewModeland otherwise delegates to browser-utilsnormalizeStringifyValue.@sentry/reactinit()does the same forisSyntheticEvent.Other adjustments
safeJoinin core now usesstringifyValuefrom normalize instead of having special handling for vue, this should streamline things a bit. as a sideeffect, we also stringify certain things better now, e.g. html elements etc. get the nice tree format in browserTests
packages/core/test/lib/utils/normalize.test.ts— covers registry semantics: stub stringifier is consulted for every visited node, falls back to defaults when it returns undefined, behaves correctly with no stringifier registered.packages/core/test/lib/utils/string.test.ts— newsafeJoin()block covers primitive/non-primitive routing, default delimiter, non-array input, Error rendering, and interaction withsetNormalizeStringifier.packages/browser-utils/test/normalizeStringifyValue.test.ts— direct and integration tests for window / document / HTMLElement, plus confirmation that Vue/React values are not intercepted.packages/vue/test/integration/normalize.test.ts— verifies Vue init's wrapper collapses Vue 2/3 ViewModels and VNodes while still delegating HTMLElement /documentto the browser variant underneath.packages/react/test/normalize.test.ts— same for React's SyntheticEvent wrap.🤖 Generated with Claude Code