perf(swagger-module): lazy load swagger-ui-dist module

[kaioduarte]

PR Checklist

Please check if your PR fulfills the following requirements:

• 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
• Feature
• 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?

• The swagger-ui-dist module is loaded before its utilization, and it loads two large files (listed below), causing some overhead on the app initialization (~800ms in a medium app).
  • https://github.com/swagger-api/swagger-ui/blob/master/swagger-ui-dist-package/index.js#L2-L3,
  • https://github.com/swagger-api/swagger-ui/blob/master/dist/swagger-ui-standalone-preset.js
  • https://github.com/swagger-api/swagger-ui/blob/master/dist/swagger-ui-bundle.js

Issue Number: N/A

What is the new behavior?

• The swagger-ui-dist module is loaded only when serving the Swagger HTML (SwaggerModule.serveStatic).

Does this PR introduce a breaking change?

• Yes
• No

Other information

Bubbleprof from Clinicjs using the E2E app.

Before	After

[micalevisk]

this will be great for serverless apps! thanks

[kamilmysliwiec]

Could we use await import() instead of require 🤔 ?

[kaioduarte]

@kamilmysliwiec we could, but that will require users to add await to the SwaggerModule.setup method.

[kamilmysliwiec]

So this is something that makes me a little concerned. If we wanted to support ESM (eventually), we'd have to use async import anyway (for lazy imports)

[kaioduarte]

I'm fine doing that change. However, do I need to update any doc to warn users about the need to await the setup method in the next release?

[micalevisk]

wouldn't that be a breaking change? Maybe we could keep require() + // TODO(v7): change to import statement.

[kamilmysliwiec]

Yeah that would be a breaking change, and that's why I'm concerned too. If we stick with the current implementation (top-level require), even if we decide to move to ESM one day, we wouldn't have to introduce any breaking changes

[micalevisk]

yeah, indeed.

Another solution that I can think of:

SwaggerModule.setup('api', app, document) // this won't use lazy loading

await SwaggerModule.setup('api', app, document, { lazy: true })

then in the next major release of @nestjs/swagger we'll remove that parameter to always use the lazy loading mode. Not sure if this is feasible tho

[kaioduarte]

Hey @kamilmysliwiec, what approach should we take? I'm not an ESM expert, but I'm happy to help and get this PR merged 🙂

[kaioduarte]

🏓

[kamilmysliwiec]

Let's ignore ESM for now. LGTM