memDecorator calls getters immediately

[neodon]

Hello,

I'm running into an issue memoizing a getter that relies on configuration set in the constructor. It appears that this line in memDecorator calls the getter immediately in addition to retrieving the getter function:

const input = target[propertyKey]; // eslint-disable-line @typescript-eslint/no-unsafe-assignment

Example:

import { memDecorator } from 'mem'

export class TestMemoizeGetter {
  constructorCalled?: boolean

  constructor() {
    this.constructorCalled = true
  }

  @memDecorator()
  public get someGetter() {
    if (!this.constructorCalled) {
      throw new Error('constructor has not been called')
    }

    return Math.random()
  }
}

const instance = new TestMemoizeGetter()

assert.doesNotThrow(() => {
  const value = instance.someGetter
  assert.strictEqual(instance.someGetter, value)
})

Output:

Error: constructor has not been called
    at Object.get someGetter (/home/neodon/p/mem-getters/index.ts:14:13)
    at file:///home/neodon/p/mem-getters/node_modules/mem/dist/index.js:84:29
    at __decorateClass (file:///home/neodon/p/mem-getters/index.ts:1:374)
    at <anonymous> (/home/neodon/p/mem-getters/index.ts:12:14)
    at ModuleJob.run (node:internal/modules/esm/module_job:194:25)

I messed around with the implementation for memDecorator and had some luck by using descriptor.value for functions and descripter.get for getter functions. It created some more problems though around this bindings. It seems that getter functions have a lot of voodoo baked in.

Let me know if you see value in this getting fixed. I can create a pull request with a failing test, and add some of the partial fix I figured out.

Also, the same issue exists with p-memoize, but it seemed better to start here since getters are synchronous.

[neodon]

I realized my initial assumption that const input = target[propertyKey]; was calling the getter probably doesn't make sense. The example code is constructing the object outside of the assertion, so it seems like the constructor should have already run before we call someGetter and get the exception.

I can dive in deeper and find out exactly what's going wrong, but I'd like to hear your thoughts first.

[fregante]

I think you’re right, that line does call the getter right when the decorator is first called:

https://github.com/sindresorhus/mem/blob/22c4b5e8fb033532eb070e24b26521e7898dfd77/index.ts#L177

instance.prop will immediately call the get prop function.

mem does not support getters at this point, I suppose. We should use the information in the descriptor (descriptor.value and descriptor.get) instead of that line

[fregante]

the same issue exists with p-memoize

It’s best to also open an issue there and link it to this one so people from both packages can contribute solutions.