medikoo / memoizee

Complete memoize/cache solution for JavaScript
ISC License
1.73k stars 61 forks source link

Cannot use 'normalizer' with WeakMap mode #70

Closed rokoroku closed 7 years ago

rokoroku commented 7 years ago

We cannot use normalizer option with weakmap mode. When combined with normalizer, it throws a TypeError: undefined is not a function.

Example usage:

const weakMemoize = require('memoize/weak');

function findById(id): Model { ... }

function findMetaById(id): ModelMeta { ... }

// throws error
weakMemoize(findMetaById, { normalizer: (args) => findById(args[0]) });
medikoo commented 7 years ago

@rokoroku I'm not able to reproduce. Code you provide is not a valid JavaScript, and when I try something closest using the valid syntax, as below, there's no error

var weakMemoize = require('./weak');

function findById(id) { }

function findMetaById(id) { }

weakMemoize(findMetaById, { normalizer: function (args) { return findById(args[0]); } });

Can you provide a test case using valid JS syntax that shows the problem?

I'm closing it for now, as I don't see the issue, but I'll be happy to reopen if valid test case will be provided

rokoroku commented 7 years ago

Ah, sorry for the lack of introductions.

if you call weak-memoized function, it throws an error.

const memoize = require('memoizee');
const weakMemoize = require('memoizee/weak');

const testFunction = (id) => ({ id: id, timestamp: new Date() });
const plainMemoized = memoize(testFunction);
const weakMemoized = weakMemoize(testFunction, {
    normalizer: (args) => plainMemoized(args[0])
});

console.log(plainMemoized(0) === plainMemoized(0)) // true
console.log(weakMemoized(0) === weakMemoized(0)) // TypeError: ​​undefined is not a function​​

@medikoo I cannot reopen this issue since you close this, please check above code again :)

medikoo commented 7 years ago

@rokoroku Thanks, indeed there was an issue in normalize validator. It was too strict as it didn't pass previously resolved normalizers. It's fixed now, and will be published within 10 minutes.

As a side note, after fix is applied you will get another error, but this one is not a bug. Weak memoization for natural reasons works only with object values as first arguments, while (at least in your test case) you try to use primitives with it

medikoo commented 7 years ago

Fix published with v0.4.3