20240627.0 #21192
20240627.0 #21192
Conversation
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
* Fix device integration filter for entityless devices * code review
* Wait for hass and config before building the card * Don't use setter * Improve code readability * Use hasupdated * Rename build to load
* Fix size of cards inside stack cards * Apply style everywhere * Fix stack * Fix grid stack * Set block only for square
WalkthroughWalkthroughThe updates enhance QR and barcode scanning functionality, improve error handling, and integrate more configuration data into selectors in Home Assistant's frontend. New class attributes, methods, and state properties were introduced to support these changes. Additionally, several Lovelace dashboard cards and components now support loading elements dynamically and include new content options. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant HaQrScanner
participant ExternalScanner
participant HaComponent
User->>+HaQrScanner: Start scanning
HaQrScanner->>ExternalScanner: _openExternalScanner()
ExternalScanner-->>HaQrScanner: Scanner ready
HaQrScanner->>ExternalScanner: _notifyExternalScanner("start")
ExternalScanner->>HaQrScanner: _qrCodeScanned(qrCodeString)
HaQrScanner->>HaQrScanner: Process QR code
HaQrScanner->>ExternalScanner: _closeExternalScanner()
HaQrScanner-->>User: Display scanned code
User->>+HaComponent: Integrate scanned data
HaComponent-->>User: Data integrated successfully
Recent review detailsConfiguration used: CodeRabbit UI Files ignored due to path filters (1)
Files selected for processing (21)
Files not reviewed due to errors (2)
Additional context usedBiome
Gitleaks
Additional comments not posted (26)
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: 11
Outside diff range comments (10)
src/panels/lovelace/cards/hui-horizontal-stack-card.ts (1)
Line range hint
26-26: Address the use ofsuperin a static context.Using
superin static methods can lead to confusion and potential errors. Consider using the class name instead.- super.sharedStyles, + HuiStackCard.sharedStyles,src/panels/lovelace/cards/hui-grid-card.ts (1)
Line range hint
73-73: Address the use ofsuperin a static context.Similar to the previous file, using
superin static methods can lead to confusion. Consider using the class name instead.- super.sharedStyles, + HuiStackCard.sharedStyles,src/components/ha-selector/ha-selector-device.ts (1)
Line range hint
147-147: Avoid non-null assertion.Using non-null assertions can lead to runtime errors if assumptions about non-nullability prove incorrect.
- this._element!.hass = this.hass; + if (this._element) this._element.hass = this.hass;src/components/ha-selector/ha-selector-area.ts (1)
Line range hint
29-29: Address the use ofanytype forvalueproperty.Using
anytype can lead to potential bugs and maintenance issues. Specifying a more precise type would improve type safety.- @property() public value?: any; + @property() public value?: AreaRegistryEntry[] | AreaRegistryEntry;src/components/ha-selector/ha-selector-floor.ts (1)
Line range hint
29-29: Address the use ofanytype forvalueproperty.Using
anytype can lead to potential bugs and maintenance issues. Specifying a more precise type would improve type safety.- @property() public value?: any; + @property() public value?: FloorRegistryEntry[] | FloorRegistryEntry;src/panels/lovelace/cards/hui-entity-filter-card.ts (2)
Line range hint
147-147: Replace non-null assertions with optional chaining.Several lines in this file use non-null assertions (
!), which can lead to runtime errors if the objects are actuallynull. It's safer to replace these with optional chaining (?.).- this.hass!.states[entityConf.entity] + this.hass?.states[entityConf.entity] - this._element!.config + this._element?.config - this._oldEntities![idx] + this._oldEntities?[idx] - this._layoutElement!.hass + this._layoutElement?.hassAlso applies to: 150-150, 155-155, 158-158, 177-177, 185-185, 190-190
Line range hint
208-208: Refactor function to reduce complexity.The
updatefunction has a complexity score of 22, which exceeds the maximum allowed of 15. Consider refactoring this function to simplify its logic, possibly by breaking it down into smaller, more manageable functions.src/panels/lovelace/editor/card-editor/hui-card-layout-editor.ts (2)
Line range hint
185-185: Avoid using the delete operator for better performance.Using the delete operator can lead to performance issues due to deoptimizations in JavaScript engines. Consider setting properties to
undefinedinstead.- delete newConfig.layout_options!.grid_columns; + newConfig.layout_options!.grid_columns = undefined; - delete newConfig.layout_options!.grid_rows; + newConfig.layout_options!.grid_rows = undefined; - delete newConfig.layout_options; + newConfig.layout_options = undefined;Also applies to: 204-204, 207-207, 210-210
Line range hint
203-203: Replace non-null assertions with optional chaining for safety.Using non-null assertions can lead to runtime errors if the object is null. It's safer to use optional chaining.
- this._yamlEditor!.setValue({}); + this._yamlEditor?.setValue({}); - this._cardElement!.config + this._cardElement?.configAlso applies to: 206-206, 209-209
src/panels/config/integrations/integration-panels/zwave_js/dialog-zwave_js-add-node.ts (1)
Line range hint
685-686: Avoid non-null assertions and enhance error handling in_validateDskAndEnterPin.Using non-null assertions can lead to runtime errors if assumptions about non-nullability fail. Consider using optional chaining or conditional checks instead.
- await zwaveValidateDskAndEnterPin( - this.hass, - this._entryId!, - this._pinInput!.value as string - ); + if (this._pinInput && this._entryId) { + await zwaveValidateDskAndEnterPin( + this.hass, + this._entryId, + this._pinInput.value as string + ); + } else { + // Handle error or invalid state + }
| private _qrCodeError(ev: CustomEvent): void { | ||
| this._error = ev.detail.message; | ||
| } |
There was a problem hiding this comment.
Specify a more detailed type for event parameter in _qrCodeError.
Using any for the event type reduces type safety. Consider specifying a more detailed type, possibly a custom event type with a defined structure for detail.message.
- private _qrCodeError(ev: CustomEvent): void {
+ private _qrCodeError(ev: CustomEvent<{ message: string }>): void {Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private _qrCodeError(ev: CustomEvent): void { | |
| this._error = ev.detail.message; | |
| } | |
| private _qrCodeError(ev: CustomEvent<{ message: string }>): void { | |
| this._error = ev.detail.message; | |
| } |
| private _qrCodeError(ev: CustomEvent): void { | ||
| this._error = ev.detail.message; | ||
| } | ||
|
|
There was a problem hiding this comment.
Refactor to reduce complexity and improve maintainability in _handleQrCodeScanned.
This method's complexity is too high, which can lead to maintenance challenges and bugs. Consider breaking it down into smaller methods, each handling a part of the QR code processing logic.
- private async _handleQrCodeScanned(qrCodeString: string): Promise<void> {
+ private async _handleQrCodeScanned(qrCodeString: string): Promise<void> {
+ // Example refactoring: separate the validation and processing steps into their own methods
+ if (!this._validateQrCode(qrCodeString)) {
+ return;
+ }
+ await this._processQrCode(qrCodeString);
+ }
+
+ private _validateQrCode(qrCodeString: string): boolean {
+ // Add validation logic here
+ return true;
+ }
+
+ private async _processQrCode(qrCodeString: string): Promise<void> {
+ // Add processing logic here
+ }Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private async _handleQrCodeScanned(qrCodeString: string): Promise<void> { | |
| // Example refactoring: separate the validation and processing steps into their own methods | |
| if (!this._validateQrCode(qrCodeString)) { | |
| return; | |
| } | |
| await this._processQrCode(qrCodeString); | |
| } | |
| private _validateQrCode(qrCodeString: string): boolean { | |
| // Add validation logic here | |
| return true; | |
| } | |
| private async _processQrCode(qrCodeString: string): Promise<void> { | |
| // Add processing logic here | |
| } |
| @@ -30,7 +30,7 @@ export class HuiHorizontalStackCard extends HuiStackCard { | |||
| height: 100%; | |||
| gap: var(--horizontal-stack-card-gap, var(--stack-card-gap, 8px)); | |||
| } | |||
| #root > * { | |||
| #root > hui-card > * { | |||
There was a problem hiding this comment.
CSS modification appears correct but consider simplifying the selector.
The CSS change aims to ensure flex properties are applied correctly. However, the selector might be more complex than necessary.
- #root > hui-card > * {
+ #root > * {
flex: 1 1 0;
min-width: 0;
}Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #root > hui-card > * { | |
| #root > * { | |
| flex: 1 1 0; | |
| min-width: 0; | |
| } |
| @@ -27,6 +28,8 @@ export class HaDeviceSelector extends LitElement { | |||
|
|
|||
| @state() private _entitySources?: EntitySources; | |||
|
|
|||
| @state() private _configEntries?: ConfigEntry[]; | |||
|
|
|||
| @property() public value?: any; | |||
There was a problem hiding this comment.
Address the use of any type for value property.
Using any type can lead to potential bugs and maintenance issues. Specifying a more precise type would improve type safety.
- @property() public value?: any;
+ @property() public value?: DeviceRegistryEntry[] | DeviceRegistryEntry;Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @property() public value?: any; | |
| @property() public value?: DeviceRegistryEntry[] | DeviceRegistryEntry; |
Tools
Biome
[error] 33-33: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
| if (!this._configEntries && this._hasIntegration(this.selector)) { | ||
| this._configEntries = []; | ||
| getConfigEntries(this.hass).then((entries) => { | ||
| this._configEntries = entries; | ||
| }); | ||
| } |
There was a problem hiding this comment.
Ensure proper handling of asynchronous operations.
The method to fetch configuration entries is correctly implemented using promises. However, consider adding error handling for the promise to manage potential failures in fetching the configuration entries.
getConfigEntries(this.hass).then((entries) => {
this._configEntries = entries;
+ }).catch((error) => {
+ console.error("Failed to fetch configuration entries:", error);
+ this._configEntries = [];
+ });Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!this._configEntries && this._hasIntegration(this.selector)) { | |
| this._configEntries = []; | |
| getConfigEntries(this.hass).then((entries) => { | |
| this._configEntries = entries; | |
| }); | |
| } | |
| if (!this._configEntries && this._hasIntegration(this.selector)) { | |
| this._configEntries = []; | |
| getConfigEntries(this.hass).then((entries) => { | |
| this._configEntries = entries; | |
| }).catch((error) => { | |
| console.error("Failed to fetch configuration entries:", error); | |
| this._configEntries = []; | |
| }); | |
| } |
| deviceIntegrations[entity.device_id!] = | ||
| deviceIntegrations[entity.device_id!] || new Set<string>(); | ||
| deviceIntegrations[entity.device_id!].add(source.domain); |
There was a problem hiding this comment.
Avoid non-null assertion.
Using non-null assertions can lead to runtime errors if assumptions about non-nullability prove incorrect.
- deviceIntegrations[entity.device_id!] = deviceIntegrations[entity.device_id!] || new Set<string>();
+ if (entity.device_id) {
+ deviceIntegrations[entity.device_id] = deviceIntegrations[entity.device_id] || new Set<string>();
+ }Committable suggestion was skipped due to low confidence.
Tools
Biome
[error] 158-158: Forbidden non-null assertion.
(lint/style/noNonNullAssertion)
[error] 159-159: Forbidden non-null assertion.
(lint/style/noNonNullAssertion)
[error] 160-160: Forbidden non-null assertion.
(lint/style/noNonNullAssertion)
| entities: EntityRegistryDisplayEntry[] | EntityRegistryEntry[], | ||
| devices?: DeviceRegistryEntry[], | ||
| configEntries?: ConfigEntry[] | ||
| ): Record<string, Set<string>> => { |
There was a problem hiding this comment.
Reduce function complexity.
The function complexity is high, which can make maintenance difficult. Consider refactoring to simplify the logic, possibly by breaking down into smaller helper functions.
- export const getDeviceIntegrationLookup = (entitySources, entities, devices, configEntries) => {
+ // Refactor into smaller functions
+ function handleEntityIntegrations(entitySources, entities, deviceIntegrations) {
+ for (const entity of entities) {
+ const source = entitySources[entity.entity_id];
+ if (!source?.domain || entity.device_id === null) {
+ continue;
+ }
+ if (!deviceIntegrations[entity.device_id]) {
+ deviceIntegrations[entity.device_id] = new Set<string>();
+ }
+ deviceIntegrations[entity.device_id].add(source.domain);
+ }
+ }
+ function handleDeviceIntegrations(devices, configEntries, deviceIntegrations) {
+ if (devices && configEntries) {
+ for (const device of devices) {
+ for (const config_entry_id of device.config_entries) {
+ const entry = configEntries.find((e) => e.entry_id === config_entry_id);
+ if (entry?.domain) {
+ if (!deviceIntegrations[device.id]) {
+ deviceIntegrations[device.id] = new Set<string>();
+ }
+ deviceIntegrations[device.id].add(entry.domain);
+ }
+ }
+ }
+ }
+ }
+ export const getDeviceIntegrationLookup = (entitySources, entities, devices, configEntries) => {
+ const deviceIntegrations: Record<string, Set<string>> = {};
+ handleEntityIntegrations(entitySources, entities, deviceIntegrations);
+ handleDeviceIntegrations(devices, configEntries, deviceIntegrations);
+ return deviceIntegrations;
+ };Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ): Record<string, Set<string>> => { | |
| // Refactor into smaller functions | |
| function handleEntityIntegrations(entitySources, entities, deviceIntegrations) { | |
| for (const entity of entities) { | |
| const source = entitySources[entity.entity_id]; | |
| if (!source?.domain || entity.device_id === null) { | |
| continue; | |
| } | |
| if (!deviceIntegrations[entity.device_id]) { | |
| deviceIntegrations[entity.device_id] = new Set<string>(); | |
| } | |
| deviceIntegrations[entity.device_id].add(source.domain); | |
| } | |
| } | |
| function handleDeviceIntegrations(devices, configEntries, deviceIntegrations) { | |
| if (devices && configEntries) { | |
| for (const device of devices) { | |
| for (const config_entry_id of device.config_entries) { | |
| const entry = configEntries.find((e) => e.entry_id === config_entry_id); | |
| if (entry?.domain) { | |
| if (!deviceIntegrations[device.id]) { | |
| deviceIntegrations[device.id] = new Set<string>(); | |
| } | |
| deviceIntegrations[device.id].add(entry.domain); | |
| } | |
| } | |
| } | |
| } | |
| } | |
| export const getDeviceIntegrationLookup = (entitySources, entities, devices, configEntries) => { | |
| const deviceIntegrations: Record<string, Set<string>> = {}; | |
| handleEntityIntegrations(entitySources, entities, deviceIntegrations); | |
| handleDeviceIntegrations(devices, configEntries, deviceIntegrations); | |
| return deviceIntegrations; | |
| }; |
Tools
Biome
[error] 149-149: Excessive complexity detected.
Please refactor this function to reduce its complexity score from 17 to the max allowed complexity 15.
(lint/complexity/noExcessiveCognitiveComplexity)
| element.hass = this.hass; | ||
| element.preview = this.preview; | ||
| if (this.hass) { | ||
| this._element!.hass = this.hass; |
There was a problem hiding this comment.
Avoid non-null assertion.
Using non-null assertions can lead to runtime errors if assumptions about non-nullability prove incorrect.
- this._element!.hass = this.hass;
+ if (this._element) this._element.hass = this.hass;Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| this._element!.hass = this.hass; | |
| if (this._element) this._element.hass = this.hass; |
Tools
Biome
[error] 105-105: Forbidden non-null assertion.
(lint/style/noNonNullAssertion)
| // For backwards compatibility | ||
| (element as any).editMode = this.preview; | ||
| (this._element as any).editMode = this.preview; |
There was a problem hiding this comment.
Avoid using any type for editMode.
Using any type can lead to potential bugs and maintenance issues. It's better to define a more specific type or at least add type checks before assignment.
- (this._element as any).editMode = this.preview;
+ if (typeof this._element !== 'undefined') {
+ (this._element as LovelaceCard).editMode = this.preview;
+ }Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| (this._element as any).editMode = this.preview; | |
| if (typeof this._element !== 'undefined') { | |
| (this._element as LovelaceCard).editMode = this.preview; | |
| } |
Tools
Biome
[error] 94-94: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
| protected update(changedProps: PropertyValues<typeof this>) { | ||
| super.update(changedProps); | ||
|
|
||
| if (this._element) { | ||
| if (changedProps.has("config") && this.hasUpdated) { | ||
| const oldConfig = changedProps.get("config"); | ||
| if (this.config !== oldConfig && this.config) { | ||
| const typeChanged = this.config?.type !== oldConfig?.type; | ||
| if (typeChanged) { | ||
| this._loadElement(this.config); | ||
| } else { | ||
| this._element?.setConfig(this.config); | ||
| fireEvent(this, "card-updated"); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Reduce function complexity in update method.
The update method is complex and handles multiple conditions which can be simplified or broken down into smaller methods to improve readability and maintainability.
- protected update(changedProps: PropertyValues<typeof this>) {
+ // Simplify by breaking down into smaller methods
+ protected update(changedProps: PropertyValues<typeof this>) {
+ super.update(changedProps);
+ this._updateConfig(changedProps);
+ this._updateHass(changedProps);
+ this._updatePreview(changedProps);
+ this._updatePanel(changedProps);
+ }
+
+ private _updateConfig(changedProps: PropertyValues<typeof this>) {
+ if (changedProps.has("config") && this.hasUpdated) {
+ const oldConfig = changedProps.get("config");
+ if (this.config !== oldConfig && this.config) {
+ const typeChanged = this.config?.type !== oldConfig?.type;
+ if (typeChanged) {
+ this._loadElement(this.config);
+ } else {
+ this._element?.setConfig(this.config);
+ fireEvent(this, "card-updated");
+ }
+ }
+ }
+ }
+
+ private _updateHass(changedProps: PropertyValues<typeof this>) {
+ if (changedProps.has("hass")) {
+ try {
+ if (this.hass) {
+ this._element.hass = this.hass;
+ }
+ } catch (e: any) {
+ this._loadElement(createErrorCardConfig(e.message, null));
+ }
+ }
+ }
+
+ private _updatePreview(changedProps: PropertyValues<typeof this>) {
+ if (changedProps.has("preview")) {
+ try {
+ this._element.preview = this.preview;
+ // For backwards compatibility
+ (this._element as any).editMode = this.preview;
+ } catch (e: any) {
+ this._loadElement(createErrorCardConfig(e.message, null));
+ }
+ }
+ }
+
+ private _updatePanel(changedProps: PropertyValues<typeof this>) {
+ if (changedProps.has("isPanel")) {
+ this._element.isPanel = this.isPanel;
+ }
+ }Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| protected update(changedProps: PropertyValues<typeof this>) { | |
| super.update(changedProps); | |
| if (this._element) { | |
| if (changedProps.has("config") && this.hasUpdated) { | |
| const oldConfig = changedProps.get("config"); | |
| if (this.config !== oldConfig && this.config) { | |
| const typeChanged = this.config?.type !== oldConfig?.type; | |
| if (typeChanged) { | |
| this._loadElement(this.config); | |
| } else { | |
| this._element?.setConfig(this.config); | |
| fireEvent(this, "card-updated"); | |
| } | |
| } | |
| } | |
| protected update(changedProps: PropertyValues<typeof this>) { | |
| super.update(changedProps); | |
| this._updateConfig(changedProps); | |
| this._updateHass(changedProps); | |
| this._updatePreview(changedProps); | |
| this._updatePanel(changedProps); | |
| } | |
| private _updateConfig(changedProps: PropertyValues<typeof this>) { | |
| if (changedProps.has("config") && this.hasUpdated) { | |
| const oldConfig = changedProps.get("config"); | |
| if (this.config !== oldConfig && this.config) { | |
| const typeChanged = this.config?.type !== oldConfig?.type; | |
| if (typeChanged) { | |
| this._loadElement(this.config); | |
| } else { | |
| this._element?.setConfig(this.config); | |
| fireEvent(this, "card-updated"); | |
| } | |
| } | |
| } | |
| } | |
| private _updateHass(changedProps: PropertyValues<typeof this>) { | |
| if (changedProps.has("hass")) { | |
| try { | |
| if (this.hass) { | |
| this._element.hass = this.hass; | |
| } | |
| } catch (e: any) { | |
| this._loadElement(createErrorCardConfig(e.message, null)); | |
| } | |
| } | |
| } | |
| private _updatePreview(changedProps: PropertyValues<typeof this>) { | |
| if (changedProps.has("preview")) { | |
| try { | |
| this._element.preview = this.preview; | |
| // For backwards compatibility | |
| (this._element as any).editMode = this.preview; | |
| } catch (e: any) { | |
| this._loadElement(createErrorCardConfig(e.message, null)); | |
| } | |
| } | |
| } | |
| private _updatePanel(changedProps: PropertyValues<typeof this>) { | |
| if (changedProps.has("isPanel")) { | |
| this._element.isPanel = this.isPanel; | |
| } | |
| } |
Tools
Biome
[error] 134-134: Excessive complexity detected.
Please refactor this function to reduce its complexity score from 24 to the max allowed complexity 15.
(lint/complexity/noExcessiveCognitiveComplexity)
Summary by CodeRabbit
New Features
last-updatedoption for state content in tile cards.Bug Fixes
Style
Chores
@types/chromecast-caf-receiverto version 6.0.16.