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

feat: pass swagger-ui additional configuration #351

Merged
merged 12 commits into from Feb 3, 2021
Merged

feat: pass swagger-ui additional configuration #351

merged 12 commits into from Feb 3, 2021

Conversation

ghost
Copy link

@ghost ghost commented Feb 2, 2021

Porpouse

This PR allows to pass a dynamic configuration to the generated swagger UI. The option that should be used in the plugin configuration in order to accomplish that is uiConfig.

Configuration docs: https://github.com/swagger-api/swagger-ui/blob/master/docs/usage/configuration.md

Implementation

Since there is an issue configuring swagger-ui-dist (that distributes only static files, see swagger-api/swagger-ui#5710) without changing the content of the static assets, the only way to configure the UI with a dynamic external configuration is by editing the index.html file and the script within that loads the swagger ui in the page.

There were two approaches:

  1. Intercepting the index.html request on the documentation routes and distributing an altered html file that load swagger UI with the configuration merged with the one passed by the user.
  2. Editing the file index.html when the project gets prepared (npm run prepare) in order to retrieving a remote configuration fetched through an XHR request on a designated endpoint.

Since the index.html file was already edited in the prepare stage I went for the second approach. Now a new route under GET documentation/uiConfig gets declared and returns the final ui configuration and the script inside the index page fetches the configuration from that endpoint. Finally the page loads the swagger ui from the given configuration.

Drawbacks

Since there is no way to serialize functions over HTTP and JSON there is no way to pass functions to the parameters of the uiConfig, so only string/object/number configuration keys will be effective. Neither the first alternative implementation allowed to do that (at least without writing the plain function in the HTML file). However I think this is a fair compromise.

Related issues

Others

The npm run test didn't run all the test inside the folder test/ since the regex in the command was working only at the first directory level. Now it runs all the test till the 3rd directory level.

Checklist

@ghost
Copy link
Author

ghost commented Feb 2, 2021

The final configuration passed to the Swagger UI is composed by merging sequentially:

  • The default swagger UI configuration
  • The user defined configuration
  • The {url, oauth2RedirectUrl} configuration

Copy link
Member

@Fdawgs Fdawgs left a comment

Choose a reason for hiding this comment

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

Commit message for 9d96c34 should be test, not feat

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

Good work!

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@mcollina mcollina requested review from climba03003 and Eomm February 2, 2021 16:38
@climba03003 climba03003 linked an issue Feb 2, 2021 that may be closed by this pull request
Copy link
Member

@climba03003 climba03003 left a comment

Choose a reason for hiding this comment

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

LGTM

I get stuck in your option 1 when I see this feature request. My other consideration was CSP, but updating the index.html in npm run prepare is a good solution without affecting CSP.
Good Approach.

@climba03003
Copy link
Member

Just a suggest, will you consider replacing the whole block of <script></script> in index.html? I think you already changed all the content inside it.
It is more simple to use a single replace.

@ghost
Copy link
Author

ghost commented Feb 2, 2021

Just a suggest, will you consider replacing the whole block of <script></script> in index.html? I think you already changed all the content inside it.
It is more simple to use a single replace.

Yeah, that can do 😄

@mcollina
Copy link
Member

mcollina commented Feb 2, 2021

@giovanni-bertoncelli so we are waiting for a bit more change.

@ghost
Copy link
Author

ghost commented Feb 2, 2021

@giovanni-bertoncelli so we are waiting for a bit more change.

Yes. I'll replace the <script> and the package json in the next days..

@ghost
Copy link
Author

ghost commented Feb 3, 2021

Ok. I've:

  • Fixed the package.json npm run test with quotes (as @climba03003 suggested)
  • Replaced entirely the <script> tag (as @climba03003 suggested)
  • Renamed the unit test

(Sorry for the double github account, they are both mine)

@ghost ghost requested review from mcollina and climba03003 February 3, 2021 09:27
@ghost ghost changed the title Pass swagger-ui additional configuration feat: pass swagger-ui additional configuration Feb 3, 2021
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@mcollina mcollina merged commit 3c2a38e into fastify:master Feb 3, 2021
@ghost
Copy link
Author

ghost commented Feb 3, 2021

Any chance that this will be released soon? :)

@Eomm
Copy link
Member

Eomm commented Feb 3, 2021

Releasing 👍

@ghost
Copy link
Author

ghost commented Feb 4, 2021

Sorry, I've seen the release but downloading with npm the resulting static/ folder contains the old content... 🤔

@Eomm
Copy link
Member

Eomm commented Feb 4, 2021

mmm the script was updated
https://unpkg.com/browse/[email protected]/lib/util/prepare-swagger-ui.js

and it seems the pre-publish did not run:

"prepare": "node lib/util/prepare-swagger-ui",

I can release a fix this evening CEST if someone can't do it earlier

@ghost
Copy link
Author

ghost commented Feb 4, 2021

Ok @Eomm is there any error in the package.json definition then?

@mcollina
Copy link
Member

mcollina commented Feb 4, 2021

I've released v4.1.1, try that.

@ghost
Copy link
Author

ghost commented Feb 4, 2021

Yeah, now it's fine 🤷🏻‍♂️
Thanks @mcollina

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.

Supply additional configuration to swagger-ui
5 participants