Use latest version of SwaggerUI by default, but allow it to be configured #628
Conversation
lib/open_api_spex/plug/swagger_ui.ex
Outdated
| Latest version of SwaggerUI is used by default, but it is possible to configure a different version: | ||
|
|
||
| ```elixir | ||
| config :open_api_spex, :swagger_ui_version, "4.14.0" |
There was a problem hiding this comment.
There was a problem hiding this comment.
I was thinking that too at one point, but then realized that currently all the plug options are passed to SwaggerUIBundle constructor and having one configuration parameter, which is handled differently seemed not correct thus your original proposal to configure it via config seemed more reasonable. Especially since there's already other configuration values, which are configued via config for open_api_spex.
After this feedback, do you still think that it would make more sense to introduce it as a plug configuration instead?
lib/open_api_spex/plug/swagger_ui.ex
Outdated
| <script src="https://cdnjs.cloudflare.com/ajax/libs/swagger-ui/4.14.0/swagger-ui-bundle.js" charset="UTF-8"> </script> | ||
| <script src="https://cdnjs.cloudflare.com/ajax/libs/swagger-ui/4.14.0/swagger-ui-standalone-preset.js" charset="UTF-8"> </script> | ||
| <script src="https://unpkg.com/swagger-ui-dist@#{@swagger_ui_version}/swagger-ui-bundle.js" charset="UTF-8"> </script> | ||
| <script src="https://unpkg.com/swagger-ui-dist@#{@swagger_ui_version}/swagger-ui-standalone-preset.js" charset="UTF-8"> </script> |
There was a problem hiding this comment.
I don't see the need to switch from cloudflare's own cdnjs to unpkg which is also now powered by cloudflare.
There was a problem hiding this comment.
As also mentioned in the issue #627 then I could not find a way to serve latest version from cdnjs and that's why I switched to the unpkg instead. At least https://cdnjs.cloudflare.com/ajax/libs/swagger-ui/latest/swagger-ui-bundle.js and https://cdnjs.cloudflare.com/ajax/libs/swagger-ui/swagger-ui-bundle.js do not work and https://cdnjs.cloudflare.com/ does not mention any other way to reference the latest version either. And even after googling it seems that cndjs does not allow to fetch latest versions on purpose (see this old issue for example among other similar issues).
If you happen to know a way to fetch latest version then there's no need to switch to unpkg indeed, otherwise there is and as mentioned in the issue then unpkg.com seems to be the de facto recommendation by SwaggerUI too (not that it matters in the end really).
lib/open_api_spex/plug/swagger_ui.ex
Outdated
| @@ -33,14 +41,16 @@ defmodule OpenApiSpex.Plug.SwaggerUI do | |||
| """ | |||
| @behaviour Plug | |||
|
|
|||
| @swagger_ui_version Application.compile_env(:open_api_spex, :swagger_ui_version, "latest") | |||
There was a problem hiding this comment.
The security risk of defaulting to "latest" feels too great for it to be the default value for me. I'd prefer "latest" be an option that can be supplied in the Plug opts, with the default being a version that is known to be compatible with the <script> tag in the template below.
There was a problem hiding this comment.
@mbuhot @jarmo it makes sense to me to default to a recent version which is known to work instead of latest. Furthermore, instead of specifying the version, we can allow setting the JS and CSS URLs:
get "/docs/api", OpenApiSpex.Plug.SwaggerUI,
path: "/my_service/docs/api_spec",
swagger_ui_js_url: "https://cdn.jsdelivr.net/npm/swagger-ui-dist@5/swagger-ui-bundle.js",
swagger_ui_css_url: "https://cdn.jsdelivr.net/npm/swagger-ui-dist@5/swagger-ui.css"This is inspired by fastapi. It also enables asset self-hosting or using the CDN of one's choice.
…igured via Plug options
jarmo
force-pushed
the
swagger_ui_version_configuration
branch
from
5cfc49d to
2c015d5
Compare
|
Updated PR related to comments/feedback above. |
…igured via Plug options (open-api-spex#628)
Closes #627