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

Move serviceworker to workbox and fix SSE interference #11538

Merged
merged 13 commits into from
May 22, 2020

Conversation

silverwind
Copy link
Member

Instead of statically hardcoding every frontend asset, this uses a type-based approach to cache all js,css and manifest.json requests.

This also fixes the issue that the service worker was interfering with EventSource because it was unconditionally handling all requests which this new implementation doesn't.

Fixes: #11092
Fixes: #7372

Instead of statically hardcoding every frontend asset, this uses a
type-based approach to cache all js,css and manifest.json requests.

This also fixes the issue that the service worker was interfering with
EventSource because it was unconditionally handling all requests which
this new implementation doesn't.

Fixes: go-gitea#11092
Fixes: go-gitea#7372
@CirnoT
Copy link
Contributor

CirnoT commented May 21, 2020

Confirmed that this solves issue with EventSource

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label May 21, 2020
Copy link
Contributor

@CirnoT CirnoT left a comment

Choose a reason for hiding this comment

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

Obviously can not test every edge case but fixes my issues with SW and populates cache properly as expected.

@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 May 21, 2020
@guillep2k guillep2k added this to the 1.13.0 milestone May 21, 2020
@guillep2k guillep2k added type/bug topic/ui Change the appearance of the Gitea UI labels May 21, 2020
@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 May 21, 2020
@silverwind
Copy link
Member Author

silverwind commented May 21, 2020

Should we remove templates.JSRenderer? I think it'll be unused after this PR and I don't see us rendering JS templates in the near future 😉.

@guillep2k
Copy link
Member

Should we remove templates.JSRenderer? I think it'll be unused after this PR and I don't see us rendering JS templates in the near future 😉.

In a separate PR, please.

@silverwind
Copy link
Member Author

@guillep2k are you sure? I think it's fine to remove here as it was only used for the serviceworker rendering and added in the original commit so I think it only makes sense to remove it now.

@silverwind
Copy link
Member Author

Removed it. @guillep2k hope it's okay for you 😉

@silverwind
Copy link
Member Author

One more feature added: Cache will now invalidate if AppVer (e.g. 1.13.0+dev-43-g8636e470d) changes. It keeps the version in localStorage for this purpose.

ui.USE_SERVICE_WORKER is still required for development as AppVer does not change between frontend rebuilds.

@techknowlogick
Copy link
Member

ping LG-TM

@techknowlogick techknowlogick merged commit 88fe7b5 into go-gitea:master May 22, 2020
@techknowlogick
Copy link
Member

@silverwind please send backport to release/v1.12 :)

silverwind added a commit to silverwind/gitea that referenced this pull request May 22, 2020
* Move serviceworker to workbox and fix SSE interference

Instead of statically hardcoding every frontend asset, this uses a
type-based approach to cache all js,css and manifest.json requests.

This also fixes the issue that the service worker was interfering with
EventSource because it was unconditionally handling all requests which
this new implementation doesn't.

Fixes: go-gitea#11092
Fixes: go-gitea#7372

* rethrow error instead of logging

* await .register

* Revert "rethrow error instead of logging"

This reverts commit 043162b.

* improve comment

* remove JSRenderer

* add version-based cache invalidation

* refactor

* more refactor

* remove comment

* rename item to fit cache name

Co-authored-by: guillep2k <18600385+guillep2k@users.noreply.github.com>
@silverwind
Copy link
Member Author

Backport done.

I'll follow up with another PR to this, there are some minor fixups to be done, but nothing crucial 😉

@silverwind silverwind deleted the workbox branch May 22, 2020 01:58
@techknowlogick techknowlogick added the backport/done All backports for this PR have been created label May 22, 2020
guillep2k added a commit that referenced this pull request May 22, 2020
* Move serviceworker to workbox and fix SSE interference

Instead of statically hardcoding every frontend asset, this uses a
type-based approach to cache all js,css and manifest.json requests.

This also fixes the issue that the service worker was interfering with
EventSource because it was unconditionally handling all requests which
this new implementation doesn't.

Fixes: #11092
Fixes: #7372

* rethrow error instead of logging

* await .register

* Revert "rethrow error instead of logging"

This reverts commit 043162b.

* improve comment

* remove JSRenderer

* add version-based cache invalidation

* refactor

* more refactor

* remove comment

* rename item to fit cache name

Co-authored-by: guillep2k <18600385+guillep2k@users.noreply.github.com>
zeripath added a commit to zeripath/gitea that referenced this pull request May 23, 2020
go-gitea#11538 moved the serviceworker to webbox but unfortunately
created the serviceworker in public/js rather than public/

This PR fixes this, fixing multiple issues with broken js
as a result of that change.

Signed-off-by: Andrew Thornton <art27@cantab.net>
zeripath added a commit that referenced this pull request May 23, 2020
#11538 moved the serviceworker to webbox but unfortunately
created the serviceworker in public/js rather than public/

This PR fixes this, fixing multiple issues with broken js
as a result of that change.

Signed-off-by: Andrew Thornton art27@cantab.net
zeripath added a commit to zeripath/gitea that referenced this pull request May 23, 2020
go-gitea#11538 moved the serviceworker to webbox but unfortunately
created the serviceworker in public/js rather than public/

This PR fixes this, fixing multiple issues with broken js
as a result of that change.

Signed-off-by: Andrew Thornton art27@cantab.net
zeripath added a commit that referenced this pull request May 23, 2020
#11538 moved the serviceworker to webbox but unfortunately
created the serviceworker in public/js rather than public/

This PR fixes this, fixing multiple issues with broken js
as a result of that change.

Signed-off-by: Andrew Thornton art27@cantab.net
ydelafollye pushed a commit to ydelafollye/gitea that referenced this pull request Jul 31, 2020
* Move serviceworker to workbox and fix SSE interference

Instead of statically hardcoding every frontend asset, this uses a
type-based approach to cache all js,css and manifest.json requests.

This also fixes the issue that the service worker was interfering with
EventSource because it was unconditionally handling all requests which
this new implementation doesn't.

Fixes: go-gitea#11092
Fixes: go-gitea#7372

* rethrow error instead of logging

* await .register

* Revert "rethrow error instead of logging"

This reverts commit 043162b.

* improve comment

* remove JSRenderer

* add version-based cache invalidation

* refactor

* more refactor

* remove comment

* rename item to fit cache name

Co-authored-by: guillep2k <18600385+guillep2k@users.noreply.github.com>
ydelafollye pushed a commit to ydelafollye/gitea that referenced this pull request Jul 31, 2020
go-gitea#11538 moved the serviceworker to webbox but unfortunately
created the serviceworker in public/js rather than public/

This PR fixes this, fixing multiple issues with broken js
as a result of that change.

Signed-off-by: Andrew Thornton art27@cantab.net
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/done All backports for this PR have been created lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. topic/ui Change the appearance of the Gitea UI type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Automate serviceworker caching manifest generation reference error in PWA serviceworker
6 participants