node-cache - feat: (breaking) moving to Keyv as the storage #1524
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1524 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 26 26
Lines 2369 2401 +32
Branches 516 530 +14
=========================================
+ Hits 2369 2401 +32 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR implements a breaking change to the node-cache package by replacing the Cacheable storage layer with direct Keyv integration. The primary goal is to simplify the storage architecture while maintaining compatibility with Keyv adapters.
Key Changes
- Replaced Cacheable with direct Keyv usage, removing the primary/secondary store concept
- Added support for shorthand TTL strings (e.g., '1h', '100ms') in addition to milliseconds
- Replaced dependency from
cacheableto@cacheable/utilsfor utility functions
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/node-cache/src/store.ts | Refactored NodeCacheStore to use Keyv directly, added manual stats tracking, removed Cacheable abstraction |
| packages/node-cache/src/index.ts | Updated NodeCache to use Stats from @cacheable/utils, implemented custom clone method |
| packages/node-cache/test/store.test.ts | Updated tests to reflect single store model, removed Keyv import, fixed typos (restult -> result) |
| packages/node-cache/test/index.test.ts | Added test for null value handling with cloning |
| packages/node-cache/package.json | Changed dependency from cacheable to @cacheable/utils |
| packages/node-cache/README.md | Updated documentation to reflect single store model, clarified TTL shorthand support |
Comments suppressed due to low confidence (1)
packages/node-cache/test/store.test.ts:50
- The test assumes that when maxKeys limit is reached (3 keys), the 4th set operation will fail and return false, so test4 should be undefined. However, the test doesn't verify the return value of the set operations. It should verify that the first 3 set operations return true and the 4th returns false to properly test the maxKeys behavior.
await store.set("test1", "value1");
await store.set("test2", "value2");
await store.set("test3", "value3");
await store.set("test4", "value4");
const result1 = await store.get("test4");
expect(result1).toBeUndefined();
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@jaredwray I've opened a new pull request, #1525, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
@jaredwray I've opened a new pull request, #1526, to work on those changes. Once the pull request is ready, I'll request review from you. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@jaredwray I've opened a new pull request, #1527, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
@jaredwray I've opened a new pull request, #1528, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
@jaredwray I've opened a new pull request, #1529, to work on those changes. Once the pull request is ready, I'll request review from you. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@jaredwray I've opened a new pull request, #1530, to work on those changes. Once the pull request is ready, I'll request review from you. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@jaredwray I've opened a new pull request, #1531, to work on those changes. Once the pull request is ready, I'll request review from you. |
Please check if the PR fulfills these requirements
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
node-cache - feat: (breaking) moving to Keyv as the storage