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

RE: [FR] Third argument to _.memoize - CacheConstructor #3319 #5775

Open
rcampbel opened this issue Nov 8, 2023 · 0 comments
Open

RE: [FR] Third argument to _.memoize - CacheConstructor #3319 #5775

rcampbel opened this issue Nov 8, 2023 · 0 comments

Comments

@rcampbel
Copy link

rcampbel commented Nov 8, 2023

It has been a few years since #3319 was labeled as "wont fix".

Is there any reconsideration to actual add the ability pass the cache object type to memoize rather then setting it at a global level?

Reasons to pass it in:

  1. Less chance of breaking other locations because the cache was changed at the global level
  2. Reduces errors caused by using an invalid value as key
  3. Reduce the need to be aware of the order of import because of the global memoize.cache
  4. Remove the dependency on a global value

Given the following files:
fileA.ts:

import memoize from 'lodash/memoize';
memoize.cache = WeakMap;
export const exampleA = memoize((arg:Record<string, uknown>) => Object.keys(arg));

fileB.ts:

import memoize from 'lodash/memoize';
export const exampleB = memoize((arg:string) => arg.replace('.', ' '));

fileC.ts:

import { exampleA } from 'fileA'; //memoize.cache = WeakMap
import { exampleB } from 'fileB'; //memoize.cache = WeakMap

console.log(exampleB('this.will.throw.error')); // throws: Uncaught TypeError: Invalid value used as weak map key 

fileD.ts:

import { exampleB } from 'fileB'; //memoize.cache = Map
import { exampleA } from 'fileA'; //memoize.cache = WeakMap

console.log(exampleB('this.will.not.throw.error')); // Note the inconsistent behavior of exampleB all due to the order of import

Observations based off the given files

  • Inconsistent cache type based off of order of import (fileC.ts vs fileD.ts)
  • Can create bugs by one location changing the global memoize.cache and break other locations
    • In order to fix, forces more code to be written.
      const memoizeCache = memoize.cache;
      memoize.cache = WeakMap;
      export const fetchData = memoize((id) => getDataPromise(id));
      memoize.cache = memoizeCache;
      
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

1 participant