-
Notifications
You must be signed in to change notification settings - Fork 56
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
Small cleanup of lit-plugin errors #303
base: main
Are you sure you want to change the base?
Conversation
any ideas why |
@@ -24,7 +24,7 @@ export class ESPHomeProcessDialog extends LitElement { | |||
<mwc-dialog | |||
open | |||
heading=${this.heading} | |||
scrimClickAction | |||
scrimClickAction="" |
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.
I don't like this. This is not an improvement.
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.
I agree this doesn't look pretty, but without this I get this error in VS Code:
same with lit-analyzer
:
Type 'true' is not assignable to 'string' 27: scrimClickAction no-incompatible-type-binding
both properties (scrimClickAction, escapeKeyAction) are of type string:
not sure how to solve that better way instead of extending mwc-dialog
and adding custom properties.
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.
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.
Attributes are always strings, a string without a value is interpreted as an empty string in HTML. This sounds like a bug in lit-analyzer.
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.
Take a look at https://github.com/esphome/dashboard/blob/main/src/components/process-dialog.ts#L25-L27.
In VS Code I see an error for scrimClickAction
:
Looking at the definition of mwc-dialog (mwc-dialog-base.d.ts) I see this:
/**
* @license
* Copyright 2019 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
import 'blocking-elements';
import 'wicg-inert';
import { MDCDialogAdapter } from '@material/dialog/adapter.js';
import MDCDialogFoundation from '@material/dialog/foundation.js';
import { BaseElement } from '@material/mwc-base/base-element.js';
export { MDCDialogCloseEventDetail } from '@material/dialog/types.js';
export declare class DialogBase extends BaseElement {
protected mdcRoot: HTMLDivElement;
protected primarySlot: HTMLElement;
protected secondarySlot: HTMLElement;
protected contentSlot: HTMLElement;
protected contentElement: HTMLDivElement;
protected conatinerElement: HTMLDivElement;
hideActions: boolean;
stacked: boolean;
heading: string;
scrimClickAction: string;
escapeKeyAction: string;
open: boolean;
defaultAction: string;
actionAttribute: string;
initialFocusAttribute: string;
set suppressDefaultPressSelector(selector: string);
/**
* @export
*/
get suppressDefaultPressSelector(): string;
protected closingDueToDisconnect?: boolean;
protected initialSupressDefaultPressSelector: string;
protected get primaryButton(): HTMLElement | null;
protected currentAction: string | undefined;
protected mdcFoundationClass: typeof MDCDialogFoundation;
protected mdcFoundation: MDCDialogFoundation;
protected boundHandleClick: ((ev: MouseEvent) => void) | null;
protected boundHandleKeydown: ((ev: KeyboardEvent) => void) | null;
protected boundHandleDocumentKeydown: ((ev: KeyboardEvent) => void) | null;
protected emitNotification(name: string, action?: string): void;
protected getInitialFocusEl(): HTMLElement | null;
protected searchNodeTreesForAttribute(nodes: Node[], attribute: string): HTMLElement | null;
protected createAdapter(): MDCDialogAdapter;
protected render(): import("lit-html").TemplateResult<1>;
protected renderHeading(): import("lit-html").TemplateResult<1>;
protected firstUpdated(): void;
connectedCallback(): void;
disconnectedCallback(): void;
forceLayout(): void;
focus(): void;
blur(): void;
protected setEventListeners(): void;
protected removeEventListeners(): void;
close(): void;
show(): void;
}
open
is a type boolean (no error), but scrimClickAction
is a type string (we get an error), I think that's the reason for the error.
Saying that I'm not sure if this is an error in the lit-plugin (vs code extensions) or if should this be fixed in the dashboard code base.
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.
https://github.com/lit/lit/blob/main/packages/reactive-element/src/reactive-element.ts#L333
Lit's fromAttribute convertor has no case for string, so it treats strings just as they get it from the HTML DOM object. An empty attribute is returned as an empty string. That is implicit. It's a bug in lit analyzer that it does not accept an empty attribute for a string value. It's not that it didn't extract the type correctly.
I've added recommended extensions (same as in HA Frontend).
I've also removed some (not all) lit-plugin errors.
I'll try to clean the rest in the next PR (plus replace confirm update all with a proper dialog)