Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(plugin): allow the use of "uiConfig" options to fastify-swagger and Swagger UI #1341

Merged
merged 4 commits into from
Jun 18, 2021
Merged

Conversation

lazharichir
Copy link
Contributor

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[X] 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?

Using @nestjs/swagger with Fastify, you cannot pass the uiConfig options at fastify-swagger plugin registration.

Issue Number: N/A

What is the new behavior?

You can pass on uiConfig as per fastify-swagger's documentation.

Does this PR introduce a breaking change?

[ ] Yes
[X] No

Other information

In order to get there, this PR adds a fourth argument to setupFastify() which is the options: SwaggerCustomOptions object. This PR also adds the uiConfig?: Record<string, any> property to SwaggerCustomOptions as hint.

With this update, we can make use of great features from Swagger UI such as authorization persistence (uiConfig.persistAuthorization) and showing the operation's id (uiConfig.displayOperationId).

@@ -8,6 +8,7 @@ export interface SwaggerCustomOptions {
swaggerUrl?: string;
customSiteTitle?: string;
validatorUrl?: string;
uiConfig?: Record<string, any>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should define a separate options object for the fastify platform.

Copy link
Contributor Author

@lazharichir lazharichir May 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With just the uiConfig property (for now)?

export interface FastifySwaggerCustomOptions {
  uiConfig?: Record<string, any>;
}

And how do you want to handle narrowing the options type since we would have to do something along the lines of SwaggerCustomOptions | FastifySwaggerCustomOptions:

  public static setup(
    path: string,
    app: INestApplication,
    document: OpenAPIObject,
    options?: SwaggerCustomOptions | FastifySwaggerCustomOptions
  ) {
    const httpAdapter = app.getHttpAdapter();
    if (httpAdapter && httpAdapter.getType() === 'fastify') {
      return this.setupFastify(
        path,
        httpAdapter,
        document,
        options as FastifySwaggerCustomOptions
      );
    }
    return this.setupExpress(
      path,
      app,
      document,
      options as SwaggerCustomOptions
    );
  }

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good! Since we're about to introduce a new FastifySwaggerCustomOptions type, I think we should rename the existing one (SwaggerCustomOptions) to ExpressSwaggerCustomOptions and define a union type SwaggerCustomOptions = ExpressSwaggerCustomOptions | FastifySwaggerCustomOptions

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kamilmysliwiec sorry, had a lot on my plate but I've done the split! :)

@kamilmysliwiec kamilmysliwiec changed the base branch from master to 8.0.0 June 18, 2021 11:36
@kamilmysliwiec kamilmysliwiec merged commit 23a11c1 into nestjs:8.0.0 Jun 18, 2021
@kamilmysliwiec
Copy link
Member

LGTM. Merged to 8.0.0 branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants