[re]: feat(@nestjs/swagger) add the ability to work with Nest RouterModule.

[shekohex]

PR Checklist

• The commit message follows our guidelines: https://github.com/nestjs/nest/blob/master/CONTRIBUTING.md
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[x] Feature
[x] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Other... Please describe:

What is the current behavior?

If we use nest router with swagger module. Swagger module doesn't know the correct routes.
Issue Number: nestjsx/nest-router#3

What is the new behavior?

add the ability to work with Nest RouterModule.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

this PR is a new one of #54 since it was messy there
so @kamilmysliwiec please merge this PR as soon as possible.
Thanks.

[thaoula]

Hi @shekohex,

Many thanks for your hard work :)

Will using this integration also honor the ApiDisableEndpoint decorator recently merged?

Would love to use the router module to define a private api (used by the spa app with cors disabled, ApiDisableEndpoint) and a public api using Swagger.

Regards,
Tarek

[shekohex]

Hi @thaoula, hmm, I'm a bit confused about the disabled Endpoints, the new changes just address the old issue, where the swagger module wasn't able to determine/detect the new changes (new endpoints) produced by RouterModule.
And about the @ApiExcludeEndpoint it just make swagger explorer returns early before adding this endpoint to the container.
So IMO the RouterModule has no business here !

Regards.

[thaoula]

Hi @shekohex,

I am not sure how the swagger module identifies the endpoints to list in the swagger UI. However, correct me if I am wrong, I am assuming that the disable endpoint will result in the endpoint being hidden in the swagger UI.

The reason for my question is i want to make sure we can mark the endpoints returned by your router as hidden so that they do not appear in the Swagger UI.

Hope that makes sense :)

[xmlking]

Hi @kamilmysliwiec
can you please merge this change.
latest combination is broken at this time.
@nestjs/swagger ^2.2.0
nest-router ^1.0.6
@nestjs/core 5.1.0

[xmlking]

@shekohex I build your branch and locally running my app with your swagger version.
now I am getting following error.

(node:16710) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'root' of undefined
    at lodash_1.filter.r (/myproject/node_modules/@xmlking/swagger/dist/swagger-transformer.js:6:61)
    at arrayFilter (/myproject/node_modules/lodash/lodash.js:582:11)
    at Function.filter (/myproject/node_modules/lodash/lodash.js:9173:14)
    at SwaggerTransformer.normalizePaths (/myproject/node_modules/@xmlking/swagger/dist/swagger-transformer.js:6:30)
    at SwaggerScanner.scanApplication (/myproject/node_modules/@xmlking/swagger/dist/swagger-scanner.js:21:50)
    at Function.createDocument (/myproject/node_modules/@xmlking/swagger/dist/swagger-module.js:9:46)
    at /myproject/apps/api/src/main.ts:31:34
    at Generator.next (<anonymous>)
    at fulfilled (/myproject/apps/api/src/main.ts:4:58)
    at process._tickCallback (internal/process/next_tick.js:68:7)
(node:16710) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:16710) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

[shekohex]

hi @xmlking , thank you for reporting,
it was a silly error, i missed the return here.
so it now should be fixed 😃

[xmlking]

@shekohex I am testing locally by cloning your repo and using npm link in your fork , and npm link @estjs/swagger in my project.
first time I got following error when I run app in my project.

TSError: ⨯ Unable to compile TypeScript:
apps/api/src/main.ts(31,49): error TS2345: Argument of type 'INestApplication & INestExpressApplication' is not assignable to parameter of type 'INestApplication'.
  Types of property 'init' are incompatible.
    Type '() => Promise<INestApplication & INestExpressApplication>' is not assignable to type '() => Promise<INestApplication>'.
      Type 'Promise<INestApplication & INestExpressApplication>' is not assignable to type 'Promise<INestApplication>'.
        Type 'INestApplication & INestExpressApplication' is not assignable to type 'INestApplication'.
apps/api/src/main.ts(32,31): error TS2345: Argument of type 'INestApplication & INestExpressApplication' is not assignable to parameter of type 'INestApplication'.

After I upgraded your fork with package.json to latest, it worked as expected.
no issues found so far. please consider to sync npm modules versions to latest.

{
  "name": "@nestjs/swagger",
  "version": "2.2.0",
  "description": "Nest - modern, fast, powerful node.js web framework (@swagger)",
  "author": "Kamil Mysliwiec",
  "license": "MIT",
  "scripts": {
    "build": "tsc -p tsconfig.json",
    "format": "prettier lib/**/*.ts --write",
    "precommit": "lint-staged",
    "prepublish:next": "npm run build",
    "publish:next": "npm publish --access public --tag next",
    "prepublish:npm": "npm run build",
    "publish:npm": "npm publish --access public"
  },
  "dependencies": {
    "lodash": "^4.17.10",
    "path-to-regexp": "^2.2.1",
    "swagger-ui-express": "^3.0.8"
  },
  "devDependencies": {
    "@nestjs/common": "^5.1.0",
    "@nestjs/core": "^5.1.0",
    "@types/node": "^10.5.2",
    "fastify-swagger": "^0.12.0",
    "husky": "^0.14.3",
    "lint-staged": "^7.2.0",
    "prettier": "^1.13.7",
    "reflect-metadata": "^0.1.12",
    "typescript": "2.7.2"
  },
  "peerDependencies": {
    "@nestjs/common": "^5.1.0",
    "@nestjs/core": "^5.1.0"
  },
  "lint-staged": {
    "*.ts": [
      "prettier --write",
      "git add -f"
    ]
  }
}

[shekohex]

@xmlking
oh, you alright, that behavior from old nest packages, but anyway it's out of this PR, i mean i will rebase that fork after this PR get marged.

and as always, thank you for reporting 👍

[xmlking]

@shekohex appreciate your quick fixes.
One issue I found is models definitions are generated by taking advantage of the reflection according to docs . this is not happening for this fork.

Expected:
Body, query, path parameters should be automatically detected and generated

@Post()
async create(@Body() createCatDto: CreateCatDto) {
  this.catsService.create(createCatDto);
}

[shekohex]

@xmlking can you share your playground (minimal repo) if possibly, so i can test that ?

[xmlking]

@shekohex
I am trying to reproduce above issue in a light test project that is mimicking setup of my internal private project.
https://github.com/xmlking/11-swagger (this is based on nestjs swagger example)
as you can see in commit logs, I am gradually adding features similar to my internal project.
so far everything works fine. I will try make this test project similar to my internal and see if I can reproduce. will let you know my finding soon.
Thanks

[xmlking]

finally i found where the issue is coming from. I am using monorepo style repo. this has tsconfig.json at project root level, and other one at API App's project root directory. somehow the root tsconfig.json is interfering with nestjs app. If i rename it or delete it, nestjs/swagger works perfectly .

You can reproduce it with my final replay repo at https://github.com/xmlking/11-swagger

Now i need to figerout how I can keep project root tsconfig.json and not interfere with nestjs!

[shekohex]

Hi @xmlking, thank you for debugging time, but in my opinion that issue is not related to your code / or nest however, that issue IMO, is a typescript configuration problem, but to be fair can you invest more time to test the last version of swagger from @nestjs/swagger with\without my changes, to see if the problem comes from my changes, if not, i think you should open a new issue so we can help with that.

and thank you again for your time 😃

[xmlking]

@shekohex I pulled your latest code any also published NPM module : @xmlking/swagger for easy testing.
Conforming that, above issue is not related to your code based on my tests. tsconfig.json interfering and effecting swagger generation only in my monorepo project setup.
Hope this can be merged soon sothat I can replace my temp NPM module with official one.

interestingly it only effect and cause problem when I run ts-node -r tsconfig-paths/register apps/api/src/main.ts but not for nodemon --config ./apps/api/nodemon.json

[kamilmysliwiec]

Thanks @shekohex 👍 Just merged :)

[lock]

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.