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

Feature request: require custom elements be registered on HTMLElementTagNameMap #73

Closed
rictic opened this issue Dec 19, 2019 · 11 comments · Fixed by #112
Closed

Feature request: require custom elements be registered on HTMLElementTagNameMap #73

rictic opened this issue Dec 19, 2019 · 11 comments · Fixed by #112
Labels
enhancement New feature or request

Comments

@rictic
Copy link
Collaborator

rictic commented Dec 19, 2019

A pass that would diagnose when a custom element is recognized in source code but it hasn't be declared on the HTMLElementTagNameMap, or if it is declared with the wrong tag name. For example:

Warns:

import {customElement, LitElement} from 'lit-element';
@customElement('foo-bar')
class FooBar extends LitElement {}
//    ~~~~~~  warning!

Warns:

import {customElement, LitElement} from 'lit-element';
@customElement('foo-bar')
class FooBar extends LitElement {}

declare global {
  interface HTMLElementTagNameMap {
    'fizz-buzz': FooBar;
//  ~~~~~~~~~~~ warning!
  }
}

Does not warn:

import {customElement, LitElement} from 'lit-element';
@customElement('foo-bar')
class FooBar extends LitElement {}

declare global {
  interface HTMLElementTagNameMap {
    'foo-bar': FooBar;
  }
}
@runem
Copy link
Owner

runem commented Dec 27, 2019

Great idea, I love this rule! This would be possible to implement after the performance branch has been finished which includes the new version of web-component-analyzer. The new version of WCA emits all definitions of the same element and not just the last one found.

@runem runem added the enhancement New feature or request label Jan 19, 2020
@e111077
Copy link

e111077 commented Mar 10, 2020

Edit: Ignore this comment

Alternative, what's wrong with adding types to @fires?

// my-element.ts
/**
 * @fires my-event {MyEventDetail}
 */
export class MyEvent extends LitElement { ... }
// main.ts
html`
  <!-- type checking happens here -->
  <my-element @my-event=${this.onMyEvent}></my-element>
...
`;

onMyEvent(e: MyEventDetail) { ... }

@rictic
Copy link
Collaborator Author

rictic commented Mar 10, 2020

+1 to checking events (I believe there's an event map as well, though it's not element-specific), but that should be a different issue IMO

@e111077
Copy link

e111077 commented Mar 10, 2020

will file

@runem
Copy link
Owner

runem commented Mar 10, 2020

I'm currently working on this branch: https://github.com/runem/lit-analyzer/tree/1.2.0 which should be able to support building this rule :-) Lately I have been caught up in web-component-analyzer-specific work, but next I'll take a look at implementing this rule.

@e111077
Copy link

e111077 commented Mar 10, 2020

oh wow I'm dumb; I totally read this as Custom Events not Custom Elements haha >.< issue filed on #85

@runem
Copy link
Owner

runem commented Jul 7, 2020

I just implemented the rule on 1.2.0 :-)

The rule is called no-missing-element-type-definition and is disabled by default both when running strict and non-strict. Please let me know if you have any comments or any additions that you want me to add to this feature.

demo

@justinfagnani
Copy link
Collaborator

That quickfix is awesome!!

@rictic
Copy link
Collaborator Author

rictic commented Jul 9, 2020

Awesome!

@rictic rictic closed this as completed Jul 9, 2020
@WickyNilliams
Copy link

I've not been able to get this working - is it broken with lit@2. See the attached screenshot. No error reported

image

@WickyNilliams
Copy link

Sorry I take it back, it is working now. Not sure what happened there!

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

Successfully merging a pull request may close this issue.

5 participants