You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Deferred from the architecture review (architectural observation #1) — see ARCHITECTURE_REVIEW.md.
Problem
There is no orderly teardown path through the library's own abstractions:
CacheAdapter has no dispose()/close() in its contract.
MemoryAdapter.close() exists (packages/core/src/memory-adapter.ts:34) but only because checkPeriodMs starts a node-cache timer — it is an ad-hoc method, not part of any interface, so nothing generic can call it.
CacheManager has no shutdown at all. A caller holding a manager has no way to release its layers without reaching into getLayers() and type-testing each one for a method that may not exist.
ZigguratModule registers no onModuleDestroy/onApplicationShutdown, so a NestJS app's graceful shutdown never reaches the cache.
The practical symptom: a MemoryAdapter with checkPeriodMs set keeps an interval alive, which can hold a Node process open past where it should have exited — in tests, in CLI tools, in serverless teardown. The docs currently tell users to call close() themselves and know which adapters have it.
BaseCacheAdapter provides a no-op default so extending adapters get it free.
MemoryAdapter.dispose() stops the timer (keep close() as a deprecated alias, or fold it in).
CacheManager.close() fans out to every layer that declares one, allSettled so a failing layer doesn't strand the others.
ZigguratModule implements OnApplicationShutdown and calls it.
Connection lifecycle stays the caller's job — the Redis/Memcached/SQLite adapters take a client you constructed and should not close it out from under you. Worth stating that explicitly in the docs so the boundary is unambiguous.
Done when
A NestJS app shutting down, or a script calling await cache.close(), releases everything the library started, without the caller knowing which adapter types are in the stack.
Deferred from the architecture review (architectural observation #1) — see
ARCHITECTURE_REVIEW.md.Problem
There is no orderly teardown path through the library's own abstractions:
CacheAdapterhas nodispose()/close()in its contract.MemoryAdapter.close()exists (packages/core/src/memory-adapter.ts:34) but only becausecheckPeriodMsstarts anode-cachetimer — it is an ad-hoc method, not part of any interface, so nothing generic can call it.CacheManagerhas no shutdown at all. A caller holding a manager has no way to release its layers without reaching intogetLayers()and type-testing each one for a method that may not exist.ZigguratModuleregisters noonModuleDestroy/onApplicationShutdown, so a NestJS app's graceful shutdown never reaches the cache.The practical symptom: a
MemoryAdapterwithcheckPeriodMsset keeps an interval alive, which can hold a Node process open past where it should have exited — in tests, in CLI tools, in serverless teardown. The docs currently tell users to callclose()themselves and know which adapters have it.Sketch
dispose?(): Promise<void>on theCacheAdapterinterface — optional keeps every existing custom adapter valid, the same approachttlPolicytook in fix: resolve architecture review findings (H1-H3, M1-M6, L1-L6) #56.BaseCacheAdapterprovides a no-op default so extending adapters get it free.MemoryAdapter.dispose()stops the timer (keepclose()as a deprecated alias, or fold it in).CacheManager.close()fans out to every layer that declares one,allSettledso a failing layer doesn't strand the others.ZigguratModuleimplementsOnApplicationShutdownand calls it.Connection lifecycle stays the caller's job — the Redis/Memcached/SQLite adapters take a client you constructed and should not close it out from under you. Worth stating that explicitly in the docs so the boundary is unambiguous.
Done when
A NestJS app shutting down, or a script calling
await cache.close(), releases everything the library started, without the caller knowing which adapter types are in the stack.