Merge injection token for cache and cache instance

[snys98]

Is there an existing issue that is already proposing this?

• I have searched the existing issues

Is your feature request related to a problem? Please describe it

for now we inject cacheManager in this way

import { CACHE_MANAGER} from '@nestjs/cache-manager';
import { Cache} from 'cache-manager';

@Controller()
export class AsyncRegisterController {
  constructor(@Inject(CACHE_MANAGER) private cacheManager: Cache) {}

which is hard to remember or code, because:
The Cache class is imported from the cache-manager, while CACHE_MANAGER token from the @nestjs/cache-manager package.

Describe the solution you'd like

we can simply wrap the Cache from cache-manager with

• cache.constants.ts

import { Cache } from "cache-manager";

export type NestCache = Cache;
export const CACHE_MANAGER = 'CACHE_MANAGER';
export const NestCache = CACHE_MANAGER;

this solution is not following correct structure, just to demonstrate my idea

Teachability, documentation, adoption, migration strategy

existing code will not be affected, all we need to do is to update documentation and mark CACHE_MANAGER as deprecated?

What is the motivation / use case for changing the behavior?

the motivation is that we can simplify our injection code to

import { NestCache } from '@nestjs/cache-manager';

@Controller()
export class AsyncRegisterController {
  constructor(@Inject(NestCache) private cacheManager: NestCache) {}

which is much easier and cleanner

and even more, we can avoid the @Inject(NestCache) by make NestCache a class that implement cache-manager's Cache

[micalevisk]

I think the the "issue" with having that indirection (NestCache type) is that isn't that clear that the cache manager is from cache-manager lib. But I'm fine with abstracting it away

Also, I'd suggest this one instead:

import { Cache } from '@nestjs/cache-manager';

@Controller()
export class Foo {
  constructor(private cacheManager: Cache) {} // no need to recall the token

yeah, it works

[snys98]

I think the the "issue" with having that indirection (NestCache type) is that isn't that clear that the cache manager is from cache-manager lib. But I'm fine with abstracting it away

Also, I'd suggest this one instead:

import { Cache } from '@nestjs/cache-manager';

@Controller()
export class Foo {
  constructor(private cacheManager: Cache) {} // no need to recall the token

yeah, it works

yes, I think either way is ok, it's just better to import 1 thing from 1 lib than 2 things from 2 libs

[jmcdo29]

Personally, I would rather import the specific token from the Nest wrapper and the actual type from the implementation library. Yes, it's another library to have to worry about, in terms of making sure it comes from the right place, but it also means that Nest can use peerDependencies for the library and allow devs to install whichever version of the library it needs, meaning less maintenance on our side when updates are released.

[snys98]

Personally, I would rather import the specific token from the Nest wrapper and the actual type from the implementation library. Yes, it's another library to have to worry about, in terms of making sure it comes from the right place, but it also means that Nest can use peerDependencies for the library and allow devs to install whichever version of the library it needs, meaning less maintenance on our side when updates are released.

From my understanding, this library is tightly related to cache-manager and it's appropriate to link cache manager as a peer dependency. It will cause trouble when user installed very old or very new versions, if he didn't know what the version that @nest/cache-manager is actually using.

[jmcdo29]

We already use peerDeps, so as long as cache-manager<=5 is installed, this library will work. If we set to dependencies then npm/yarn/pnpm will auto-install the package

[kamilmysliwiec]

We could fix this up by adding the extra useExisting provider (= thus not introducing any breaking changes)

[kamilmysliwiec]

Let's track this here #256