nitro icon indicating copy to clipboard operation
nitro copied to clipboard

feat: use pre-bundled `swagger-ui-dist`

Open HigherOrderLogic opened this issue 2 years ago โ€ข 4 comments

๐Ÿ”— Linked issue

Resolve https://github.com/unjs/nitro/issues/1757.

โ“ Type of change

  • [ ] ๐Ÿ“– Documentation (updates to the documentation, readme, or JSdoc annotations)
  • [ ] ๐Ÿž Bug fix (a non-breaking change that fixes an issue)
  • [x] ๐Ÿ‘Œ Enhancement (improving an existing functionality like performance)
  • [ ] โœจ New feature (a non-breaking change that adds functionality)
  • [ ] ๐Ÿงน Chore (updates to the build process or auxiliary tools and libraries)
  • [ ] โš ๏ธ Breaking change (fix or feature that would cause existing functionality to change)

๐Ÿ“š Description

Use pre-bundled package swagger-ui-dist for offline usage.

๐Ÿ“ Checklist

  • [x] I have linked an issue or discussion.
  • [ ] I have updated the documentation accordingly.

HigherOrderLogic avatar Oct 19 '23 05:10 HigherOrderLogic

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (e8fa771) 51.94% compared to head (5183b26) 52.38%. Report is 140 commits behind head on main.

:exclamation: Current head 5183b26 differs from pull request most recent head df35fb3. Consider uploading reports for the commit df35fb3 to get more accurate results

Files Patch % Lines
src/runtime/routes/swagger.ts 0.00% 1 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1840      +/-   ##
==========================================
+ Coverage   51.94%   52.38%   +0.44%     
==========================================
  Files         174      170       -4     
  Lines       12035    11781     -254     
  Branches      913      907       -6     
==========================================
- Hits         6252     6172      -80     
+ Misses       5686     5513     -173     
+ Partials       97       96       -1     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Oct 19 '23 05:10 codecov[bot]

Having this as a non-optional dependency is a drawback in terms of bundle size as not everyone will need this. I think the better way would be to allow customization of the URL/endpoint (or add a section to the documentation of how to add a UI route and remove the UI from the builtin routes altogether). In the end there are also alternative UIs like redoc and scalar which people want to use (and the comment from @pi0 that unjs might develop something similar in the future as well)

septatrix avatar Mar 08 '24 13:03 septatrix

Swagger route is only added in dev build, hence the production bundle size wont be affected.

HigherOrderLogic avatar Mar 09 '24 12:03 HigherOrderLogic

Swagger route is only added in dev build, hence the production bundle size wont be affected.

But the download size will. It an additional 10MB on top of what currently seems to be ~180MB for nuxt itself. This slows down installation, CIs, impact people with metered bandwidth etc. โ€“ all for something which many people might never use.

septatrix avatar Mar 09 '24 13:03 septatrix

Thanks for PR. Keeping this idea on hold since there are install size concerns + we are introducing new uis (/_nitro/scalar)

pi0 avatar Apr 04 '24 18:04 pi0