From 3e12ac94ef9056fd0dd729a584e686f53d915482 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 15 Apr 2026 16:02:43 +0000 Subject: [PATCH 1/3] node-cache - fix: has() now checks TTL expiration (#1617) has() previously returned true for expired keys, breaking the has()/get() pattern used with the original node-cache. It now checks the TTL, emits "expired", and removes the key (when deleteOnExpire is enabled), matching node-cache's behavior. --- packages/node-cache/src/index.ts | 23 +++++++-- packages/node-cache/test/index.test.ts | 64 +++++++++++++++++++++----- 2 files changed, 73 insertions(+), 14 deletions(-) diff --git a/packages/node-cache/src/index.ts b/packages/node-cache/src/index.ts index 13be31b1..9cac7caa 100644 --- a/packages/node-cache/src/index.ts +++ b/packages/node-cache/src/index.ts @@ -443,12 +443,29 @@ export class NodeCache extends Hookified { } /** - * Returns boolean indicating if the key is cached. + * Returns boolean indicating if the key is cached. If the key has expired, it + * will return false and the key will be removed from the cache when + * `deleteOnExpire` is enabled (matches the original node-cache behavior). * @param {string | number} key if the key is a number it will convert it to a string - * @returns {boolean} true if the key is cached + * @returns {boolean} true if the key is cached and not expired */ public has(key: string | number): boolean { - return this.store.has(this.formatKey(key)); + const keyValue = this.formatKey(key); + const result = this.store.get(keyValue); + if (!result) { + return false; + } + + if (result.ttl > 0 && result.ttl < Date.now()) { + if (this.options.deleteOnExpire) { + this.del(key); + } + + this.emit("expired", keyValue, result.value); + return false; + } + + return true; } /** diff --git a/packages/node-cache/test/index.test.ts b/packages/node-cache/test/index.test.ts index de67af9e..3a66a748 100644 --- a/packages/node-cache/test/index.test.ts +++ b/packages/node-cache/test/index.test.ts @@ -73,10 +73,9 @@ describe("NodeCache", () => { const result = cache.mset(list); expect(result).toBe(true); expect(cache.get(goodKey)).toBe(goodValue); - // Negative TTL item is stored but expires immediately on get - expect(cache.has(badKey)).toBe(true); + // Negative TTL item is stored but has() already sees it as expired + expect(cache.has(badKey)).toBe(false); expect(cache.get(badKey)).toBe(undefined); - // After get triggers expiration + deleteOnExpire, key is removed expect(cache.has(badKey)).toBe(false); }); @@ -241,6 +240,50 @@ describe("NodeCache", () => { expect(has).toBe(true); }); + test("has() should return false for expired keys (issue #1617)", async () => { + const cache = new NodeCache({ checkperiod: 0 }); + const key = faker.string.uuid(); + cache.set(key, faker.lorem.word(), 0.05); + expect(cache.has(key)).toBe(true); + await sleep(100); + expect(cache.has(key)).toBe(false); + }); + + test("has() should emit expired and delete when deleteOnExpire is true", async () => { + const cache = new NodeCache({ + checkperiod: 0, + deleteOnExpire: true, + }); + const key = faker.string.uuid(); + const value = faker.lorem.word(); + cache.set(key, value, 0.05); + let expiredKey: string | undefined; + let expiredValue: string | undefined; + cache.on("expired", (k, v) => { + expiredKey = k; + expiredValue = v; + }); + await sleep(100); + expect(cache.has(key)).toBe(false); + expect(expiredKey).toBe(key); + expect(expiredValue).toBe(value); + expect(cache.keys()).not.toContain(key); + }); + + test("has() should not delete expired keys when deleteOnExpire is false", async () => { + const cache = new NodeCache({ + checkperiod: 0, + deleteOnExpire: false, + }); + const key = faker.string.uuid(); + cache.set(key, faker.lorem.word(), 0.05); + await sleep(100); + // has() still reports false because the key is expired + expect(cache.has(key)).toBe(false); + // But the underlying entry is still in the store + expect(cache.keys()).toContain(key); + }); + test("should return the stats of the cache", () => { const cache = new NodeCache({ checkperiod: 0 }); const key1 = faker.string.uuid(); @@ -324,11 +367,10 @@ describe("NodeCache", () => { const key = faker.string.uuid(); const result = cache.set(key, faker.lorem.word(), -1); expect(result).toBe(true); - // Key is stored with a past expiration timestamp - expect(cache.has(key)).toBe(true); - // get() sees it's expired and returns undefined + // has() treats the expired key as absent (matches node-cache behavior) + expect(cache.has(key)).toBe(false); + // get() also sees it's expired and returns undefined expect(cache.get(key)).toBe(undefined); - // After get triggers expiration + deleteOnExpire, key is removed expect(cache.has(key)).toBe(false); }); @@ -338,8 +380,8 @@ describe("NodeCache", () => { cache.set(key, faker.lorem.word()); const result = cache.ttl(key, -1); expect(result).toBe(true); - // Key exists but will expire on next access - expect(cache.has(key)).toBe(true); + // has() treats the expired key as absent + expect(cache.has(key)).toBe(false); expect(cache.get(key)).toBe(undefined); expect(cache.has(key)).toBe(false); }); @@ -349,7 +391,7 @@ describe("NodeCache", () => { const key = faker.string.uuid(); const result = cache.set(key, faker.lorem.word(), "-1"); expect(result).toBe(true); - expect(cache.has(key)).toBe(true); + expect(cache.has(key)).toBe(false); expect(cache.get(key)).toBe(undefined); expect(cache.has(key)).toBe(false); }); @@ -360,7 +402,7 @@ describe("NodeCache", () => { cache.set(key, faker.lorem.word()); const result = cache.ttl(key, "-5"); expect(result).toBe(true); - expect(cache.has(key)).toBe(true); + expect(cache.has(key)).toBe(false); expect(cache.get(key)).toBe(undefined); expect(cache.has(key)).toBe(false); }); From 55c1e07c82df1c416a60cec9465e5f790d8ca1ac Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 15 Apr 2026 17:05:15 +0000 Subject: [PATCH 2/3] node-cache - fix: emit "expired" at most once per entry When deleteOnExpire is false, repeatedly accessing an expired key via get()/has() (or hitting it via the checkData() interval sweep) would emit the "expired" event on every call, flooding listeners. Centralize the expiration path in a handleExpired() helper and track an "expiredEmitted" flag on the entry so the event fires once. The flag is reset when the entry is re-set or its ttl is re-scheduled. mget()/take() go through get() and inherit this behavior. --- packages/node-cache/src/index.ts | 52 +++++++++++++++++--------- packages/node-cache/test/index.test.ts | 43 +++++++++++++++++++++ 2 files changed, 77 insertions(+), 18 deletions(-) diff --git a/packages/node-cache/src/index.ts b/packages/node-cache/src/index.ts index 9cac7caa..e4425572 100644 --- a/packages/node-cache/src/index.ts +++ b/packages/node-cache/src/index.ts @@ -50,6 +50,12 @@ export type NodeCacheItem = PartialNodeCacheItem & { * The ttl of the item in milliseconds. 0 = unlimited */ ttl: number; + /** + * Internal flag indicating whether an "expired" event has already been + * emitted for this entry. Used when `deleteOnExpire` is false to avoid + * emitting the event repeatedly for every subsequent access. + */ + expiredEmitted?: boolean; }; export enum NodeCacheErrors { @@ -228,13 +234,8 @@ export class NodeCache extends Hookified { if (result) { if (result.ttl > 0) { if (result.ttl < Date.now()) { - if (this.options.deleteOnExpire) { - this.del(key); - } - this._stats.incrementMisses(); - // Event - this.emit("expired", this.formatKey(key), result.value); + this.handleExpired(key, result); return undefined; } @@ -402,6 +403,9 @@ export class NodeCache extends Hookified { result.ttl = 0; } + // Re-scheduling the expiration resets the "already emitted" flag so a + // future expiration of this key will fire the event again. + result.expiredEmitted = false; this.store.set(this.formatKey(key), result); return true; } @@ -450,18 +454,13 @@ export class NodeCache extends Hookified { * @returns {boolean} true if the key is cached and not expired */ public has(key: string | number): boolean { - const keyValue = this.formatKey(key); - const result = this.store.get(keyValue); + const result = this.store.get(this.formatKey(key)); if (!result) { return false; } if (result.ttl > 0 && result.ttl < Date.now()) { - if (this.options.deleteOnExpire) { - this.del(key); - } - - this.emit("expired", keyValue, result.value); + this.handleExpired(key, result); return false; } @@ -604,15 +603,32 @@ export class NodeCache extends Hookified { private checkData(): void { for (const [key, value] of this.store.entries()) { if (value.ttl > 0 && value.ttl < Date.now()) { - if (this.options.deleteOnExpire) { - this.del(key); - } - - this.emit("expired", this.formatKey(key), value.value); + this.handleExpired(key, value); } } } + /** + * Handles expiration for a cache entry. Deletes the entry when + * `deleteOnExpire` is enabled, and emits the "expired" event at most once + * per entry so repeated accesses to an expired key don't flood listeners + * when `deleteOnExpire` is false. + */ + private handleExpired(key: string | number, entry: NodeCacheItem): void { + const keyValue = this.formatKey(key); + const alreadyEmitted = entry.expiredEmitted === true; + + if (this.options.deleteOnExpire) { + this.del(key); + } else { + entry.expiredEmitted = true; + } + + if (!alreadyEmitted) { + this.emit("expired", keyValue, entry.value); + } + } + private createError(errorCode: string, key?: string): Error { let error = errorCode; /* v8 ignore next -- @preserve */ diff --git a/packages/node-cache/test/index.test.ts b/packages/node-cache/test/index.test.ts index 3a66a748..989e203a 100644 --- a/packages/node-cache/test/index.test.ts +++ b/packages/node-cache/test/index.test.ts @@ -284,6 +284,49 @@ describe("NodeCache", () => { expect(cache.keys()).toContain(key); }); + test("expired event fires only once per key across has/get/checkData when deleteOnExpire is false", async () => { + const cache = new NodeCache({ + checkperiod: 0, + deleteOnExpire: false, + }); + const key = faker.string.uuid(); + cache.set(key, faker.lorem.word(), 0.05); + let count = 0; + cache.on("expired", () => { + count++; + }); + await sleep(100); + cache.has(key); + cache.has(key); + cache.get(key); + cache.get(key); + // Exercise the interval sweep path explicitly + // biome-ignore lint/complexity/useLiteralKeys: accessing private method for testing + (cache as unknown as { checkData: () => void })["checkData"](); + expect(count).toBe(1); + }); + + test("re-scheduling ttl() resets expired-emit de-duplication", async () => { + const cache = new NodeCache({ + checkperiod: 0, + deleteOnExpire: false, + }); + const key = faker.string.uuid(); + cache.set(key, faker.lorem.word(), 0.05); + let count = 0; + cache.on("expired", () => { + count++; + }); + await sleep(100); + cache.has(key); // first expiration: fires + expect(count).toBe(1); + // Re-schedule with a new short TTL, which should reset the flag + cache.ttl(key, 0.05); + await sleep(100); + cache.has(key); // second expiration: fires again + expect(count).toBe(2); + }); + test("should return the stats of the cache", () => { const cache = new NodeCache({ checkperiod: 0 }); const key1 = faker.string.uuid(); From b9c3915bcc32dbb9e6fe7d30047a4ea6890628f5 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 15 Apr 2026 17:10:36 +0000 Subject: [PATCH 3/3] node-cache - refactor: always emit "expired" on access Drop the expiredEmitted de-duplication. The "expired" event now fires on every access to an expired entry (get/has/checkData), regardless of the deleteOnExpire option. Callers that want one-shot semantics can track that themselves. --- packages/node-cache/src/index.ts | 20 ++------------------ packages/node-cache/test/index.test.ts | 25 ++----------------------- 2 files changed, 4 insertions(+), 41 deletions(-) diff --git a/packages/node-cache/src/index.ts b/packages/node-cache/src/index.ts index e4425572..19259295 100644 --- a/packages/node-cache/src/index.ts +++ b/packages/node-cache/src/index.ts @@ -50,12 +50,6 @@ export type NodeCacheItem = PartialNodeCacheItem & { * The ttl of the item in milliseconds. 0 = unlimited */ ttl: number; - /** - * Internal flag indicating whether an "expired" event has already been - * emitted for this entry. Used when `deleteOnExpire` is false to avoid - * emitting the event repeatedly for every subsequent access. - */ - expiredEmitted?: boolean; }; export enum NodeCacheErrors { @@ -403,9 +397,6 @@ export class NodeCache extends Hookified { result.ttl = 0; } - // Re-scheduling the expiration resets the "already emitted" flag so a - // future expiration of this key will fire the event again. - result.expiredEmitted = false; this.store.set(this.formatKey(key), result); return true; } @@ -610,23 +601,16 @@ export class NodeCache extends Hookified { /** * Handles expiration for a cache entry. Deletes the entry when - * `deleteOnExpire` is enabled, and emits the "expired" event at most once - * per entry so repeated accesses to an expired key don't flood listeners - * when `deleteOnExpire` is false. + * `deleteOnExpire` is enabled and emits the "expired" event. */ private handleExpired(key: string | number, entry: NodeCacheItem): void { const keyValue = this.formatKey(key); - const alreadyEmitted = entry.expiredEmitted === true; if (this.options.deleteOnExpire) { this.del(key); - } else { - entry.expiredEmitted = true; } - if (!alreadyEmitted) { - this.emit("expired", keyValue, entry.value); - } + this.emit("expired", keyValue, entry.value); } private createError(errorCode: string, key?: string): Error { diff --git a/packages/node-cache/test/index.test.ts b/packages/node-cache/test/index.test.ts index 989e203a..d7df09be 100644 --- a/packages/node-cache/test/index.test.ts +++ b/packages/node-cache/test/index.test.ts @@ -284,7 +284,7 @@ describe("NodeCache", () => { expect(cache.keys()).toContain(key); }); - test("expired event fires only once per key across has/get/checkData when deleteOnExpire is false", async () => { + test("expired event fires on every access while deleteOnExpire is false", async () => { const cache = new NodeCache({ checkperiod: 0, deleteOnExpire: false, @@ -303,28 +303,7 @@ describe("NodeCache", () => { // Exercise the interval sweep path explicitly // biome-ignore lint/complexity/useLiteralKeys: accessing private method for testing (cache as unknown as { checkData: () => void })["checkData"](); - expect(count).toBe(1); - }); - - test("re-scheduling ttl() resets expired-emit de-duplication", async () => { - const cache = new NodeCache({ - checkperiod: 0, - deleteOnExpire: false, - }); - const key = faker.string.uuid(); - cache.set(key, faker.lorem.word(), 0.05); - let count = 0; - cache.on("expired", () => { - count++; - }); - await sleep(100); - cache.has(key); // first expiration: fires - expect(count).toBe(1); - // Re-schedule with a new short TTL, which should reset the flag - cache.ttl(key, 0.05); - await sleep(100); - cache.has(key); // second expiration: fires again - expect(count).toBe(2); + expect(count).toBe(5); }); test("should return the stats of the cache", () => {