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.

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

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

Open
Misiu opened this issue Sep 26, 2022 · 2 comments

Comments

@Misiu
Copy link

Misiu commented Sep 26, 2022

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
Copy link
Author

Misiu commented Jul 10, 2023

still an error

@Misiu
Copy link
Author

Misiu commented Sep 13, 2023

Still an issue.
Ref: esphome/dashboard#303 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant