search

groupon / node-cached

A simple caching library for node.js, inspired by the Play cache API
BSD 3-Clause "New" or "Revised" License
94 stars 15 forks source link

Cache creation documentation is ambiguous. #26

Closed fdegiuli closed 8 years ago

fdegiuli commented 8 years ago

The documentation for the cached function (https://github.com/fdegiuli/node-cached#cachedname-string-options---cache) say that name defaults to 'default'. This implies that name is optional. The problem is that calling cached({ options... }) makes a default noop cache that's actually named {options...}.

It should either be made clear that an unnamed cache (i.e. the "default" cache) cannot take configuration parameters–so it will always be a noop cache–or detect when the name argument is an object and use is as the options argument, setting the cache name to "default".

I'm leaning toward the first option, to avoid any confusion that might happen when creating multiple unnamed caches and always getting the same (default) cache object back.

Let me know what you think, and I'll be happy to send you guys a PR.

jkrems commented 8 years ago

I think I'm leaning towards making the name required and adding a deprecation warning when no name is provided (or the name isn't a string). Nice catch, the current docs definitely are misleading.

jkrems commented 8 years ago

Fixed via #27 in v4.2.2.