Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

why not Reorder getTargetType check after proxyMap lookup in createReactiveObject #12807

Open
Marchcn-Ben opened this issue Feb 3, 2025 · 1 comment · May be fixed by #12824
Open

why not Reorder getTargetType check after proxyMap lookup in createReactiveObject #12807

Marchcn-Ben opened this issue Feb 3, 2025 · 1 comment · May be fixed by #12824
Labels
🔨 p3-minor-bug Priority 3: this fixes a bug, but is an edge case that only affects very specific usage. scope: reactivity

Comments

@Marchcn-Ben
Copy link

Marchcn-Ben commented Feb 3, 2025

Vue version

3.4.30

Link to minimal reproduction

https://codesandbox.io/p/devbox/vigilant-lamarr-45l4f9

Steps to reproduce

  1. Create a reactive object a using reactive({ a: 1 }).
  2. Create another reactive object b using reactive({ b: 2 }).
  3. Assign markRaw(toRaw(a)) to b.a.
  4. Check if b.a === a using strict equality (===).
import { reactive, markRaw, toRaw, isReactive } from 'vue';

const a = reactive({ a: 1 });
const b = reactive({ b: 2 });
b.a = markRaw(toRaw(a));

console.log(b.a === a); 

What is expected?

Since markRaw is used to mark toRaw(a) as a raw (non-reactive) object, b.a should return the original raw object a.
Therefore, b.a === a should return false, as b.a should not reference the same reactive proxy a.

What is actually happening?

but now b.a === a returns true

System Info

System:
    OS: macOS 15.1.1
    CPU: (8) arm64 Apple M2
    Memory: 294.02 MB / 16.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 22.12.0 - /opt/homebrew/bin/node
    npm: 10.9.0 - /opt/homebrew/bin/npm
  Browsers:
    Chrome: 132.0.6834.160
    Safari: 18.1.1

Any additional comments?

Suggestion: Reorder getTargetType check before proxyMap lookup in createReactiveObject.

While reading the Vue 3 source code, I noticed that in the createReactiveObject function, the proxyMap lookup is performed before the getTargetType check. I believe reordering these steps could improve both the logical clarity and performance in certain edge cases.

The current implementation prioritizes the proxyMap lookup, likely to optimize for the common case where the target is already reactive.
However, reordering the steps would make the function more robust and slightly more efficient for edge cases (e.g., invalid targets).
This change is backward-compatible and does not introduce any breaking changes.

I would appreciate feedback from the Vue core team on whether this change aligns with the design goals of Vue's reactivity system. If this suggestion is accepted, I would be happy to submit a pull request with the proposed changes.

@edison1105
Copy link
Member

This should be considered as a bug.
PR welcome.

@edison1105 edison1105 added scope: reactivity 🔨 p3-minor-bug Priority 3: this fixes a bug, but is an edge case that only affects very specific usage. labels Feb 3, 2025
FatRadish pushed a commit to FatRadish/core that referenced this issue Feb 8, 2025
FatRadish pushed a commit to FatRadish/core that referenced this issue Feb 8, 2025
FatRadish pushed a commit to FatRadish/core that referenced this issue Feb 8, 2025
FatRadish pushed a commit to FatRadish/core that referenced this issue Feb 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 p3-minor-bug Priority 3: this fixes a bug, but is an edge case that only affects very specific usage. scope: reactivity
Projects
None yet
2 participants