node-cache - fix: has was not removing expired keys

packages/node-cache/src/index.ts

@@ -228,13 +228,8 @@ export class NodeCache<T> 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;
 				}
 
@@ -443,12 +438,24 @@ export class NodeCache<T> 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 result = this.store.get(this.formatKey(key));
+		if (!result) {
+			return false;
+		}
+
+		if (result.ttl > 0 && result.ttl < Date.now()) {
+			this.handleExpired(key, result);
+			return false;
+		}
+
+		return true;
 	}
 
 	/**
@@ -587,15 +594,25 @@ export class NodeCache<T> 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.
+	 */
+	private handleExpired(key: string | number, entry: NodeCacheItem<T>): void {
+		const keyValue = this.formatKey(key);
+
+		if (this.options.deleteOnExpire) {
+			this.del(key);
+		}
+
+		this.emit("expired", keyValue, entry.value);
+	}
+
 	private createError(errorCode: string, key?: string): Error {
 		let error = errorCode;
 		/* v8 ignore next -- @preserve */

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,72 @@ 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<string>({
+			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<string>({
+			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("expired event fires on every access while deleteOnExpire is false", async () => {
+		const cache = new NodeCache<string>({
+			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(5);
+	});
+
 	test("should return the stats of the cache", () => {
 		const cache = new NodeCache({ checkperiod: 0 });
 		const key1 = faker.string.uuid();
@@ -324,11 +389,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 +402,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 +413,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 +424,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);
 	});