Generated controllers can import unused decorators/empty constructor.

[evantrimboli]

Steps to reproduce

Using the open api generator, generate a controller that does not use any request bodies or does not pass any parameters.

This can be problematic depending on the compiler options, for example having unused parameters. The same could be said for an empty parameterless constructor.

Current Behavior

The param and requestBody decorators are imported in the controller file. An empty constructor is generated.

Expected Behavior

The param and requestBody decorators should not be imported. The constructor should be omitted.

Link to reproduction sandbox

See here: https://github.com/strongloop/loopback-next/blob/72cb8aafad31588deff96fb1af6d33fa03fe5fd5/packages/cli/generators/openapi/templates/src/controllers/controller-template.ts.ejs#L2

The imports are made unconditionally.

Additional information

win32 x64 12.5.0
├─ @loopback/boot@1.4.4
├─ @loopback/context@1.20.2
├─ @loopback/core@1.8.5
├─ @loopback/http-server@1.4.4
├─ @loopback/metadata@1.2.5
├─ @loopback/openapi-v3@1.7.0
├─ @loopback/repository-json-schema@1.8.0
├─ @loopback/repository@1.8.2
├─ @loopback/rest-explorer@1.2.5
├─ @loopback/rest@1.16.3
├─ @loopback/service-proxy@1.2.5
├─ loopback-connector@4.8.0
├─ loopback-datasource-juggler@4.8.2

Related Issues

N/A

[raymondfeng]

@evantrimboli We generate the empty constructor intentionally to make it easy for developers to add dependency injection with constructor parameters.

import {operation, param, requestBody} from '@loopback/rest'; is typically required by REST controllers. You can run npm run lint:fix afterward to remove them.

[evantrimboli]

Regarding eslint, I am fairly sure that's still not offered as a solution out of the box:

eslint/eslint#11668

I know you can't support all possible configurations, perhaps they could be options one could configure on the generator? Or alternatively provide some kind of hook to allow for more manual intervention.

[bajtos]

I would hope it should be relatively easy to detect whether operation, param and requestBody are used and import them only when needed.

Here is the template we use to generate the controller file:

• https://github.com/strongloop/loopback-next/blob/master/packages/cli/generators/openapi/templates/src/controllers/controller-template.ts.ejs

Before the template is executed, we need to gather data from the OpenAPI spec, this is done by files in packages/cli/generators/openapi directory.

@evantrimboli would you like to investigate this yourself and contribute a pull request to fix the way how imports are generated? See https://loopback.io/doc/en/contrib/code-contrib-lb4.html and https://loopback.io/doc/en/lb4/submitting_a_pr.html to get you started.

[evantrimboli]

At this point I don't have the time/expertise to dig into it. Perhaps in the future.

[evantrimboli]

Just for future reference, it would also be good if we could have the option to generate the controllers in a form similar to this:

abstract class MyController {
  @operation('post', '/login')
  async login(@requestBody() _requestBody: Credentials): Promise<LoginResult> {
    return await this.doLogin();
  }

  abstract doLogin(_requestBody: Credentials): Promise<LoginResult>;
}

That way we could extend these generated classes in a type-safe way without needing to modify the generated code.