Nest: Add a @Deprecated decorator for deprecation warnings

[BrunnerLivio]

I'm submitting a...

[ ] Regression 
[ ] Bug report
[x] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead post your question on Stack Overflow.

Current behavior

At the moment there is no normalized way to add a deprecation warning of a functionality

Expected behavior

There should be a @Depreacted(message?: string) decorator which logs a deprecation warning when calling the function.

E.g.

class MyService {

  @Deprecrated('The noGood method will be deprecated in the next major release. Please use the reallyGood method of the MyService instance')
  public noGood() {
    // my super old function
    console.log('No good :(');
  }

  public reallyGood() {
    console.log('Wow this function is so much nicer');
  }
}

[...]

this.myService.noGood(); 

// prints out:
// Deprecation warning: The noGood method will be deprecated in the next major release. Please use the reallyGood method of the MyService instance
// No good :(

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

It would be useful for a developer to add a clear deprecation warning for a function. This can also be useful for NestJS or its additional modules.

I think it should be part of Nest, so it can be integrated with the already existing logger.

Other

Deprecating before runtime (Intelisense) is currently not possible with TypeScript. See issue here
https://github.com/Microsoft/TypeScript/issues/390

[lazarljubenovic]

I'd expect @Deprecated as a decorator on a controller method to include deprecation warning in the Response, not the log it. Like, deprecating an API method (endpoint), not a method from code. For deprecating methods, TSDoc's /** @deprecated */ is an already established way. Although, I agree, not runtime-specific -- but then again why would Nest provide a runrime deprecation warning for a custom method in code? It seems like a job for a completely separate general TypeScript decorator (there's probably something on npm already).

One of the ideas is to use 299 code for a deprecated endpoint, which could be what this decorator would do by default.

[BrunnerLivio]

@lazarljubenovic

I'd expect @Deprecated as a decorator on a controller method to include deprecation warning in the Response

Agree this could be useful too, but my post is about a different concern.

For deprecating methods, TSDoc's /** @deprecated */ is an already established way

@deprecated JSDoc is currently ignored by TSC (issue: https://github.com/Microsoft/TypeScript/issues/390). Therefore it will not warn the user during compilation in case he is using a deprecated functionality.

but then again why would Nest provide a runrime deprecation warning for a custom method in code?

The benefit is generally for Nest module owners. For me as a maintainer of a Nest module, I want to warn the user in case he/she is calling a deprecated function or inject a deprecated provider.
The only other way to support both JavaScript and TypeScript users of NestJS would then be eslint, but we can not force our users to use a linter.

It seems like a job for a completely separate general TypeScript decorator

What'd be your suggestion for this, to support both of our concerns? Add one decorator for API deprecation and one for programmatic deprecation?

(there's probably something on npm already).

Yup, there is probably something on NPM, but I do not think we should rely on a NPM package for such a small functionality. In addition I want to integrate the deprecation warnings with the Nest logger. I think we'd be better off without a NPM package.

[lazarljubenovic]

is currently ignored by TSC

Ah, indeed. WebStorm plays nice though and puts a line through the method call. :heart:

The benefit is generally for Nest module owners. For me as a maintainer of a Nest module, I want to warn the user in case he/she is calling a deprecated function or inject a deprecated provider.

The reason why I don't think it belongs to nest is that this statement can be rewritten for every single package: _The benefit is generally for ××× module owners. For me as a maintainer of a ××× module, I want to warn the user in case he/she is calling a deprecated function or inject a deprecated provider._

Which brings me to...

Yup, there is probably something on NPM, but I do not think we should rely on a NPM package for such a small functionality.

I'm not sure I still see the point in adding this to Nest. Wouldn't then EVERY single framework in the world need such a decorator?

And one more reason why such a general decorator shouldn't be included in a particular framework like Nest is exactly this:

Agree this could be useful too, but my post is about a different concern.

We'd have two decorators for deprecating now, which is cluttering the API surface with similar things unnecessarily.

[BrunnerLivio]

@lazarljubenovic
I see you point, I think you're right. Looking through the source code of NestJS it seems like it is used by deprecated npm package. The only downside is with this approach, the package is probably not integrated with the NestJS logger, so it will be harder for the user to suppress such warnings. But that is a different topic.
I'll close this issue now. I encourage you (@lazarljubenovic) to create a new issue for controller deprecation functionality.

[lock[bot]]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.