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

pMemoizeDecorator is using the same cache instance when using a custom cache #46

Closed
mdawar opened this issue Jun 11, 2022 · 0 comments
Closed

Comments

@mdawar
Copy link
Contributor

mdawar commented Jun 11, 2022

Since decorators are called on the class declaration and not instantiation, the same cache instance is being used by all the class instances when using the custom cache option for pMemoizeDecorator.

Here's a failing test that shows what I mean:

test('.pMemoizeDecorator() with custom cache', async (t) => {
  let returnValue = 1;

  class TestClass {
    @pMemoizeDecorator({ cache: new Map() })
    async counter() {
      return returnValue++;
    }
  }

  const alpha = new TestClass();
  t.is(await alpha.counter(), 1);
  t.is(await alpha.counter(), 1, 'The method should be memoized');

  const beta = new TestClass();
  t.is(await beta.counter(), 2, 'The method should not be memoized across instances');
});

The test output:

.pMemoizeDecorator() with custom cache

The method should not be memoized across instances

Difference:

- 1
+ 2

› file://test.ts:213:4

─

1 test failed

The output shows that the same cache instance is being used for all the TestClass instances, which is the expected behavior because the pMemoizeDecorator is being called once on the class declaration, so the same options object is being passed to pMemoize.

I've created this issue to discuss the options to solve this problem before creating a pull request.

The first solution that comes to my mind is accepting a function for the cache option that returns an instance:

interface Options<FunctionToMemoize extends AnyAsyncFunction, CacheKeyType> {
  readonly cache?:
    | CacheStorage<CacheKeyType, AsyncReturnType<FunctionToMemoize>>
    | (() => CacheStorage<CacheKeyType, AsyncReturnType<FunctionToMemoize>>);
}
@mdawar mdawar closed this as completed Jun 20, 2022
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

1 participant