theKashey / memoize-state

The magic memoization for the State management. ✨🧠
MIT License
329 stars 16 forks source link

Result with nested memoize functions not deproxyfied #66

Open ian-luca opened 3 years ago

ian-luca commented 3 years ago

When I try to use memoize nested the result is not correctly deproxyfied. Is there something wrong with doing

const selectSomeContext = memoize((state, contextId) => state.foo.contexts[contextId]);

const selectSomethingRequiringContext = memoize((state, contextId) => {
    const context = selectSomeContext(state, contextId);

    return state.bar[context.currentSetting].map(x => x.val);
});

I expected the result to be deproxyfied as normal, but in the above scenario the resulting array would be still wrapped in its proxy.

I know I could just do state.foo.contexts[contextId] inside selectSomethingRequiringContext, but thats only the most basic example.

theKashey commented 3 years ago

No proxy should exist outside the memoization function. Can I ask for clarification on used data types, ie what is state.foo.contexts?

ian-luca commented 3 years ago

It would be something like { [contextId: string]: { contextSetting: number } } in the provided scenario

ian-luca commented 3 years ago

Okay, I think I was a bit overexcited with posting this issue, as the example provided does not trigger the encountered error. But fortunately I was able to recreate the problem with some testing.

When only ever selecting selectSomethingRequiringContext everything works as expected, but when I want to use the nested memoize selectSomeContext standalone, after selectSomethingRequiringContext a proxy is returned instead of a result object.

Maybe memoize has to be more context aware to fully support nesting? I Imagine this will lead to cache invalidations too, as the proxied state that is handed down is considered a specific argument?

My expectation would have been that nested memoize usages respect the parent context to the point of a unified affected path. For example when a nested memoize returns a complete array of objects unfiltered from state, but the upper scope filters that array, the upper scope only lists the filtered ones as affected

theKashey commented 3 years ago

So the first step to solving any issue - is to have a "red" test. Can you codify working (I mean "not working") reproduction of a problem and post it here?

ian-luca commented 3 years ago

Here's a working (failing) example: https://codepen.io/ianluca/pen/ZEymMKz


import memoize from "https://cdn.skypack.dev/memoize-state";

interface IState {
  foo: {
    [fooIdentifier: number]: {
      barIdx: number;      
    };
  };
  bar: {
    [idx: number]: { 
      val: number;
    };
  };
}

const selectFoo = memoize((state: IState, fooIdentifier: number) => state.foo[fooIdentifier]);

const selectBarForFoo = memoize((state: IState, fooIdentifier: number) => {
    const foo = selectFoo(state, fooIdentifier);

    return state.bar[foo.barIdx];
});

const state: IState = {
  foo: { 1: { barIdx: 1 } },
  bar: { 1: { val: 2 } },
};

console.log(selectBarForFoo(state, 1));
console.log(selectFoo(state, 1));