vuejs / core

🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
https://vuejs.org/
MIT License
47.59k stars 8.32k forks source link

Reactivity VS object with non-writable non-configurable property #3024

Open Liwoj opened 3 years ago

Liwoj commented 3 years ago

Version

3.0.5

Reproduction link

https://codesandbox.io/s/vue-3-proxy-over-object-with-non-writable-non-configurable-property-56z7n?file=/src/components/HelloWorld.vue

Steps to reproduce

Create an object with non-writable, non-configurable property and wrap it with reactive() proxy

setup() {
    const inner = { a: 1, b: 2 };
    let outer = {};
    Object.defineProperty(outer, "prop", {
      value: inner,
      writable: false,
      configurable: false,
    });

    outer = reactive(outer); // coment this line to fix the issue

    return {
      outer,
    };
  },

What is expected?

Proxy returned should conform to the ECMA specification - Invariants of the Essential Internal Methods

Specifically the one for Get method:

If P was previously observed as a non-configurable, non-writable own data property of the target with value v, then [[Get]] must return the SameValue.

So the proxy returned by reactive should not make object stored in prop property reactive proxy or change it's value in any other way...

What is actually happening?

Object stored in prop property is converted into reactive proxy and browsers throw an error when accessed from template:

Firefox: TypeError: proxy must report the same value for the non-writable, non-configurable property '"prop"'

Chrome: TypeError: 'get' on proxy: property 'prop' is a read-only and non-configurable data property on the proxy target but the proxy did not return its actual value (expected '#<Object>' but got '[object Object]')

posva commented 3 years ago

You can use markRaw to tell Vue to skip any property:

Object.defineProperty(outer, "prop", {
      value: markRaw(inner),
      writable: false,
      configurable: false,
    });
Liwoj commented 3 years ago

That is nice workaround for easy example like this but what if you get some complicated (and maybe nested) structure from some 3rd party lib totally independent from Vue?

The fact that same code worked just fine in Vue 2 (leaving the prop non-reactive) is making it even worse as people will start migrating and now the code is suddenly not working and throwing strange errors they don't understand. At least some note in migration guide would be great. Warning in migration build would be even better....

snwokenk commented 3 years ago

was getting this same issue with ethersjs Provider objects. wrapping it with markRaw before assignment solved the issue. solved the error

JayWelsh commented 3 years ago

Also ran into the same issue with Ethersjs library, markRaw solved it, thanks @snwokenk

octavioamu commented 2 years ago

@snwokenk you save me, been days trying to figure out that. For those having this error 'get' on proxy: property 'provider' is a read-only .... you should wrap the provider with markRaw like:

const web3 = new Web3Provider(wallet.provider, 'any');
provider = markRaw(web3);
codeAndxv commented 1 year ago

@octavioamu thx

ESP-Marc commented 11 months ago

I was getting this problem when trying clone reactive objects for the purpose of saving an undo state.

You can't use structuredclone because you get some window error.

If you use JSON.parse(JSON.stringify(value)) you will get some unwanted conversion of empty special types outputting as NULL object and invalidating strict types.

I found the most preservative method is to use lodash-es libs cloenDeepWith combine with Vue's toRaw.

If value being cloned is a complex nested reactive object then cloneDeepWith(value, toRaw) seems to do the trick...

BenShelton commented 11 months ago

This is becoming a problem for us even using Vue 3.3.10 because of how an external library generates code. More specifically https://github.com/timostamm/protobuf-ts. The code it generates uses some cases of defineProperty to create reflection information.

This isn't something we can easily overwrite to put markRaw everywhere, as we'd have to do some fancy search-replace on the generated code to replace everywhere this appears in every class that is generated (which is a lot for us!) Also we use these definitions in both Vue and Node code, so we'd have to generate 2 copies of these definitions, one with markRaw and one without, which is not a viable workaround.

This causes issues because we want to make these classes reactive, which does work, but as soon as we try to read this reflection information we get these errors and it isn't available to us. It's also a pain in unit tests, because when jest tries to diff between the actual/expected values, if either was made reactive then the diff will fail with the same error and we can't actually see what was wrong.

The possible fix for this was closed for an unknown reason, can that be revisited? (see https://github.com/vuejs/core/pull/3025)

soletan commented 10 months ago

Using markRaw isn't a solution, even in the OPs use case, according to the official documentation of markRaw and the warning given there. In example given before, an inner property is to be marked as raw prior to making the containing object reactive. Hence, the markRaw is lost according to the very similar example in the documentation.

I'm running into this issue as part of a library when trying to prevent expensive conversions of data into its reactive counterpart by tracking the provided data and the results in a weak map for runtime caching. A single unique object is assigned to the source and later also to the result so that multiple provisions of the same source can detect a cached promise for the pending conversions as well as provisions of a result can find additional data that has been put in cache next to the result.

However, due to this bug all this caching doesn't work for the cache key - that unique object - isn't kept raw I assume.

IMHO there is no workaround as of now.

shining-mind commented 6 months ago

We are facing the same issue, and neither markRaw nor toRaw is a viable solution for us. Additionally, we have our own proxy system that correctly handles this scenario, and we haven't observed any negative impact on performance. Therefore, I think that #3025 should be revisited.

By the way, here is our implementation of the get trap: https://github.com/V4Fire/Core/blob/fe6d90858e089cec1f157bbbecdb39d5823bd97e/src/core/object/watch/engines/proxy.ts#L254

ultimateshadsform commented 1 month ago

Using three.js with vue 3 is a mess.