theKashey / proxyequal

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

Is this an expected behavior of proxyMap #12

Closed dai-shi closed 5 years ago

dai-shi commented 5 years ago

Hi, I'm trying to improve https://github.com/dai-shi/react-hooks-easy-redux with proxyMap, but it doesn't behave as I expect. Do I misunderstand the usage?

$ node 
> const { proxyState } = require('proxyequal')
undefined
> const proxyMap = new WeakMap()
undefined
> const state = { a: 1, b: 2 }
undefined
> const trapped1 = proxyState(state, null, proxyMap)
undefined
> trapped1.state.a
1
> trapped1.affected
[ '.a' ]
> const trapped2 = proxyState(state, null, proxyMap)
undefined
> trapped2.state.b
2
> trapped2.affected
[]
> trapped1.affected
[ '.a', '.b' ]

I was expecting trapped2.affected to be ['.b'] and trapped1.affected to be kept ['.a'].

theKashey commented 5 years ago

Yeah. It should be this way, and should not.

 state = { a, b }
 // it should maintain ref equality
 newState.a === oldState.b

So a would be the same Proxy for both instances. That should be that way. And it would have callbacks, defined for the first use, not the second. That should not be that way.

It's a bit complicated, but one have to return the same objects, with a different setup. And I am not sure it is possible.

To be more concrete - for this case it is possible - push "path" on component access, and pop it just after. So.any.deep.property access would work.

Once you store variable somewhere const {a:oldA}=oldState; const {a: newA } = newState - information is lost.

Probably:

  1. Push/pop the right accesser on access.
  2. Update all active proxies on unprefixed access.

Look like .2 would fix the current buggy behaviour, then you lose the track, and .1 would make tracking more accurate.

PS: Thank you for an issue.

dai-shi commented 5 years ago

Complicated... but I understand the object identity. Let me try caching trapped in the front. Thanks!

dai-shi commented 5 years ago

Here's what I did: https://github.com/dai-shi/react-hooks-easy-redux/commit/23e2c5ab75fed0f36787358ea52af12bb48f1697

The benchmark result seems good: https://github.com/dai-shi/react-hooks-easy-redux/issues/3#issuecomment-466833745