runem / lit-analyzer

Monorepository for tools that analyze lit-html templates
MIT License
317 stars 36 forks source link

lit analyzer does not accept an empty attribute for a string value - false positive? #279

Open Misiu opened 1 year ago

Misiu commented 1 year ago

I'm trying to contribute to https://github.com/esphome/dashboard and learn typescript and lit doing that. Right now I'm trying to add lit-analyzer package to repo and fix all errors is thows. One of the errors I noticed is: image

that is the empty value of the scrimClickAction attribute.

After looking at the definition of mwc-dialog I can 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;
}

the scrimClickAction attribute has a string as a type, but as suggested here:

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.

According to the docs (https://github.com/material-components/material-web/tree/mwc/packages/dialog#propertiesattributes) setting an empty value will prevent closing the dialog, this is the behavior I want, so setting an empty attribute without an empty value should be fine. The project I mentioned (esphome/dashboard) works perfectly with empty values for scrimClickAction, so I'd like to know if this is a correct error or a bug in lit-analyzer?

Misiu commented 1 year ago

still an error

Misiu commented 11 months ago

Still an issue. Ref: https://github.com/esphome/dashboard/pull/303#discussion_r978755319