From 471fd22beff19b1894d4be8ecb7f799857e5a75a Mon Sep 17 00:00:00 2001 From: Richie Bendall Date: Tue, 20 Apr 2021 01:57:44 +1200 Subject: [PATCH 1/4] Don't memoize functions across instances when `.decorator()` is used Signed-off-by: Richie Bendall --- index.ts | 34 ++++++++++++++++++++++++++++++++-- readme.md | 9 ++++++++- test.ts | 28 ++++++++++++++++++++++++---- 3 files changed, 64 insertions(+), 7 deletions(-) diff --git a/index.ts b/index.ts index dc6c59e..2afc59d 100644 --- a/index.ts +++ b/index.ts @@ -4,6 +4,8 @@ import mapAgeCleaner = require('map-age-cleaner'); type AnyFunction = (...arguments_: any) => any; +const decoratorInstanceMap = new WeakMap(); + const cacheStore = new WeakMap(); interface CacheStorageContent { @@ -66,6 +68,15 @@ interface Options< readonly cache?: CacheStorage>; } +interface DecoratorOptions { + /** + Whether the same memoized function should be used in different instances or a different one should be initialised for each one. + + @default false + */ + readonly isAcrossInstances?: boolean; +} + /** [Memoize](https://en.wikipedia.org/wiki/Memoization) functions - An optimization used to speed up consecutive function calls by caching the result of calls with identical input. @@ -170,7 +181,10 @@ mem.decorator = < FunctionToMemoize extends AnyFunction, CacheKeyType >( - options: Options = {} + { + isAcrossInstances = false, + ...options + }: Options & DecoratorOptions = {} ) => ( target: any, propertyKey: string, @@ -182,7 +196,23 @@ mem.decorator = < throw new TypeError('The decorated value must be a function'); } - descriptor.value = mem(input, options); + if (isAcrossInstances) { + descriptor.value = mem(input, options); + return; + } + + delete descriptor.value; + delete descriptor.writable; + + descriptor.get = function () { + if (!decoratorInstanceMap.has(this)) { + const value = mem(input, options); + decoratorInstanceMap.set(this, value); + return value; + } + + return decoratorInstanceMap.get(this); + }; }; /** diff --git a/readme.md b/readme.md index 5a4ac37..d57946a 100644 --- a/readme.md +++ b/readme.md @@ -204,7 +204,14 @@ Returns a TypeScript decorator which memoizes the given function. Type: `object` -Same as options for `mem()`. +Same as options for `mem()` and the following: + +##### isAcrossInstances + +Type: `boolean`\ +Default: `false` + +Whether the same memoized function should be used in different instances or a different one should be initialised for each one. ```ts import mem = require('mem'); diff --git a/test.ts b/test.ts index edb8733..9e67e9c 100644 --- a/test.ts +++ b/test.ts @@ -238,12 +238,12 @@ test('prototype support', t => { }); test('.decorator()', t => { - class TestClass { - index = 0; + let returnValue = 1; + class TestClass { @mem.decorator() counter() { - return ++this.index; + return returnValue; } } @@ -251,8 +251,28 @@ test('.decorator()', t => { t.is(alpha.counter(), 1); t.is(alpha.counter(), 1, 'The method should be memoized'); + returnValue++; + const beta = new TestClass(); - t.is(beta.counter(), 1, 'The method should not be memoized across instances'); + t.is(beta.counter(), 2, 'The method should not be memoized across instances'); + + returnValue++; + + class TestClass2 { + @mem.decorator({isAcrossInstances: true}) + counter() { + return returnValue; + } + } + + const charlie = new TestClass2(); + t.is(charlie.counter(), 3); + t.is(charlie.counter(), 3, 'The method should be memoized'); + + returnValue++; + + const delta = new TestClass2(); + t.is(delta.counter(), 3, 'The method should be memoized across instances'); }); test('mem.clear() throws when called with a plain function', t => { From 33b94305f27a32a68cc07e8f7c61fa0bad360e87 Mon Sep 17 00:00:00 2001 From: Richie Bendall Date: Tue, 20 Apr 2021 23:25:35 +1200 Subject: [PATCH 2/4] Remove `isAcrossInstances` Signed-off-by: Richie Bendall --- index.ts | 21 ++------------------- readme.md | 11 ++--------- test.ts | 18 ------------------ 3 files changed, 4 insertions(+), 46 deletions(-) diff --git a/index.ts b/index.ts index 2afc59d..44c0080 100644 --- a/index.ts +++ b/index.ts @@ -68,15 +68,6 @@ interface Options< readonly cache?: CacheStorage>; } -interface DecoratorOptions { - /** - Whether the same memoized function should be used in different instances or a different one should be initialised for each one. - - @default false - */ - readonly isAcrossInstances?: boolean; -} - /** [Memoize](https://en.wikipedia.org/wiki/Memoization) functions - An optimization used to speed up consecutive function calls by caching the result of calls with identical input. @@ -152,7 +143,7 @@ const mem = < export = mem; /** -@returns A TypeScript decorator which memoizes the given function. +@returns A [decorator](https://github.com/tc39/proposal-decorators) to memoize class methods or static class functions. @example ``` @@ -181,10 +172,7 @@ mem.decorator = < FunctionToMemoize extends AnyFunction, CacheKeyType >( - { - isAcrossInstances = false, - ...options - }: Options & DecoratorOptions = {} + options: Options = {} ) => ( target: any, propertyKey: string, @@ -196,11 +184,6 @@ mem.decorator = < throw new TypeError('The decorated value must be a function'); } - if (isAcrossInstances) { - descriptor.value = mem(input, options); - return; - } - delete descriptor.value; delete descriptor.writable; diff --git a/readme.md b/readme.md index cc42bc7..ec928c2 100644 --- a/readme.md +++ b/readme.md @@ -198,7 +198,7 @@ Refer to the [caching strategies](#caching-strategy) section for more informatio ### mem.decorator(options) -Returns a [decorator](https://github.com/tc39/proposal-decorators) to memoize class methods. +Returns a [decorator](https://github.com/tc39/proposal-decorators) to memoize class methods or static class functions. Notes: @@ -210,14 +210,7 @@ Notes: Type: `object` -Same as options for `mem()` and the following: - -##### isAcrossInstances - -Type: `boolean`\ -Default: `false` - -Whether the same memoized function should be used in different instances or a different one should be initialised for each one. +Same as options for `mem()`. ```ts import mem = require('mem'); diff --git a/test.ts b/test.ts index 9e67e9c..b34996c 100644 --- a/test.ts +++ b/test.ts @@ -255,24 +255,6 @@ test('.decorator()', t => { const beta = new TestClass(); t.is(beta.counter(), 2, 'The method should not be memoized across instances'); - - returnValue++; - - class TestClass2 { - @mem.decorator({isAcrossInstances: true}) - counter() { - return returnValue; - } - } - - const charlie = new TestClass2(); - t.is(charlie.counter(), 3); - t.is(charlie.counter(), 3, 'The method should be memoized'); - - returnValue++; - - const delta = new TestClass2(); - t.is(delta.counter(), 3, 'The method should be memoized across instances'); }); test('mem.clear() throws when called with a plain function', t => { From ff911a320311cf0af2e02a185c43f467c928548f Mon Sep 17 00:00:00 2001 From: Richie Bendall Date: Tue, 20 Apr 2021 23:42:56 +1200 Subject: [PATCH 3/4] Update index.ts Co-authored-by: Sindre Sorhus --- index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/index.ts b/index.ts index 44c0080..7961784 100644 --- a/index.ts +++ b/index.ts @@ -143,7 +143,7 @@ const mem = < export = mem; /** -@returns A [decorator](https://github.com/tc39/proposal-decorators) to memoize class methods or static class functions. +@returns A [decorator](https://github.com/tc39/proposal-decorators) to memoize class methods or static class methods. @example ``` From 3b360e8d786fa00cf873ae62a433fab2a97ca3c3 Mon Sep 17 00:00:00 2001 From: Richie Bendall Date: Tue, 20 Apr 2021 23:43:22 +1200 Subject: [PATCH 4/4] Update readme.md --- readme.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readme.md b/readme.md index ec928c2..2d6377e 100644 --- a/readme.md +++ b/readme.md @@ -198,7 +198,7 @@ Refer to the [caching strategies](#caching-strategy) section for more informatio ### mem.decorator(options) -Returns a [decorator](https://github.com/tc39/proposal-decorators) to memoize class methods or static class functions. +Returns a [decorator](https://github.com/tc39/proposal-decorators) to memoize class methods or static class methods. Notes: