fix(reactivity): respect readonly during ref unwrapping #13905
Conversation
WalkthroughAdjusts BaseReactiveHandler.get to unwrap refs differently on readonly proxies: when accessing a ref, derive the value (preserving array-index return of the Ref). If the proxy is readonly and the derived value is an object, return a readonly-wrapped version; otherwise return the value. Adds a test asserting nested ref inside readonly is considered readonly. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User Code
participant P as Readonly Proxy
participant H as BaseReactiveHandler.get
participant R as Ref
participant RO as readonly()
U->>P: access target[key]
P->>H: get(target, key, receiver)
alt resolved value is Ref
H->>R: inspect ref (res)
alt target is array AND key is integer
note right of H #E8F0FF: preserve array-index behavior
H-->>P: return Ref (res)
else
H->>R: const value = res.value
alt proxy is readonly AND value is object
H->>RO: readonly(value)
RO-->>H: readonly-wrapped object
H-->>P: return readonly-wrapped object
else
H-->>P: return value
end
end
else
H-->>P: existing get behavior (unchanged)
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)packages/reactivity/__tests__/readonly.spec.ts (2)
🔇 Additional comments (2)
Comment |
Size ReportBundles
Usages
|
@vue/compiler-core
@vue/compiler-dom
@vue/compiler-sfc
@vue/compiler-ssr
@vue/reactivity
@vue/runtime-core
@vue/runtime-dom
@vue/server-renderer
@vue/shared
vue
@vue/compat
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/reactivity/src/baseHandlers.ts (1)
122-123: Check behavioral change: readonly(arrayOfRefs)[i] becomes a readonly-wrapped RefFor array + integer keys,
valueis the Ref itself, and we now returnreadonly(ref). That prevents writes to.value(warns/no-op). If this change is intentional, please add tests to codify it; if not, guard against wrapping Refs:- return isReadonly && isObject(value) ? readonly(value) : value + return isReadonly && isObject(value) && !isRef(value) + ? readonly(value) + : valueSuggested tests:
readonly({ a: ref([]) }).a.push(...)warns and no mutation (bug repro).readonly([ref(0)])[0].value = 1— assert expected behavior per your decision (warn/no-op vs allowed).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/reactivity/src/baseHandlers.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/reactivity/src/baseHandlers.ts (3)
packages/reactivity/src/ref.ts (7)
value(123-134)value(136-158)value(307-309)value(311-313)value(360-363)value(365-367)value(380-382)packages/shared/src/general.ts (2)
isIntegerKey(77-81)isObject(53-54)packages/reactivity/src/reactive.ts (2)
isReadonly(336-338)readonly(205-215)
⏰ 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
🔇 Additional comments (1)
packages/reactivity/src/baseHandlers.ts (1)
120-124: LGTM: readonly now applied to unwrapped ref values on readonly proxiesThis fixes the reported mutation via
pushonreadonly({ myArray: ref([]) })by returning a readonly-wrapped array/object from ref properties. Behavior matches deep-readonly expectations while preserving array-index non-unwrapping.
edison1105
added
ready to merge
|
/ecosystem-ci run |
|
📝 Ran ecosystem CI: Open
|
close #13903
Summary by CodeRabbit