Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Result with nested memoize functions not deproxyfied #66

Open
ian-luca opened this issue Sep 20, 2021 · 5 comments
Open

Result with nested memoize functions not deproxyfied #66

ian-luca opened this issue Sep 20, 2021 · 5 comments

Comments

@ian-luca
Copy link

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
Copy link
Owner

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
Copy link
Author

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

@ian-luca
Copy link
Author

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
Copy link
Owner

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
Copy link
Author

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));

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants