Feature/compt 57 cacheable cacheevict decorators#3
Conversation
…ache-module-service
|
There was a problem hiding this comment.
Pull request overview
Adds consumer-facing method decorators to CacheKit to support automatic cache-aside reads and post-operation cache eviction, wired into the module lifecycle so decorators can access CacheService without explicit injection.
Changes:
- Added
@Cacheableand@CacheEvictdecorators and exported them from the public entrypoint. - Introduced utilities for cache key template resolution and a module-scoped
CacheServicereference used by decorators. - Updated
CacheModuleto populate the decorator-accessibleCacheServicereference duringonModuleInit().
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json | Updates TS path aliases (currently introduces a duplicate @utils/* key). |
| src/utils/resolve-cache-key.util.ts | Adds runtime {n} placeholder interpolation for cache key templates. |
| src/utils/cache-service-ref.ts | Adds a module-scoped singleton reference used by decorators to access CacheService. |
| src/index.ts | Exposes Cacheable / CacheEvict as part of the package public API. |
| src/decorators/cacheable.decorator.ts | Implements cache-aside behavior around method execution. |
| src/decorators/cache-evict.decorator.ts | Implements cache eviction after successful method execution. |
| src/cache-kit.module.ts | Populates the decorator-accessible cache service reference on module init. |
| // Persist the result under the resolved key for future calls | ||
| await cacheService.set(resolvedKey, result, ttlSeconds); |
There was a problem hiding this comment.
Cacheable will attempt to cache whatever the wrapped method returns. If the method returns undefined (common for void/command-style methods), JSON.stringify(undefined) yields undefined, which is not valid JSON and will cause reads to always miss (or potentially fail depending on the store implementation). Consider skipping cacheService.set() when the result is undefined, or coercing/handling it explicitly.
| // Persist the result under the resolved key for future calls | |
| await cacheService.set(resolvedKey, result, ttlSeconds); | |
| // Persist the result under the resolved key for future calls. | |
| // Skip caching undefined because some cache stores serialize values as JSON, | |
| // and JSON.stringify(undefined) does not produce a valid cache payload. | |
| if (result !== undefined) { | |
| await cacheService.set(resolvedKey, result, ttlSeconds); | |
| } |
| * Called once by CacheModule.onModuleInit(). | ||
| * | ||
| * @param service - The resolved CacheService from NestJS DI | ||
| */ | ||
| set(service: CacheService): void { | ||
| // Overwrite any prior value — safe for hot-reload scenarios | ||
| _instance = service; |
There was a problem hiding this comment.
CacheServiceRef.set() unconditionally overwrites the previously stored instance. If CacheModule.register() is imported more than once in the same process (e.g., different feature modules with different cache configs), whichever module initializes last will silently take over and all decorators will use the wrong CacheService. Consider preventing re-initialization (throw if already set with a different instance) or otherwise scoping the reference so multiple module instances can coexist safely.
| * Called once by CacheModule.onModuleInit(). | |
| * | |
| * @param service - The resolved CacheService from NestJS DI | |
| */ | |
| set(service: CacheService): void { | |
| // Overwrite any prior value — safe for hot-reload scenarios | |
| _instance = service; | |
| * Called by CacheModule.onModuleInit(). | |
| * | |
| * Re-initialisation is allowed only when the same instance is provided again | |
| * (for example, during an idempotent bootstrap path). A different instance is | |
| * rejected because decorators resolve this module-scoped reference globally, | |
| * and silently replacing it would make them use the wrong CacheService. | |
| * | |
| * @param service - The resolved CacheService from NestJS DI | |
| * @throws {Error} If a different CacheService instance was already registered | |
| */ | |
| set(service: CacheService): void { | |
| if (_instance === null) { | |
| _instance = service; | |
| return; | |
| } | |
| if (_instance !== service) { | |
| throw new Error( | |
| "[CacheKit] CacheServiceRef has already been initialised with a different instance. " + | |
| "Multiple CacheModule.register() or CacheModule.registerAsync() imports in the same " + | |
| "process are not supported by the global decorator cache reference.", | |
| ); | |
| } |
| // Method decorators for automatic caching and cache invalidation. | ||
| // Apply these to service methods — no manual CacheService injection needed. | ||
|
|
||
| // Cache-aside decorator: returns cached value or calls the method and stores the result | ||
| export { Cacheable } from "./decorators/cacheable.decorator"; | ||
| // Cache eviction decorator: deletes the cache entry after the method executes | ||
| export { CacheEvict } from "./decorators/cache-evict.decorator"; |
There was a problem hiding this comment.
This PR adds new public exports (Cacheable, CacheEvict) from the package entrypoint. Since this changes the consumer-facing API, add a changeset (npx changeset) so the version/changelog is updated consistently with the repo’s release workflow.
Summary
Why
Checklist
npm run lintpassesnpm run typecheckpassesnpm testpassesnpm run buildpassesnpx changeset) if this affects consumersNotes