Remove the ServiceWorker and manifest.json

[silverwind]

There is barely any performance benefit of SW-based caching and it causes a lot of issues after updating a instance and when the cache is stale, causing weird UI issues like chunk loading errors and other unexpected behaviour.

I kept the unregistration in the code so existing serviceworkers will properly uninstall after an update. We may be able to eventually drop the code completely in a future version after most users have ran the uninstallation.

manifest.json is removed because it serves no real purpose without a serviceworker and to eliminate another HTTP request for the sake of performance.

Fixes: #12344

[zeripath]

Is there no way we can stick monaco in the serviceworker to reduce the time it takes for that to start working?

[silverwind]

With no ServiceWorker, we still have the regular HTTP cache of the browser so things like Monaco will still be in there.

[silverwind]

Monaco is just one big chunk of JS (4MB) which takes time to execute even if the source files are cached. It may be worth to revisit #12193

[silverwind]

@zeripath revived that PR in #12401. It definitely increases monaco loading speed. Filenames are ugly but I guess that is acceptable for performance.

[CirnoT]

Not against (in fact strongly in favor) but as discussed before it will be a breaking change for those using or relying on Gitea being PWA.

[silverwind]

I don't see how this could be breaking. The ServiceWorker was only used to cache assets. None of those assets can work standalone without the server available so there was never any offline functionality.

[CirnoT]

https://developer.mozilla.org/en-US/docs/Web/Progressive_web_apps/Installable_PWAs

Simply speaking the 'Install' icon wont show anymore to create easy shortcut and open Gitea in its own window. Pretty useless for us to be honest, but nonetheless it will change the behavior.

[CirnoT]

Might as well drop manifest.json too as it won't be used without SW

[silverwind]

Indeed, removing manifest.json might be a good idea to eliminate another HTTP request on every page. It currently pretty much only serves to set background_color (wrongly in the case of non-default theme) and theme_color (has a HTML meta tag alternative, which we already set).

[silverwind]

manifest.json removed as well and updated OP with a issue reference.

[silverwind]

Removed two images that were only used in manifest.json and README (replaced it with the SVG there).

[codecov-commenter]

Codecov Report

Merging #12400 into master will increase coverage by 0.00%.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master   #12400   +/-   ##
=======================================
  Coverage   43.80%   43.80%           
=======================================
  Files         630      631    +1     
  Lines       69821    69844   +23     
=======================================
+ Hits        30585    30596   +11     
- Misses      34300    34305    +5     
- Partials     4936     4943    +7     

Impacted Files	Coverage Δ
modules/setting/setting.go	46.33% <ø> (-0.11%)	⬇️
modules/templates/helper.go	46.05% <ø> (-0.34%)	⬇️
routers/routes/routes.go	88.97% <ø> (+0.33%)	⬆️
modules/indexer/stats/queue.go	52.94% <0.00%> (-11.77%)	⬇️
modules/util/sanitize.go	37.50% <0.00%> (-3.68%)	⬇️
modules/lfs/content_store.go	48.33% <0.00%> (-1.67%)	⬇️
models/attachment.go	57.74% <0.00%> (-1.41%)	⬇️
modules/repository/init.go	58.26% <0.00%> (-0.94%)	⬇️
models/unit_tests.go	74.75% <0.00%> (-0.92%)	⬇️
services/pull/check.go	56.15% <0.00%> (-0.88%)	⬇️
... and 30 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 faa676c...e92d553. Read the comment docs.

[Floaf]

Will not this affect future Push notifications? #10884

[silverwind]

I guess it'd be possible to keep the SW but just disable the (broken) asset caching, so it essentially would do nothing.

[wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf]

Wouldn't it be better to just change the app.ini config parameter USE_SERVICE_WORKER to disable SW by default, instead of removing the SW functionality entirely?
That way users who'd still prefer to have it would not be affected (they would get to keep the broken caching + mainfest.json) and the default would be not to use it.

[silverwind]

Hmm, actually not a bad idea but I'd introduce a new config switch that controls whether the caching is enabled (off by default). When no SW-managed feature is enabled (currently there's only one), uninstall it.

[wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf]

Hmm, actually not a bad idea but I'd introduce a new config switch that controls whether the caching is enabled (off by default). When no SW-managed feature is enabled (currently there's only one), uninstall it.

cool, that'd do very well, too :)

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 months. Thank you for your contributions.

[lunny]

Please resolve the conflicts.

[silverwind]

Don't think I'll pursue this anymore. I have a fix in mind using SKIP_WAITING for the stale serviceworker issue but it's quite hard to reproduce/test it, may check out again later.