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

Disable service worker by default (#18914) #19342

Merged
merged 1 commit into from
Apr 7, 2022

Conversation

silverwind
Copy link
Member

Backport #18914 to 1.16 as requested in #19341 (comment). It's a harmless change that should fix a lot browser cache issues. Later on, I plan to remove the ServiceWorker entirely.

The service worker causes a lot of issues with JS errors after instance
upgrades while not bringing any real performance gain over regular HTTP
caching.

Disable it by default for this reason. Maybe later we can remove it
completely, as I simply see no benefit in having it.
@silverwind silverwind added this to the 1.16.6 milestone Apr 7, 2022
@guillep2k
Copy link
Member

This will only have effect on new installations and users that have that field left unconfigured. Perhaps a mention in the release notes should be in order?

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 7, 2022
@silverwind
Copy link
Member Author

I think generally, we should be able to expect users to not have copy-pasted the example config as-is so unless they willingly set this option, they should rely on the default.

But yes, a note could be added to further reinforce that this is essentially a bugfix and we suggest to disable it in all cases for production setups.

Copy link
Contributor

@wxiaoguang wxiaoguang left a comment

Choose a reason for hiding this comment

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

Since it's useless, I think we can just set UI.UseServiceWorker=false

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Apr 7, 2022
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Apr 7, 2022
@6543 6543 added the type/enhancement An improvement of existing functionality label Apr 7, 2022
@6543 6543 merged commit 61c7732 into go-gitea:release/v1.16 Apr 7, 2022
@silverwind silverwind deleted the backport-18914 branch April 8, 2022 07:38
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants