refactor(core): Inline Vue ViewModel checks in normalize and safeJoin#19855
refactor(core): Inline Vue ViewModel checks in normalize and safeJoin#19855
Conversation
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| if ( | ||
| typeof value === 'object' && | ||
| value !== null && | ||
| ('__isVue' in value || '_isVue' in value || '__v_isVNode' in value) |
There was a problem hiding this comment.
Property existence check differs from original truthiness check
Low Severity
The inlined check uses '__isVue' in value || '_isVue' in value || '__v_isVNode' in value which tests property existence, while the original isVueViewModel tested property truthiness (value.__isVue || value._isVue || value.__v_isVNode). The in operator returns true even if the property value is false, 0, null, or undefined. An object with e.g. { __isVue: false } would now be incorrectly identified as a Vue ViewModel when it previously would not have been. The PR claims identical behavior, but this is a subtle semantic difference.
Additional Locations (1)
size-limit report 📦
|
node-overhead report 🧳Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.
|
The isVueViewModel() and getVueInternalName() functions from is.ts and stacktrace.ts were only called from two sites: normalize.ts (stringifyValue) and string.ts (safeJoin). Inlining the checks directly at these call sites allows tree-shaking to eliminate the standalone function definitions from bundles that pull in normalize/string but not is.ts for other reasons. Uses the `in` operator instead of `as any` casts to satisfy the linter's no-explicit-any / no-unsafe-member-access rules. The Vue check prevents infinite console warning loops when stringifying Vue 2/3 ViewModel instances or VNodes (see #8981). Saves ~15 bytes gzipped on the base browser bundle. Co-Authored-By: Claude claude@anthropic.com
1141c3c to
21a51b1
Compare
Semver Impact of This PR🟢 Patch (bug fixes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨
Bug Fixes 🐛Deps
Other
Internal Changes 🔧Core
Other
🤖 This preview updates automatically when you update the PR. |
There was a problem hiding this comment.
I'm going to close this. This might slightly improve bundle size but it drastically reduces readability. Plus this vue stuff is prone to sneak into core code, so I'd rather have one/two reusable functions we can just plug into the code path. If we reuse the function once more, it should amortize the bundle cost of declaring a function.


Summary
Inline
isVueViewModel()andgetVueInternalName()at their only two call sites. Saves ~15 bytes gzipped.Problem
isVueViewModel()andgetVueInternalName()are standalone exported functions inis.tsandstacktrace.ts, but they are only called from two places:normalize.ts(stringifyValue) andstring.ts(safeJoin). Keeping them as separate imports prevents tree-shaking from eliminating their definitions.Solution
Inline the check directly using the
inoperator (satisfiesno-explicit-any/no-unsafe-member-accesslint rules):The Vue check prevents infinite console warning loops when stringifying Vue 2/3 ViewModel instances or VNodes (see #8981). The behavior is identical.
Part of #19833.
Co-Authored-By: Claude claude@anthropic.com