theKashey / proxyequal

There is a bit more smart way to compare things, than a shallow equal.
MIT License
72 stars 7 forks source link

TypeError: 'get' on proxy: property 'byId' 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]') #25

Open brunolemos opened 4 years ago

brunolemos commented 4 years ago

Hey, I'm experimenting using memoize-state as an alternative to reselect on some selectors and I'm getting this error:

TypeError: 'get' on proxy: property 'byId' 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]')

According to the stack trace, the error is coming from the get method of the proxyequal proxy.

Here's a small reproducible without using any libraries (from this stackoverflow answer):

var obj = {};
Object.defineProperty(obj, "prop", { 
  configurable: false,
  value: {},
});

var p = new Proxy(obj, {
  get(target, property, receiver) {
    return new Proxy(Reflect.get(target, property, receiver), {});
  },
});

var val = p.prop;

Some observations:

I found this pull request which contains a fix, maybe it would work here as well. Here's the code.

If I understood correctly proxyequal needs to handle this case. @theKashey What do you think?

theKashey commented 4 years ago

Proposed "fix" would prevent result from being deeper proxied and cut the tracking of the internal information. From the "browser" prospective the error is valid, and you don't have to deep proxy readonly, and especially non-configurable data, but that's not true (immer unfreezes objects later)

However, I could guarantee that immer is not a root cause - proxyequal and immer are playing well together. Please check your 28 times render.

Anyway, look like I have to implement this moment. It should not throw.

theKashey commented 4 years ago

There are some bugs within immer, I mean - I have them in production, especially with "freeze"(and IE11) stuff, but every time I've tried to create an isolated example to create an issue for @mweststrate - everything works perfectly. Looking back to the immer history - there are many issues around this, and they were closed multiple times. I think there is some edge case, a combination of many factors, you (and I) are somehow experiencing. I failed to find the root cause, and just removed immer for single one operation - array.filter on one reducer. I hope your will find your one, or find the way to mitigate it.

Something like...

const removeFetching = (list, action, request) => {
  list[action] = list[action]
    // elements would we wrapped in immer proxy - ask for the original value
    .filter(d => original(d) !== request);
   // 👆return immer proxies to the original state
   // sometimes
};
.....
produce(state, draft => {
      merge(draft, action.response);
-      removeFetching(draft.fetching, action.endpoint, action.data);
});
+ removeFetching(state.fetching, action.endpoint, action.data);

(and of course it works at code sandbox)

brunolemos commented 4 years ago

I'm still investigating, but disabling the immer auto freeze feature made the error go away!

import { setAutoFreeze } from 'immer'

setAutoFreeze(false)

immer unfreezes objects later

I don't think so based on this comment and this code example:

const { produce } = require('immer')

const obj = { a: 1 }
console.log(Object.getOwnPropertyDescriptors(obj))
// { a: { value: 1, writable: true, enumerable: true, configurable: true } }

const objAfterImmer = produce(obj, () => {})
console.log(Object.getOwnPropertyDescriptors(objAfterImmer))
// { a: { value: 1, writable: false, enumerable: true, configurable: false } }
theKashey commented 4 years ago

immer unfreezes objects later Ah, yeah, "immutability". But now I am a bit worried why it does work with auto-freeze for me.

brunolemos commented 4 years ago

But now I am a bit worried why it does work with auto-freeze for me.

It’s possible it was caused by using immer on the top level reducer. The issue seemed to happen only after my CLEANUP action runs (which is at the root reducer).

dai-shi commented 3 years ago

Hey guys, we faced the same error with react-tracked + immer v8. I don't know how it was prior to immer v8.

https://github.com/dai-shi/proxy-compare/pull/8 The fix seems unfortunate, but hope it is the correct way.