dai-shi / proxy-compare

Compare two objects using accessed properties with Proxy
https://www.npmjs.com/package/proxy-compare
MIT License
283 stars 18 forks source link

May I ask why the behavior is not quite as expected? #65

Closed ityuany closed 7 months ago

ityuany commented 10 months ago

I think maybe my understanding of proxy-compare is not deep enough. I encountered a phenomenon that I couldn't understand while using it, and I hope to get your answer.

Thank you very much.

import { isChanged, createProxy } from 'proxy-compare';
const data = {
  td: {
    brandId: 1,
    tkRepositoryIds: 2,
  },
};
const affected = new WeakMap();
const p = createProxy(data, affected);
p.td.brandId;
p.td;
console.log(affected);

const res = isChanged(
  data,
  {
    td: {
      brandId: 1,
      tkRepositoryIds: 4,
    },
  },
  affected,
  new WeakMap()
);

//  I think his result should be true, but the actual result is false. I don't quite understand.
console.log(res);
dai-shi commented 10 months ago

Well, it's how it's designed. (Or, it can be said, it's the limitation with runtime solution.)

p.td.brandId;
p.td;

We treat it ☝️ the same as 👇

p.td;
p.td.brandId;

which is equivalent to 👇

p.td.brandId;

(We might be able to detect the access order to overcome this, but it would be less predictable, so the current behavior is our design decision.)

ityuany commented 10 months ago

To summarize,

I found that affected actually records both brandId and td, so they should be compared simultaneously. I don't quite understand why.


p.td;

p.td.brandId;

which is equivalent to 👇


p.td.brandId;
dai-shi commented 10 months ago

What would be your expectation of the behavior of this?

const x = p.td;
x.brandId;
ityuany commented 10 months ago

What would be your expectation of the behavior of this?

const x = p.td;
x.brandId;

The expected behavior, as I understand it, is to track td and brandId. When either td or brandId changes to any value, isChange should return false.

ityuany commented 10 months ago

For example, this has caused some confusing behavior in valtio.

valtio-demo

import { useEffect } from 'react';
import { proxy, useSnapshot } from 'valtio';

const store = proxy({
  td: {
    brandId: 1,
    tkRepositoryIds: 2,
  },
});

export default function App() {
  const snap = useSnapshot(store);

  useEffect(() => {
    console.log('effect-->', snap.td.brandId, snap.td);
  }, [snap.td, snap.td.brandId]); 

  return (
    <>
      <button
        onClick={() => {
          store.td.tkRepositoryIds = Math.random();
        }}
      >
        点我
      </button>
    </>
  );
}

👇👇👇 Because snap.td and snap.td.brandId are written in the deps of useEffect, my intuition tells me that any changes in the values of td and brandId should trigger a rerender of the component.

  useEffect(() => {
    console.log('effect-->', snap.td.brandId, snap.td);
  }, [snap.td, snap.td.brandId]); 

👇👇👇 And the following button mutates td.tkRepositoryIds, so theoretically I think td has also changed, and the component should rerender.

<button onClick={() => {
  store.td.tkRepositoryIds = Math.random();
}}>
        点我
</button>
dai-shi commented 10 months ago
const x = p.td;
x.brandId;

We can't distinguish that ☝️ from this 👇 .

p.td;
p.td.brandId;

For valtio, we consider it a limitation. https://github.com/pmndrs/valtio/blob/d1e462e14fe1d0a161c5514718ce10c1980c35ba/docs/how-tos/some-gotchas.mdx#L51 https://github.com/pmndrs/valtio/blob/d1e462e14fe1d0a161c5514718ce10c1980c35ba/docs/how-tos/some-gotchas.mdx#L135

ityuany commented 10 months ago

ok , thanks .