dart-archive / angular_analyzer_plugin

WORK MOVED TO dart-lang/angular repository
https://github.com/dart-lang/angular/tree/master/angular_analyzer_plugin
68 stars 13 forks source link

Better support for directives without dashes, validation of html tags #248

Open MichaelRFairhurst opened 7 years ago

MichaelRFairhurst commented 7 years ago

Current html tag detection simply boils down to "does it have a dash." If it does its considered a component, and if it doesn't its considered an html tag.

This creates problems with transclusions:

@Component(
  selector: 'mypaneltag',
  template: '<ng-content select="foo-bar"></ng-content>'
)
class ...
  <mypaneltag>
    <foo-bar>...</foo-bar>
  </mypaneltag>

This reports "unknown tag foo-bar" because it thinks "mypaneltag" is a native tag, doesn't search for transclusions, and then "foo-bar" doesn't get the special treatment it deserves.

It also means we don't catch misspelled html tags:

  <dvi class="...

The former is easy to solve, the latter is a bit trickier because not all tags are described in dart:html, and it'd be a good idea to keep it forward compatible by accepting unknown tags. There's also complications like svg.

Perhaps best is to see close matches and suggest "did you mean div?" but there's also the option of using @NgIgnoreErrors for people using new tags. We'd just have to put reasonable effort into capturing the current spec well before I'd justify that.

mk13 commented 7 years ago

We could possibly add the 1-off mispelled error detection and recovery into the angular_ast. It's already scanning for seemingly 'dangling close tags', we could add another layer of logic such that in the case of a dangling close tag, we can see if it is 1-off, and if so - raise 'did you mean X' error and recover to treat it as such.

MichaelRFairhurst commented 7 years ago

Reading this again, I wonder if we can handle one-off misspellings for more than just native elements.

It'd be cool if we could massage <my-rndom-component into <my-random-component, because otherwise (like in the case of div), we flag a huge number of secondary errors, and can't analyze let bindings like #randomcomp and #thediv...

mk13 commented 7 years ago

The first issue is now resolved correct?

MichaelRFairhurst commented 7 years ago

Yeah the former is solved. I believe I didn't mess with the "is native html" logic, but did mess with the "should check transclusions" logic. It used to only check transclusions for non-html, but now it checks transclusions for everything that 1. didn't match a component defined in dart:html, and 2. matched a component.

In terms of the second issue, we could report issues with tags that don't have any components matched, regardless of dashes, but it means that we must have all html tags in dart:html or supplement that somehow.

mk13 commented 7 years ago

One possibility is to also define all standard HTML tags within our project while also importing dart:html and deduping similar ones found in both; it should at least give us some control rather than being fully dependent on dart:html.

zoechi commented 7 years ago

There is also always the possibility to use custom tag names that are not Angular components intentionally. For example when Polymer components are used with Angular.

Angular (TS) introduced CUSTOM_ELEMENTS_SCHEMA to inform Angular that there are custom elements which are not supposed to be Angular components https://github.com/angular/angular/blob/master/CHANGELOG.md#200-rc5-2016-08-09 I don't know details about how this is implemented.

Some way to register custom elements somewhere in Angular or the analyzer plugin for them to be ignored would be my preferred solution. This would also work well enough for me for element names that are not provided by dart:html (I guess that wouldn't be many and also less common ones.