Error when call `createProxy` twice with the same but frozen object using `targetCache`

[StepanDyubin]

Hi! I caught an error when trying to pass targetCache.

Problem appears if call createProxy with a regular object, then with the same (by link) but frozen object.

const targetCache = new WeakMap();

const original = { list: [{ i: 10 }, { i: 20 }] };
const proxy1 = createProxy(original, new WeakMap(), undefined, targetCache);

Object.freeze(original);

const proxy2 = createProxy(original, new WeakMap(), undefined, targetCache);
console.log(proxy2.list);

Error:

'get' on proxy: property 'list' is a read-only and non-configurable data property on the proxy target but the proxy did not return its actual value (expected '[object Array]' but got '[object Array]')

https://codesandbox.io/p/sandbox/proxy-compare-test-forked-ksskgg

First time we call createProxy, it stores target without a copy. And then if call createProxy with frozen object, it hits in to targetCache and receives that target without copy. https://github.com/dai-shi/proxy-compare/blob/1c0f138cbaa5bb0dd736f305d43e74bb1ecf0c74/src/index.ts#L209C1-L219C1

In my case I can fix it if I guarantee that an object is frozen when call createProxy first time. But you can fix it if check needsToCopyTargetObject before get from the targetCache.

Example of solution:

const target = getOriginalObject(obj);
let copied;

if (needsToCopyTargetObject(target)) {
   [, copied] = targetCache && (targetCache as TargetCache<typeof obj>).get(obj);

  if (!copied) {
     copied = copyTargetObject(target);
  }

  targetCache?.set(obj, [target, copied]);
}

Do you think is there a reason to fix that case at your library? I can open a PR if you are.

[dai-shi]

Thanks for reporting.

I can't guarantee if a PR will be accepted, but if you are fine with it, can you open one please? Is it possible to add a failing test case first?

I'm also curious if your proposal is about omitting target cache if needsToCopyTargetObject returns false. So, everything isn't yet clear. We also need to run tests with valtio and others.

[StepanDyubin]

Hello and thanks for the answer!

I opened a PR with failing test case and my solution. https://github.com/dai-shi/proxy-compare/pull/71/files

It works fine in my case. And I'll be very glad if you can check this for your cases.

Also if you have any doubts or questions, I'm ready to help you.

[dai-shi]

Thanks. I'm busy at the moment, and will take a look later. I will also run tests in react-tracked, valtio, and proxy-memoize.

[dai-shi]

In my case I can fix it if I guarantee that an object is frozen when call createProxy first time.

After reviewing #71, I think your workaround seems reasonable. Or, you can delete it from the target cache when you call Object.freeze.

Or, create a wrapper:

import { createProxy as createProxyOrig } from 'proxy-compare';

export const createProxy = <T>(                                                
  obj: T,                                                                     
  affected: WeakMap<object, unknown>,                                          
  proxyCache?: WeakMap<object, unknown>,        
  targetCache?: WeakMap<object, unknown>,
): T => {
  if (targetCache?.has(obj) && Object.isFrozen(obj) && !Object.isFrozen(targetCache.get(obj)[0])) {
    targetCache.delete(obj)
  }
  return createProxyOrig(obj, affected,proxyCache, targetCache);
};