fix(compiler-ssr): support Set model for checkbox/select v-model in SSR#15036
fix(compiler-ssr): support Set model for checkbox/select v-model in SSR#15036chatman-media wants to merge 1 commit into
Conversation
Client-side v-model already branches isArray/isSet/else for checkbox and select-multiple, but the SSR codegen and its runtime helpers only checked Array.isArray and fell through to a plain truthy check. A Set model rendered every checkbox as checked/every option as selected regardless of contents (an empty Set rendered everything checked).
📝 WalkthroughWalkthroughAdds Set support to Vue's SSR v-model codegen and runtime. A new ChangesSSR v-model Set support
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)Skipped — this PR extends existing branching logic with a new conditional path rather than introducing multi-component sequential interactions. Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/server-renderer/src/helpers/ssrVModelHelpers.ts (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffConsider centralizing the array/set/fallback resolution logic.
The array→set→fallback ternary for checkbox "checked" state is now duplicated across four sites (two here, two in
ssrVModel.tscodegen). Not a defect, but a shared runtime helper (e.g.ssrResolveModelChecked(model, value)) could reduce duplication and ease future extension (e.g. additional collection types).Also applies to: 20-26, 45-51
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/server-renderer/src/helpers/ssrVModelHelpers.ts` at line 1, The array/set/fallback checked-state resolution logic is duplicated in the v-model SSR helpers; centralize it into a shared runtime helper such as ssrResolveModelChecked(model, value) and reuse it from ssrVModelHelpers and the ssrVModel codegen paths. Update the checkbox checked-state handling in the existing helper functions (including the branches that currently use isArray, isSet, looseEqual, and looseIndexOf) to call the shared helper so the behavior stays identical while reducing duplication and making future collection-type support easier.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/server-renderer/src/helpers/ssrVModelHelpers.ts`:
- Line 1: The array/set/fallback checked-state resolution logic is duplicated in
the v-model SSR helpers; centralize it into a shared runtime helper such as
ssrResolveModelChecked(model, value) and reuse it from ssrVModelHelpers and the
ssrVModel codegen paths. Update the checkbox checked-state handling in the
existing helper functions (including the branches that currently use isArray,
isSet, looseEqual, and looseIndexOf) to call the shared helper so the behavior
stays identical while reducing duplication and making future collection-type
support easier.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 84e7473b-5832-4811-be12-922469242c57
📒 Files selected for processing (6)
packages/compiler-ssr/__tests__/ssrVModel.spec.tspackages/compiler-ssr/src/runtimeHelpers.tspackages/compiler-ssr/src/transforms/ssrVModel.tspackages/server-renderer/__tests__/ssrDirectives.spec.tspackages/server-renderer/src/helpers/ssrVModelHelpers.tspackages/server-renderer/src/internal.ts
Client-side v-model for checkbox and
<select multiple>already branches onisArray/isSet/else (seevModelCheckboxandsetSelectedinruntime-dom/src/directives/vModel.ts), but the SSR codegen incompiler-ssrand the runtime helpers inserver-rendereronly checkedArray.isArrayand fell back to treating the model as a plain truthy value otherwise. So aSetmodel on the server just diverged from the client - worst case, an emptySetrendered every single checkbox as checked, sincenew Set()is truthy.Added an
isSetbranch alongside the existing array check in both the<option>/checkbox codegen and thessrRenderDynamicModel/ssrGetDynamicModelPropsruntime helpers, mirroring the client logic. Added tests covering select-multiple with a Set, checkbox with a Set, and the empty-Set case specifically (including through the dynamic-type and v-bind codepaths) since that's the one that silently does the wrong thing.Summary by CodeRabbit
New Features
v-modelnow correctly supportsSetvalues for<select>and checkbox inputs, including multiple selection and dynamic cases.Bug Fixes
selectedandcheckedstates so items in aSetare matched by membership instead of being treated like a single value.v-modelwithv-bindand nested option structures.