move swagger-ui to webpack/npm and update it to 3.24.3

[silverwind]

Created a second webpack output file for swagger-ui which is loaded on the /api/swagger route. One notable difference is the absence of the swagger favicon that was previously used which is now the gitea icon. I see no easy way to restore that favicon, so I decided to not keep it.

[silverwind]

Also, this will update swagger-ui from 3.13.4 to 3.24.3.

[6543]

@silverwind can you add a screenshot
edit: thanks

[silverwind]

Screenshot added, but it's kind of pointless because page content has not changed 😉

[6543]

thought it would change more ... 😅 - code change looks good and local test worked :)
... (because of version update)

[sapk]

@silverwind doesn't this mean that the swagger.js will be build each time ?
It feel better to keep this outside from the gitea js config.

For info, by updating swagger-ui this will fix few security issues (but maybe not need a backport since only xss that we shouldn't be affected)

• https://github.com/swagger-api/swagger-ui/releases/tag/v3.18.0
• https://github.com/swagger-api/swagger-ui/releases/tag/v3.23.11
• https://github.com/swagger-api/swagger-ui/releases/tag/v3.24.1

[silverwind]

doesn't this mean that the swagger.js will be build each time ?

It will only build once per change in the web_src directory, or when node_modules change.

Not 100% sure if make is smart enough to track changes in node_modules, so it might be better to track package-lock.json instead.

[silverwind]

It feel better to keep this outside from the gitea js config.

I think it rightly belongs in the JS parts and swagger-ui also recommends the bundling approach.

[sapk]

I have doubt if this should be tightly configured to gitea js build but this is an improvement and it could be improved again later if it cause problem.

[techknowlogick]

Thanks for PR, I’m on mobile and am having difficulty adding a comment so I am blocking this until I can get to a desktop to add review.

[silverwind]

Investigating removal of source map for swagger.js as it's rather pointless for something that is 99.99% third party code and it will speed up the build.

[silverwind]

@sapk I switched to swagger-ui-dist, let me know what you think. Now, it just copies the files to public during the webpack build. That way, build will be faster with the downside that we can no longer control the polyfills for the swagger-ui code.

[silverwind]

Let's wait on @sapk to approve the latest change to swagger-ui-dist before merging. I did it based on request from #9715 (comment). Personally, I have a slight preference for the previous swagger-ui variant because it allows us to control the polyfills and it seems like the cleaner approach overall.

[silverwind]

Decided to go with swagger-ui approach. It just seems cleaner that way. The webpack build completes in 6 seconds on my machine with swagger-ui included in the build, which seems plenty fast to me.

Good to go from my side.

[sapk]

@silverwind It is more the fact that it is in the same build process than gitea js that bother me the most compared to using directly release files but it feel more a personal intuition and not backed by real experience. So I am totally ok with merging this and see if it become a problem (which will be limited since only for builder) or if I was wrong to think so. Otherwise this PR is totally LGTM.

[silverwind]

Okay. When it comes to JS modules, I don't expect release files to be present and many modules do not even provide them because it is generally expected that the user takes care of their bundling/polyfilling needs. swagger-ui actually goes the extra mile to provide those files separately, which is nice, but unnecessary given that we already bundle.

[sapk]

By release file, I mean like previously, totally outside npm/node_modules but don't bother with that.

[lafriks]

I would also prefer that we build dependencies ourselves as most libraries in npm are provided with idea to be built on end product

[silverwind]

Squashed and pushed a few minor cosmetic tweaks.

[silverwind]

CI failed with unrelated error, please restart.

level=error msg="Running error: ctrlflow: failed prerequisites: [email protected]/gitea/modules/markup/markdown [code.gitea.io/gitea/modules/markup.test]"

[silverwind]

Rebased. @techknowlogick want to lift your block?

dismiss block

[codecov-io]

Codecov Report

Merging #9714 into master will decrease coverage by 0.15%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #9714      +/-   ##
==========================================
- Coverage   42.32%   42.17%   -0.16%     
==========================================
  Files         600      592       -8     
  Lines       78357    78155     -202     
==========================================
- Hits        33161    32958     -203     
- Misses      41140    41150      +10     
+ Partials     4056     4047       -9

Impacted Files	Coverage Δ
models/repo_branch.go	50.43% <0%> (-49.57%)	⬇️
models/context.go	8.33% <0%> (-37.5%)	⬇️
routers/api/v1/repo/issue.go	35.09% <0%> (-18.32%)	⬇️
services/repository/repository.go	47.91% <0%> (-8.34%)	⬇️
models/repo_generate.go	3.8% <0%> (-6.62%)	⬇️
models/access.go	66.32% <0%> (-5.53%)	⬇️
services/pull/temp_repo.go	31.62% <0%> (-2.57%)	⬇️
models/action.go	46.87% <0%> (-2.53%)	⬇️
models/repo_list.go	74.42% <0%> (-2.39%)	⬇️
routers/api/v1/org/org.go	71.17% <0%> (-1.34%)	⬇️
... and 47 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 44de66b...fd40e58. Read the comment docs.