Reactive functions do not correctly recognize when an object's extensibility changes
Version
3.2.31
Reproduction link
Steps to reproduce
const original = {}
const foo = reactive(original)
const fooIsReactive = isReactive(foo)
Object.preventExtensions(original)
const bar = reactive(original)
// An error occurred due to proxyMap
const barIsReactive = isReactive(bar)
What is expected?
barIsReactive === false
What is actually happening?
barIsReactive === true because of proxyMap cache
Why do you consider this to be a bug?
When the underlying object has existing properties, those can still be changed (like the original), and the reactive proxy can be used to observe those changes.
When someone tries to add a new property, they will get an error. as is expected from a non-extensible object.
Why do you consider this to be a bug?
When the underlying object has existing properties, those can still be changed (like the original), and the reactive proxy can be used to observe those changes.
When someone tries to add a new property, they will get an error. as is expected from a non-extensible object.
I take your point, but at the same time, what I don't understand is why shouldn't a non-extensible object be set up as a reactive object.
what I don't understand is why shouldn't a non-extensible object be set up as a reactive object.
That's not my point, is there a misunderstanding?
I'm saying it's fine to do so. And I don't see any problems in the SFC demo you provided.
// case 1
const original = {existingProp:''}
const foo = reactive(original)
Object.preventExtensions(original)
const bar = reactive(original) // bar is reactive obj
// case 2
const original = {existingProp:''}
Object.preventExtensions(original)
const foo = reactive(original) // foo is not reactive obj
// It's not a problem , but behavior inconsistency
// Maybe I'm fastidious, sorry about that.
// I know it won't have a bad effect.
I have the same confusion. Intuitively, reactive(non-extensible object)
should not get a reactive object.
That's why I submitted that PR.
Thanks, now I get it ☺️
I was confused because I wasn't aware that case 2 exists.
So yes, the behavior should likely be the same for both.
I'd personally rather see both to support reactive,. But it seems the original intention was to not Support that.
I don't think this behavior should be changed. There are two conflicting desired behavior here at play:
- Calling
reactive
on the same object should always return the same proxy, if it already exists. - Calling
reactive
on an object with an existing proxy, but later changed to be non-extensible, should return the object itself instead of the proxy.
(2) is really a contrived edge case that I don't really see a real world case for, whereas (1) is a common rule that many users are used to and is actually useful. Therefore I believe it's better to keep (1) rather than changing to for (2).