-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Link old service worker to new name #21581
Conversation
WalkthroughWalkthroughThe recent updates enhance file management and service worker functionality in the build process. Exclusion patterns for compression have been broadened to improve output control. The service worker generation has been refined by integrating symbolic links, ensuring better maintenance and deployment. Additionally, the service worker registration logic has been simplified to streamline updates, although this may impact how outdated service workers are handled. Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
src/util/register-service-worker.ts (1)
Line range hint
15-18
:
Potential issue with outdated service workers.The removal of the conditional block that checks if the active service worker's script URL includes "service_worker.js" means that outdated service workers will no longer be unregistered and the page will not be forced to reload. This might lead to users having outdated service workers, which could cause issues with updates.
Consider adding a mechanism to handle outdated service workers.
Looks like building the wheel resolves the link instead of copying it. Temporarily putting this in draft to fix... |
Wheels don't support symlinks, so a copy is created. It therefore won't have a compressed version, but not a big deal since it's seen once and only once per client. |
I've tested what I could, starting from 2024.7 and then upgrading again and the new service worker did replace the old one. |
Proposed change
Fixes #21542 by pointing the old service worker name to the new one, and revert #21561 as it is ineffective (new frontend never gets installed, so it will never see that code). This PR needs home-assistant/core#123131.
What is happening is that the HTML is being served by the old SW. This then loads the old frontend and continues to try to register the SW under the old name, which gets a 404 and so it won't get the update. The HTML is being handled with a "stale while revalidate" strategy, so it would be expected that the browser would just get the update anyway on the next visit. Browsers and the iOS app seem to do just that, but android web view seems to get in the way of this for some reason.
I'd have to do more digging to figure out why, but this change should fix it regardless. It might be that other browsers effectively unregister the old SW on the 404 and android doesn't, or that android's service worker controller is altering behavior. 🤷🏻♂️
Type of change
Example configuration
Additional information
Checklist
If user exposed functionality or configuration variables are added/changed:
Summary by CodeRabbit
New Features
Bug Fixes
Refactor