Inject ResizeObserver polyfill where needed #20754
Conversation
|
@bramkragten the small overall bundle increases are because this now polyfills dependencies that use |
steverep
force-pushed
the
inject-resize-observer
branch
from
3e24278
to
26ae02b
Compare
WalkthroughWalkthroughThe updates primarily focus on integrating a Changes
Tip New Features and ImprovementsReview SettingsIntroduced new personality profiles for code reviews. Users can now select between "Chill" and "Assertive" review tones to tailor feedback styles according to their preferences. The "Assertive" profile posts more comments and nitpicks the code more aggressively, while the "Chill" profile is more relaxed and posts fewer comments. AST-based InstructionsCodeRabbit offers customizing reviews based on the Abstract Syntax Tree (AST) pattern matching. Read more about AST-based instructions in the documentation. Community-driven AST-based RulesWe are kicking off a community-driven initiative to create and share AST-based rules. Users can now contribute their AST-based rules to detect security vulnerabilities, code smells, and anti-patterns. Please see the ast-grep-essentials repository for more information. New Static Analysis ToolsWe are continually expanding our support for static analysis tools. We have added support for Tone SettingsUsers can now customize CodeRabbit to review code in the style of their favorite characters or personalities. Here are some of our favorite examples:
Revamped Settings PageWe have redesigned the settings page for a more intuitive layout, enabling users to find and adjust settings quickly. This change was long overdue; it not only improves the user experience but also allows our development team to add more settings in the future with ease. Going forward, the changes to Miscellaneous
Recent Review DetailsConfiguration used: CodeRabbit UI Files selected for processing (16)
Additional Context UsedBiome (149)
Path-based Instructions (1)
Additional comments not posted (5)
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: 0
Outside diff range and nitpick comments (32)
src/resources/compatibility.ts (3)
Line range hint
7-28: Consider usingfor...ofinstead offorEachfor better performance and readability in polyfill implementations.- arr.forEach((item) => { + for (const item of arr) {
Line range hint
6-29: Convert the function expression to an arrow function for consistency with modern JavaScript practices.- (function (arr) { + ((arr) => {
Line range hint
48-48: Avoid reassigning function parameters as it can lead to confusion. Consider using a different variable.- force = !!force; + const coercedForce = !!force;src/state-summary/state-card-input_number.ts (2)
Line range hint
126-126: Avoid using non-null assertions. Consider using optional chaining or proper checks before accessing properties.- this.shadowRoot!.querySelector(".state") + this.shadowRoot?.querySelector(".state")Also applies to: 147-147
Line range hint
2-2: Ensure that imports used only as types are correctly marked withimport type.- import { HassEntity } from "home-assistant-js-websocket"; + import type { HassEntity } from "home-assistant-js-websocket";Also applies to: 3-3, 11-11
src/state-summary/state-card-number.ts (2)
Line range hint
103-103: Avoid using non-null assertions. Consider using optional chaining or proper checks before accessing properties.- this.shadowRoot!.querySelector(".state") + this.shadowRoot?.querySelector(".state")
Line range hint
3-3: Ensure that imports used only as types are correctly marked withimport type.- import { HomeAssistant } from "../types"; + import type { HomeAssistant } from "../types";Also applies to: 8-8
src/panels/lovelace/entity-rows/hui-input-number-entity-row.ts (3)
Line range hint
155-155: Avoid using non-null assertions without checking for null or undefined.Consider adding a null check before using the
!operator to ensure that the object is not null or undefined. This will prevent potential runtime errors.
Line range hint
174-174: Avoid repeated non-null assertions.You've used the non-null assertion operator multiple times on the same line. Ensure that the object is not null or undefined before using it, or refactor the code to perform the check once.
Also applies to: 174-174, 178-178
Line range hint
2-9: Optimize imports by removing unused ones.You have several imports that are only used as types. These can be imported using
import typeto make it clear that they are only used for type checking and not included in the JavaScript output.- import { CSSResultGroup, LitElement, PropertyValues, css, html, nothing } from "lit"; + import type { CSSResultGroup, LitElement, PropertyValues } from "lit"; + import { css, html, nothing } from "lit"; - import { customElement, property, state } from "lit/decorators"; + import type { customElement, property, state } from "lit/decorators"; - import { HomeAssistant } from "../../../types"; + import type { HomeAssistant } from "../../../types";Also applies to: 16-16, 20-20
src/panels/lovelace/entity-rows/hui-number-entity-row.ts (3)
Line range hint
161-161: Avoid using non-null assertions without checking for null or undefined.Consider adding a null check before using the
!operator to ensure that the object is not null or undefined. This will prevent potential runtime errors.
Line range hint
180-180: Avoid repeated non-null assertions.You've used the non-null assertion operator multiple times on the same line. Ensure that the object is not null or undefined before using it, or refactor the code to perform the check once.
Also applies to: 180-180, 183-183, 183-183
Line range hint
2-9: Optimize imports by removing unused ones.You have several imports that are only used as types. These can be imported using
import typeto make it clear that they are only used for type checking and not included in the JavaScript output.- import { CSSResultGroup, LitElement, PropertyValues, css, html, nothing } from "lit"; + import type { CSSResultGroup, LitElement, PropertyValues } from "lit"; + import { css, html, nothing } from "lit"; - import { customElement, property, state } from "lit/decorators"; + import type { customElement, property, state } from "lit/decorators"; - import { HomeAssistant } from "../../../types"; + import type { HomeAssistant } from "../../../types";Also applies to: 16-16, 20-20
src/panels/lovelace/cards/hui-calendar-card.ts (2)
Line range hint
89-89: Avoid using non-null assertions without checking for null or undefined.Multiple lines in your code use the non-null assertion operator without prior null checks. This can lead to runtime errors if the objects are actually null or undefined. Consider adding appropriate checks or handling potential null values gracefully.
Also applies to: 136-136, 162-162, 181-181, 191-191, 192-192, 197-197, 204-204, 217-217
Line range hint
2-9: Optimize imports by removing unused ones.Several imports in your file are only used for type checking. You can optimize these imports by using
import type, which will not include them in the JavaScript output, reducing the bundle size.- import { css, CSSResultGroup, html, LitElement, PropertyValues, nothing } from "lit"; + import type { CSSResultGroup, LitElement, PropertyValues } from "lit"; + import { css, html, nothing } from "lit"; - import { customElement, property, state } from "lit/decorators"; + import type { customElement, property, state } from "lit/decorators"; - import type { HomeAssistant } from "../../../types"; + import { HomeAssistant } from "../../../types"; - import { Calendar, CalendarEvent, fetchCalendarEvents } from "../../../data/calendar"; + import type { Calendar, CalendarEvent } from "../../../data/calendar"; + import { fetchCalendarEvents } from "../../../data/calendar";Also applies to: 13-13, 17-21
src/panels/lovelace/entity-rows/hui-media-player-entity-row.ts (2)
Line range hint
147-147: Avoid unnecessary template literals.You have used template literals in several places where simple strings would suffice. This is unnecessary and can be simplified.
- .label=${this.hass.localize(`ui.card.media_player.media_play`)} + .label=${this.hass.localize("ui.card.media_player.media_play")} - .label=${this.hass.localize(`ui.card.media_player.media_pause`)} + .label=${this.hass.localize("ui.card.media_player.media_pause")} - .label=${this.hass.localize(`ui.card.media_player.media_stop`)} + .label=${this.hass.localize("ui.card.media_player.media_stop")}Also applies to: 157-157, 168-168
Line range hint
320-320: Avoid using non-null assertions without checking for null or undefined.Multiple lines in your code use the non-null assertion operator without prior null checks. This can lead to runtime errors if the objects are actually null or undefined. Consider adding appropriate checks or handling potential null values gracefully.
Also applies to: 320-320, 322-322, 326-326, 332-332, 332-332, 341-341, 342-342, 347-347, 348-348, 353-353, 354-354, 359-359, 360-360, 365-365, 366-366, 371-371
src/components/map/ha-map.ts (5)
Line range hint
9-9: Consider renaming theMaptype fromleafletto avoid shadowing the globalMapobject.- import type { Map } from "leaflet"; + import type { Map as LeafletMap } from "leaflet";
Line range hint
131-131: Avoid using non-null assertions without checking for null or undefined.- this.leafletMap!.setZoom(this.zoom); + if (this.leafletMap) { + this.leafletMap.setZoom(this.zoom); + }Also applies to: 154-154, 176-176, 177-177, 178-178, 185-185, 189-189
Line range hint
224-228: Consider usingfor...ofloops instead offorEachfor better readability and potential performance benefits.- this._mapItems.forEach((marker) => marker.remove()); + for (const marker of this._mapItems) { + marker.remove(); + }Also applies to: 231-235, 257-257, 263-265, 301-301, 312-379
Line range hint
231-231: Specify a more precise type instead ofany.- this.layers?.forEach((layer: any) => { + this.layers?.forEach((layer: Layer) => {
Line range hint
262-262: Ensure elements are not null before accessing them to avoid runtime errors.- const map = this.renderRoot.querySelector("#map"); + const map = this.renderRoot.querySelector("#map"); + if (!map) return;Also applies to: 326-326, 331-331, 347-347
src/panels/lovelace/components/hui-energy-period-selector.ts (2)
Line range hint
25-25: Optimize imports by marking type-only imports explicitly.- import type { RequestSelectedDetail } from "@material/mwc-list/mwc-list-item"; - import type { UnsubscribeFunc } from "home-assistant-js-websocket"; - import type { DateRangePickerRanges } from "../../../components/ha-date-range-picker"; - import type { EnergyData, getEnergyDataCollection } from "../../../data/energy"; + import { RequestSelectedDetail } from "@material/mwc-list/mwc-list-item"; + import { UnsubscribeFunc } from "home-assistant-js-websocket"; + import { DateRangePickerRanges } from "../../../components/ha-date-range-picker"; + import { EnergyData, getEnergyDataCollection } from "../../../data/energy";Also applies to: 26-33, 56-56, 58-58
Line range hint
293-293: Avoid non-null assertions to ensure robust error handling.Consider adding null checks or using optional chaining (
?.) to handle potential null or undefined values safely.Also applies to: 350-350, 430-431, 439-440, 451-451, 475-476, 532-533, 541-542, 558-559, 572-573, 588-589
src/panels/lovelace/cards/hui-weather-forecast-card.ts (1)
Line range hint
85-85: Avoid non-null assertions to ensure robust error handling.Consider adding null checks or using optional chaining (
?.) to handle potential null or undefined values safely.Also applies to: 108-108, 109-109, 110-110, 191-191, 192-192, 244-244, 245-245, 302-302, 323-323, 344-344, 345-345, 349-349, 352-352, 361-361, 362-362, 368-368, 369-369, 377-377
src/panels/lovelace/cards/hui-media-control-card.ts (2)
Line range hint
170-170: Prefer template literals over string concatenation for better readability and maintainability.- "background-image": `linear-gradient(to right, ${this._backgroundColor}, ${this._backgroundColor + "00"})`, + "background-image": `linear-gradient(to right, ${this._backgroundColor}, ${this._backgroundColor}00)`,
Line range hint
243-243: Consider verifying object existence before usage to avoid potential runtime errors from non-null assertions.- this._config!.entity + this._config?.entity ?? throw new Error("Configuration is undefined.")Apply similar checks for other non-null assertions throughout the file.
Also applies to: 244-244, 280-280, 473-473, 489-489, 499-499, 506-506, 510-510, 519-519, 520-520, 521-521, 529-529, 534-534, 538-538, 547-547, 549-549
src/components/media-player/ha-media-player-browse.ts (5)
Line range hint
9-17: Consider removing unused type imports to clean up the code.- import type { LitVirtualizer } from "@lit-labs/virtualizer"; - import type { MediaPlayerItem } from "../../data/media-player"; - import type { HomeAssistant } from "../../types";These imports are only used for type annotations and can be safely removed if they are not used elsewhere in the code.
Also applies to: 32-39
Line range hint
204-204: Avoid using non-null assertions unless absolutely necessary.Using non-null assertions (
!) bypasses TypeScript's strict null checks, which can lead to runtime errors if the assumptions about non-nullability are incorrect. Consider adding proper checks or handling potential null values gracefully.Also applies to: 217-217, 624-624, 686-686
Line range hint
261-261: Use simple strings instead of template literals when not needed.- backgroundImage = `none`; + backgroundImage = "none";Template literals are unnecessary here since there is no dynamic content being interpolated.
Line range hint
707-707: Avoid reassigning parameters as it can lead to confusing behaviors.- const item = (ev.currentTarget as any).item; + const { item } = ev.currentTarget as any;This change avoids direct reassignment and makes the code cleaner by using destructuring.
Line range hint
720-720: Specify explicit types instead of usingany.Using
anytype defeats the purpose of TypeScript's static typing. It's better to specify explicit types for better type safety and code clarity.- const item = (ev.currentTarget as any).item; + const item = (ev.currentTarget as HTMLElement & { item: MediaPlayerItem }).item;Adjust the type annotations to reflect the actual expected types.
Also applies to: 740-740, 785-785, 849-849
|
Visually no differences on legacy build with polyfill. It just needs the open question to be addressed to merge. |
|
Actually, thinking about it more, it would not make sense to limit where the polyfill gets injected. Since The polyfill sets a global object, that would lead to inconsistent usage depending on how things are bundled or which pages the user decides to visit. |
Proposed change
Inject polyfill for
ResizeObserverusing Babel plugin and remove manual loads. Feature detection is simplified and a top levelawaitis used to only load it when needed.I made sure there were no errors, but since functionally the observer callbacks mostly do visual stuff, please double check the areas where we use them. Note it's easy to force both the polyfill just by changing the feature detection to
if (true), and the legacy build can be forced just by editingindex.htmlto remove the scripts that do imports.Type of change
Example configuration
Additional information
Checklist
If user exposed functionality or configuration variables are added/changed:
Summary by CodeRabbit
New Features
ResizeObserverpolyfill, ensuring compatibility with older browsers.Refactor
ResizeObserveracross multiple components by removing theloadPolyfillIfNeededfunction calls.Chores
These changes improve the performance and maintainability of the application while ensuring better cross-browser compatibility.