Skip to content
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.

Sign up for GitHub

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Small cleanup of lit-plugin errors #303

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fd7ac7f
cleanup
Misiu Sep 21, 2022
6d62811
prettier
Misiu Sep 21, 2022
19ff400
prettier
Misiu Sep 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .vscode/extensions.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"recommendations": [
"dbaeumer.vscode-eslint",
"esbenp.prettier-vscode",
"bierner.lit-html",
"runem.lit-plugin"
]
}
2 changes: 1 addition & 1 deletion src/adopt/adopt-dialog.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class ESPHomeAdoptDialog extends LitElement {
@query("mwc-textfield[name=name]") private _inputName!: TextField;

protected render() {
let heading;
let heading = "";
let content;

if (this._state === "ask") {
Expand Down
2 changes: 1 addition & 1 deletion src/components/process-dialog.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export class ESPHomeProcessDialog extends LitElement {
<mwc-dialog
open
heading=${this.heading}
scrimClickAction
scrimClickAction=""
Copy link
Member

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.

Copy link
Contributor Author

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:
image

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:
image

not sure how to solve that better way instead of extending mwc-dialog and adding custom properties.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More info: https://github.com/material-components/material-components-web/tree/master/packages/mdc-dialog#dialog-actions

Copy link
Member

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.

Copy link
Contributor Author

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:
image

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.

Copy link
Member

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.

@closed=${this._handleClose}
>
<esphome-remote-process
Expand Down
4 changes: 2 additions & 2 deletions src/editor/editor-dialog.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ class ESPHomeEditorDialog extends LitElement {
return html`<mwc-dialog
open
heading=${`Edit ${this.fileName}`}
scrimClickAction
escapeKeyAction
scrimClickAction=""
escapeKeyAction=""
@closed=${this._handleClose}
>
<mwc-snackbar leading></mwc-snackbar>
Expand Down
6 changes: 3 additions & 3 deletions src/install-choose/install-choose-dialog.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class ESPHomeInstallChooseDialog extends LitElement {
private _abortCompilation?: AbortController;

protected render() {
let heading;
let heading = "";
let content;
let hideActions = false;

Expand Down Expand Up @@ -232,7 +232,7 @@ class ESPHomeInstallChooseDialog extends LitElement {
<mwc-dialog
open
heading=${heading}
scrimClickAction
scrimClickAction=""
@closed=${this._handleClose}
.hideActions=${hideActions}
>
Expand All @@ -248,7 +248,7 @@ class ESPHomeInstallChooseDialog extends LitElement {
<mwc-circular-progress
active
?indeterminate=${progress === undefined}
.progress=${progress !== undefined ? progress / 100 : undefined}
.progress=${progress !== undefined ? progress / 100 : 0}
density="8"
></mwc-circular-progress>
${progress !== undefined
Expand Down
6 changes: 3 additions & 3 deletions src/install-web/install-web-dialog.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export class ESPHomeInstallWebDialog extends LitElement {
private _platform?: ValueOf<typeof chipFamilyToPlatform>;

protected render() {
let heading;
let heading = "";
let content;
let hideActions = false;

Expand Down Expand Up @@ -105,7 +105,7 @@ export class ESPHomeInstallWebDialog extends LitElement {
<mwc-dialog
open
heading=${heading}
scrimClickAction
scrimClickAction=""
@closed=${this._handleClose}
.hideActions=${hideActions}
>
Expand All @@ -121,7 +121,7 @@ export class ESPHomeInstallWebDialog extends LitElement {
<mwc-circular-progress
active
?indeterminate=${progress === undefined}
.progress=${progress !== undefined ? progress / 100 : undefined}
.progress=${progress !== undefined ? progress / 100 : 0}
density="8"
></mwc-circular-progress>
${progress !== undefined
Expand Down
2 changes: 1 addition & 1 deletion src/logs-target/logs-target-dialog.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ class ESPHomeLogsTargetDialog extends LitElement {
<mwc-dialog
open
heading=${heading}
scrimClickAction
scrimClickAction=""
@closed=${this._handleClose}
>
${content}
Expand Down
2 changes: 1 addition & 1 deletion src/logs-webserial/logs-webserial-dialog.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class ESPHomeLogsWebSerialDialog extends LitElement {
<mwc-dialog
open
.heading=${this.configuration ? `Logs ${this.configuration}` : "Logs"}
scrimClickAction
scrimClickAction=""
@closed=${this._handleClose}
>
<ewt-console
Expand Down
2 changes: 1 addition & 1 deletion src/no-port-picked/no-port-picked-dialog.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class ESPHomeNoPortPickedDialog extends LitElement {
<mwc-dialog
open
heading="No port selected"
scrimClickAction
scrimClickAction=""
@closed=${this._handleClose}
>
<div>
Expand Down
2 changes: 1 addition & 1 deletion src/rename/rename-dialog.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class ESPHomeRenameDialog extends LitElement {
<mwc-dialog
open
heading=${`Rename ${this.configuration}`}
scrimClickAction
scrimClickAction=""
@closed=${this._handleClose}
>
${this._error ? html`<div class="error">${this._error}</div>` : ""}
Expand Down
2 changes: 1 addition & 1 deletion src/show-api-key/show-api-key-dialog.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class ESPHomeShowApiKeyDialogDialog extends LitElement {
<mwc-dialog
open
heading=${`API key for ${this.configuration}`}
scrimClickAction
scrimClickAction=""
@closed=${this._handleClose}
>
${this._apiKey === undefined
Expand Down
16 changes: 6 additions & 10 deletions src/wizard/wizard-dialog.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ export class ESPHomeWizardDialog extends LitElement {
@query("mwc-textfield[name=password]") private _inputPassword!: TextField;

protected render() {
let heading;
let heading = "";
let content;
let hideActions = false;

Expand Down Expand Up @@ -146,7 +146,7 @@ export class ESPHomeWizardDialog extends LitElement {
<mwc-dialog
open
heading=${heading}
scrimClickAction
scrimClickAction=""
@closed=${this._handleClose}
.hideActions=${hideActions}
>${content}</mwc-dialog
Expand All @@ -161,7 +161,7 @@ export class ESPHomeWizardDialog extends LitElement {
<mwc-circular-progress
active
?indeterminate=${progress === undefined}
.progress=${progress !== undefined ? progress / 100 : undefined}
.progress=${progress !== undefined ? progress / 100 : 0}
density="8"
></mwc-circular-progress>
${progress !== undefined
Expand Down Expand Up @@ -191,11 +191,7 @@ export class ESPHomeWizardDialog extends LitElement {
`;
}

private _renderAskESPHomeWeb(): [
string | undefined,
TemplateResult,
boolean
] {
private _renderAskESPHomeWeb(): [string, TemplateResult, boolean] {
const heading = "New device";
let hideActions = false;
const content = html`
Expand Down Expand Up @@ -245,9 +241,9 @@ export class ESPHomeWizardDialog extends LitElement {
return [heading, content, hideActions];
}

private _renderBasicConfig(): [string | undefined, TemplateResult, boolean] {
private _renderBasicConfig(): [string, TemplateResult, boolean] {
if (this._hasWifiSecrets === undefined) {
return [undefined, this._renderProgress("Initializing"), true];
return ["", this._renderProgress("Initializing"), true];
}
const heading = supportsWebSerial ? "New device" : "Create configuration";
let hideActions = false;
Expand Down
2 changes: 1 addition & 1 deletion web.esphome.io/src/install-adoptable/install-adoptable-dialog.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class ESPHomeInstallAdoptableDialog extends LitElement {
<mwc-dialog
open
heading="Prepare your device for adoption"
scrimClickAction
scrimClickAction=""
@closed=${this._handleClose}
>
<div>
Expand Down
2 changes: 1 addition & 1 deletion web.esphome.io/src/install-upload/install-upload-dialog.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class ESPHomeInstallUploadDialog extends LitElement {
<mwc-dialog
open
heading="Install your existing ESPHome project"
scrimClickAction
scrimClickAction=""
@closed=${this._handleClose}
>
<div>Select the project that you want to install on your device.</div>
Expand Down