Enable service worker for legacy build #21177
Conversation
WalkthroughWalkthroughThe changes introduce a new Changes
Recent review detailsConfiguration used: CodeRabbit UI Files ignored due to path filters (2)
Files selected for processing (11)
Files not reviewed due to errors (1)
Additional context usedPath-based instructions (3)
Biome
Additional comments not posted (13)
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 Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range comments (9)
src/entrypoints/custom-panel.ts (2)
Line range hint
94-94: Replace non-null assertions with optional chaining.Using non-null assertions can lead to runtime errors. Replace them with optional chaining to safely access properties.
- panelEl!.addEventListener("hass-toggle-menu", forwardEvent); - window.parent.customPanel!.navigate( - window.parent.customPanel!.registerIframe(initialize, setProperties), + panelEl?.addEventListener("hass-toggle-menu", forwardEvent); + window.parent.customPanel?.navigate( + window.parent.customPanel?.registerIframe(initialize, setProperties),Also applies to: 104-104, 140-140
Line range hint
95-95: Specify explicit types instead of usingany.Using
anytype disables many type checking rules and should be avoided. Specify more explicit types for better type safety.- const installingWorker = reg.installing; - const errorScreen = document.createElement("supervisor-error-screen") as any; - const errorScreen = document.createElement("hass-error-screen") as any; + const installingWorker: ServiceWorker | null = reg.installing; + const errorScreen: SupervisorErrorScreen = document.createElement("supervisor-error-screen") as SupervisorErrorScreen; + const errorScreen: HassErrorScreen = document.createElement("hass-error-screen") as HassErrorScreen;Also applies to: 109-109, 114-114, 117-117
src/entrypoints/service-worker.ts (3)
Line range hint
88-88: Consider refactoring to avoid using the delete operator.The delete operator can lead to performance issues due to deoptimizations in JavaScript engines.
- delete payload.data.jwt; + payload.data.jwt = undefined;
Line range hint
101-101: Use template literals for string concatenation.Using template literals instead of string concatenation is a more modern and readable approach in JavaScript.
- "Using fallback for:" + url + `Using fallback for: ${url}`
Line range hint
120-120: Specify explicit types to avoid implicit 'any'.Variables
iandclientare implicitly typed as 'any', which can lead to runtime errors if misused.- let i; - let client; + let i: number; + let client: WindowClient;Also applies to: 171-171, 172-172
src/types.ts (1)
Line range hint
71-71: Specify types instead of using 'any'.The use of 'any' type should be minimized to enhance type safety. Specifying explicit types helps in catching errors during compilation.
- export type Constructor<T = any> = new (...args: any[]) => T; + export type Constructor<T = unknown> = new (...args: unknown[]) => T;Also applies to: 117-117, 195-195, 201-201, 253-253, 256-256, 269-269, 292-292, 293-293, 296-296
src/panels/config/info/ha-config-info.ts (1)
Line range hint
106-106: Avoid using 'any' type for better type safety.Using 'any' type can lead to less predictable code behavior and more difficult maintenance. Specifying more precise types would improve the robustness of the code.
- const customUiList: Array<{ name: string; url: string; version: string }> = (window as any).CUSTOM_UI_LIST || []; + const customUiList: Array<{ name: string; url: string; version: string }> = (window as Window & { CUSTOM_UI_LIST?: Array<{ name: string; url: string; version: string }> }).CUSTOM_UI_LIST || [];Also applies to: 215-215, 217-217
build-scripts/webpack.cjs (1)
Line range hint
43-43: Consider refactoring to reduce cognitive complexity.The
createWebpackConfigfunction is complex and may be hard to maintain. Consider breaking it down into smaller, more manageable functions.build-scripts/bundle.cjs (1)
Line range hint
1-1: Use thenode:protocol when importing Node.js built-in modules.This is a best practice to clearly distinguish built-in modules from local ones.
- const path = require("path"); + const path = require("node:path");
|
If this is considered as a "breaking change", what is expected to be done by the user? |
|
It's not breaking... I checked the wrong box. |
| !env.useRollup() && !latestBuild | ||
| ? { | ||
| import: "./src/entrypoints/service-worker.ts", | ||
| layer: "sw", |
There was a problem hiding this comment.
How do these layers work, can't find anything about it in the webpack docs...
There was a problem hiding this comment.
Yeah the docs on it are quite sparse. It's more or less a tag/label for a module. You can assign it by entry like this, or via a condition in the module config. Then you can act on it in cache groups, by issuer layer in the module config (like I do here to change the loader options), and they also get grouped in the stats output.
In this PR, it boils down to:
- The service worker entry and everything it imports is assigned to the "sw" layer.
- For any module in the "sw" layer, Babel transpiles using the "legacy-sw" environment instead of "legacy".
|
Core PR has been approved. |
Proposed change
Create separate service workers for the modern and legacy builds. The legacy SW targets browsers that also support them to avoid some large unnecessary transpiling and polyfills. Adjusts the user-facing
__BUILD__global to the more user-friendly "modern" and "legacy" terms.Workbox is patched with GoogleChrome/workbox#3234 in order to use the injection method which properly adjusts the source maps.
404 errors will occur without home-assistant/core#120488.
Type of change
Example configuration
Additional information
Checklist
If user exposed functionality or configuration variables are added/changed:
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
workbox-buildto address specific issues.Style
Chores
source-map-urldependency.