Skip to content

Conversation

alex-snezhko
Copy link
Contributor

@alex-snezhko alex-snezhko commented Oct 5, 2025

close #13436

Expand cases under which the selected attribute is added onto an option tag in SSR:

  • Consider text nodes/interpolations as direct children of the option
  • Consider v-text

An additional side effect of this PR is to fix the current behavior showcased by this playground example, which I believe to be a bug: link

Example

const app = createSSRApp({
  setup() {
    const r = ref('asdf')
    const val = 'asdf'
    return { r, val }
  },
  template: `
    <select v-model="r">
      <option>{{ val }}</option>
      <option>fdsa</option>
    </select>
  `
})

// SSR output: <select><option selected>asdf</option><option>fdsa</option></select>

Summary by CodeRabbit

  • New Features

    • Improved SSR support for v-model on select elements: option values now resolve correctly from text, interpolations, loops, slots, templates, and nested structures.
  • Bug Fixes

    • More reliable selected-state computation in SSR for both single values and arrays, including cases where option values are derived rather than explicitly bound.
  • Tests

    • Added extensive SSR tests covering many select/v-model scenarios and mixed option content.

Copy link

coderabbitai bot commented Oct 5, 2025

Walkthrough

Adds SSR v-model handling for <option> by deriving option values from bound attributes or collapsed text/interpolations, updates selected-state codegen to handle array vs non-array models, and adds extensive SSR tests for <select>/<option> scenarios. No public API changes.

Changes

Cohort / File(s) Summary
SSR v-model transform
packages/compiler-ssr/src/transforms/ssrVModel.ts
Adds internal OptionValue type and findOptionValue() to resolve option value from value binding, text directives, or collapsed text/interpolations; introduces collapseTextBetweenComments() utility; updates imports to include TemplateLiteral, TextNode, createTemplateLiteral, and findDir; changes selected-state codegen to emit Array-check conditional using _ssrLooseContain vs _ssrLooseEqual.
SSR tests for v-model on select/option
packages/compiler-ssr/__tests__/ssrVModel.spec.ts
Adds many new SSR unit tests and inline snapshots covering options with literal text, trimmed/spaced text, interpolations, v-text, v-for, slots/content, optgroups, and template/conditional compositions to validate option value derivation and selected logic.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant T as Template (AST)
  participant C as SSR Compiler
  participant X as ssrVModel Transform
  participant G as Codegen
  participant H as SSR Helpers

  T->>C: parse template -> AST
  C->>X: apply v-model transform for <select>/<option>
  X->>X: findOptionValue()\n- if value binding -> bound expr (maybeArray = true)\n- else if text directive -> text node (maybeArray = false)\n- else collapse text/interpolations -> TemplateLiteral (maybeArray = false)
  X->>G: emit selected logic\n- if maybeArray -> Array.isArray(model) ? _ssrLooseContain(value) : _ssrLooseEqual(value)\n- else -> _ssrLooseEqual(value)
  G-->>T: SSR render code referencing helpers (loose contain/equal)
  Note right of H: Helpers evaluate at render time to set `selected`
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Suggested labels

ready to merge, scope: ssr, :hammer: p3-minor-bug

Poem

A rabbit in code, ears tuned to the trace,
I hop through options, sniff out their place.
Bindings or text, I tuck values in rows,
Array or single—my matching nose knows.
SSR tidy, I thump—selected shows. 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title concisely highlights the primary change to the SSR compiler’s v-model handling for option selection, accurately reflecting the expanded inclusion logic without extraneous detail.
Linked Issues Check ✅ Passed The PR updates the SSR v-model transform to derive option values from text nodes, interpolations, and v-text when no explicit value attribute is present and adjusts the generated selection logic to compare against these derived values, and it adds comprehensive tests covering these scenarios, fully addressing the behavior described in issue #13436.
Out of Scope Changes Check ✅ Passed All modifications occur within the SSR v-model transform and related test files and directly support deriving and comparing option values based on text content, with no unrelated or external changes introduced.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

github-actions bot commented Oct 5, 2025

Size Report

Bundles

File Size Gzip Brotli
runtime-dom.global.prod.js 102 kB 38.6 kB 34.8 kB
vue.global.prod.js 160 kB 58.7 kB 52.2 kB

Usages

Name Size Gzip Brotli
createApp (CAPI only) 46.7 kB 18.3 kB 16.7 kB
createApp 54.7 kB 21.3 kB 19.5 kB
createSSRApp 58.9 kB 23 kB 21 kB
defineCustomElement 60 kB 23 kB 21 kB
overall 68.8 kB 26.5 kB 24.2 kB

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c16f8a9 and fc0df0d.

📒 Files selected for processing (2)
  • packages/compiler-ssr/__tests__/ssrVModel.spec.ts (1 hunks)
  • packages/compiler-ssr/src/transforms/ssrVModel.ts (4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/compiler-ssr/src/transforms/ssrVModel.ts (3)
packages/compiler-core/src/ast.ts (9)
  • createConditionalExpression (758-772)
  • createCallExpression (728-739)
  • ExpressionNode (91-91)
  • TemplateLiteral (449-452)
  • PlainElementNode (140-149)
  • createSimpleExpression (685-698)
  • createTemplateLiteral (801-809)
  • TemplateChildNode (93-102)
  • TextNode (176-179)
packages/compiler-ssr/src/runtimeHelpers.ts (3)
  • SSR_INCLUDE_BOOLEAN_ATTR (15-17)
  • SSR_LOOSE_CONTAIN (19-19)
  • SSR_LOOSE_EQUAL (18-18)
packages/compiler-core/src/utils.ts (2)
  • findProp (299-320)
  • findDir (282-297)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: Redirect rules
  • GitHub Check: Header rules
  • GitHub Check: Pages changed
  • GitHub Check: test / unit-test-windows

Copy link

pkg-pr-new bot commented Oct 5, 2025

Open in StackBlitz

@vue/compiler-core

npm i https://pkg.pr.new/@vue/compiler-core@13963

@vue/compiler-dom

npm i https://pkg.pr.new/@vue/compiler-dom@13963

@vue/compiler-sfc

npm i https://pkg.pr.new/@vue/compiler-sfc@13963

@vue/compiler-ssr

npm i https://pkg.pr.new/@vue/compiler-ssr@13963

@vue/reactivity

npm i https://pkg.pr.new/@vue/reactivity@13963

@vue/runtime-core

npm i https://pkg.pr.new/@vue/runtime-core@13963

@vue/runtime-dom

npm i https://pkg.pr.new/@vue/runtime-dom@13963

@vue/server-renderer

npm i https://pkg.pr.new/@vue/server-renderer@13963

@vue/shared

npm i https://pkg.pr.new/@vue/shared@13963

vue

npm i https://pkg.pr.new/vue@13963

@vue/compat

npm i https://pkg.pr.new/@vue/compat@13963

commit: 15d6f92

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fc0df0d and 15d6f92.

📒 Files selected for processing (2)
  • packages/compiler-ssr/__tests__/ssrVModel.spec.ts (1 hunks)
  • packages/compiler-ssr/src/transforms/ssrVModel.ts (4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/compiler-ssr/tests/ssrVModel.spec.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/compiler-ssr/src/transforms/ssrVModel.ts (3)
packages/compiler-core/src/ast.ts (8)
  • createConditionalExpression (758-772)
  • createCallExpression (728-739)
  • ExpressionNode (91-91)
  • TemplateLiteral (449-452)
  • createSimpleExpression (685-698)
  • createTemplateLiteral (801-809)
  • TemplateChildNode (93-102)
  • TextNode (176-179)
packages/compiler-ssr/src/runtimeHelpers.ts (3)
  • SSR_INCLUDE_BOOLEAN_ATTR (15-17)
  • SSR_LOOSE_CONTAIN (19-19)
  • SSR_LOOSE_EQUAL (18-18)
packages/compiler-core/src/utils.ts (2)
  • findProp (299-320)
  • findDir (282-297)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Redirect rules
  • GitHub Check: Header rules
  • GitHub Check: Pages changed

Comment on lines +66 to +80
? createConditionalExpression(
createCallExpression(`Array.isArray`, [model]),
createCallExpression(context.helper(SSR_LOOSE_CONTAIN), [
model,
value.node,
]),
createCallExpression(context.helper(SSR_LOOSE_EQUAL), [
model,
value.node,
]),
)
: createCallExpression(context.helper(SSR_LOOSE_EQUAL), [
model,
value.node,
]),
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Restore array-model handling for text-derived option values

When an <select multiple v-model="array"> option relies on collapsed text / interpolation (no explicit value attribute), findOptionValue() sets maybeArray = false, so we never run the Array.isArray branch. As a result the SSR output compares the array model against a string template literal via ssrLooseEqual, which always fails and the selected attribute is omitted. This regresses multi-select parity for the very cases this PR extends. Please mark these derived values as maybeArray (or otherwise keep the array containment path) so multiple-selection continues to work.

🤖 Prompt for AI Agents
In packages/compiler-ssr/src/transforms/ssrVModel.ts around lines 66-80, the
generated code skips the Array.isArray containment branch for option values
derived from collapsed text/interpolation (maybeArray was set false), causing
multi-selects to never mark selected; restore array-model handling by treating
these derived text/interpolation option values as maybeArray (or otherwise
ensure the Array.isArray conditional call path is used) so the generated
expression keeps the Array.isArray ? ssrLooseContain : ssrLooseEqual logic for
those cases; implement by setting maybeArray = true for derived/text-based
option values (or by forcing the conditional expression to include the
Array.isArray branch) so SSR selected attributes are produced correctly for
multiple selects.

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.

<option> without value attr won't have selected attr in the SSR output
1 participant