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

Use local copy of workbox service worker scripts #1358

Merged
merged 3 commits into from
Oct 1, 2021
Merged

Use local copy of workbox service worker scripts #1358

merged 3 commits into from
Oct 1, 2021

Conversation

ajgon
Copy link
Contributor

@ajgon ajgon commented Sep 21, 2021

This is my attempt to fix #1147 . Looks like its not that complicated as workbox-cli provides anything necessary to make this process painless.

However I'm not a JS dev by any means, so I'm not sure if it's a "proper" way of doing this, or just ugly hack (especially the mv workbox-* workbox part which ensures consistency of directory name between versions, and keeps only necessary modules). But I believe it's a good starting point, to get rid of this "google call".

ajgon and others added 2 commits September 21, 2021 21:01
- Only add prod js, without maps. Reduces the size from 170k to 24k
- Removed it from build. As it is small now, we can add it to source, and have a script to just update it whenever it is required
- Fixed relative paths in navidrome-service-worker.js, should now work with BaseUrl != ''
@deluan
Copy link
Member

deluan commented Sep 26, 2021

Hey, thanks for this. I actually changed some things in your PR:

  • Only add prod js, without maps. Reduces the size from 170k to 24k
  • Removed it from build. As it is small now, we can add it to source, and have a script to just update it whenever it is required
  • Fixed relative paths in navidrome-service-worker.js, should now work with BaseUrl != ''

Let me know your thoughts or any concerns

@ajgon
Copy link
Contributor Author

ajgon commented Sep 27, 2021

Looking great 👍

@deluan
Copy link
Member

deluan commented Sep 27, 2021

Hey @certuna, can you please try your "set" of PWA validations with this build?

@certuna
Copy link
Contributor

certuna commented Sep 28, 2021

yes, I will test this

@certuna
Copy link
Contributor

certuna commented Sep 30, 2021

no issues so far

@deluan deluan merged commit be3a6dc into navidrome:master Oct 1, 2021
Dnouv pushed a commit to Dnouv/navidrome that referenced this pull request Oct 4, 2021
* Use local copy of workbox service worker scripts

* Refactor workbox integration:

- Only add prod js, without maps. Reduces the size from 170k to 24k
- Removed it from build. As it is small now, we can add it to source, and have a script to just update it whenever it is required
- Fixed relative paths in navidrome-service-worker.js, should now work with BaseUrl != ''

Co-authored-by: Deluan <deluan@navidrome.org>
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Navidrome connects to google
3 participants