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

Documentation unclear of how the memoizer is used #455

Closed
Mr-Wallet opened this issue May 15, 2020 · 4 comments
Closed

Documentation unclear of how the memoizer is used #455

Mr-Wallet opened this issue May 15, 2020 · 4 comments

Comments

@Mr-Wallet
Copy link

The README describes how createSelectorCreator can be used to override the defaultMemoizer and how it will be called with the resultFunc and additional memoizeOptions. However, if my reading of the source code is correct, it's also called before calling the input functions to short-circuit their evaluation, and that isn't called with the memoizeOptions. These are important details for implementing a custom memoizer, so it seems like an oversight in the documentation.

@srubin
Copy link

srubin commented Jun 24, 2020

This bit me recently. I'd appreciate being able to specify a different memoize function for the two purposes. I guess I could do this using memoizeOptions:

function memoizeInputArgs(fn) { /* ... */ }
function memoizeResultArgs(fn) { /* ... */ }

const selector = createSelectorCreator((fn, isResultFunction) =>
  isResultFunction ? memoizeResultArgs(fn) : memoizeInputArgs(fn), true)
);

but it might be cleaner to allow passing a second memoize function to createSelectorCreator.

@srubin
Copy link

srubin commented Jun 24, 2020

I ended up with this slightly more fleshed-out version:

type MemoizeFn = (func: Function, option1?: any, option2?: any, option3?: any) => any;

export function myCreateSelectorCreator(
    memoizeResultFnArgs: MemoizeFn,
    memoizeResultFnArgsOptions: any[] = [],
    memoizeInputFnArgs: MemoizeFn = defaultMemoize,
    memoizeInputFnArgsOptions: any[] = [],
) {
    function memoize(fn: Function, areResultArgs: boolean) {
        if (areResultArgs) {
            return memoizeResultFnArgs(fn, ...memoizeResultFnArgsOptions);
        }
        return memoizeInputFnArgs(fn, ...memoizeInputFnArgsOptions);
    }
    return createSelectorCreator(
        memoize,
        true, // only passed to `memoize` for result args, not input args
    );
}

@markerikson
Copy link
Contributor

This is mostly a duplicate of #376 and related to #359 .

aryaemami59 added a commit to aryaemami59/reselect that referenced this issue Nov 11, 2023
- Add documentation regarding new features.
- Redo `CodeSandbox` examples to align better with documentation. Solves reduxjs#598.
- Fix GitHub workflow badge URL reduxjs#628.
- Add instructions for `bun` and `pnpm`. Solves reduxjs#621.
- Fix minor type issues regarding `createStructuredSelector`. Solves reduxjs#499.
- `argsMemoize` and `argsMemoizeOptions` solve reduxjs#359.
- Remove `Redux` legacy patterns including `connect` and `switch` statements from docs.  Solves reduxjs#515.
- Replace legacy code patterns with modern syntax like `useSelector` and functional components.
- Implementation of `argsMemoize` solves reduxjs#376.
- Document order of execution in `Reselect`. Solves reduxjs#455.
- Add benchmarks to confirm the info inside the documentation.
- Add more type tests with `vitest`.
- Fix more hover preview issues with types.
@aryaemami59
Copy link
Contributor

This one has been resolved by #636.

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

No branches or pull requests

5 participants