aurelia / template-lint

Sanity check of Aurelia-flavor template HTML
Apache License 2.0
56 stars 17 forks source link

Fix View-Model <-> View Resolution #113

Closed atsu85 closed 6 years ago

atsu85 commented 7 years ago

Let's say I have 2 files:

my-component.ts
my-component.html

and my-component.ts contains smth like:

export class Helper {
...
}
export class MyComponent {
...
  x: Helper;
}

and when field x is used in template, then linter emits error:

ERROR: cannot find 'x' in type 'Helper '

Right now the error message is misleading.

PS. While I think it is good idea to put component class as the first class in TS file, it shouldn't be a restriction for the linter (ideally there could be another rule that could be enabled/disabled to check if VM class is the first class in source code) - linter should match the VM class using same rules (naming conventions in this case) as it is done by Aurelia.

MeirionHughes commented 7 years ago

I'm pretty sure I spoke to Rob about this a while back and the convention is Aurelia expects the vm to be the first exported class. i.e. you can change MyComponent to BlahBlah and it will still work.

atsu85 commented 7 years ago

@MeirionHughes, that only applies for routable components, but not for custom elements. For example if I rename custom element class (that doesn't have annotation that changes the name of custom element tag) to smth else and try to reload the page, then custom element is not rendered.

And if I just add another class before custom element class, then custom element still works as expected, but template-lint produces the error i described above.

@EisenbergEffect - this seems a bit inconsistent, why should routable components (view's) act differently compared to custom elements?

MeirionHughes commented 7 years ago

hmm okay. There is the useView and noView decorators to contend with too. (view-model picking whatever view it wants). Need to think about this some more...

jdanyow commented 7 years ago

May need to setup some of the scenarios and debug them. The ModuleAnalyzer and it's callers would be a good place to start.

MeirionHughes commented 7 years ago

Main issue I have here is I don't (can't) know the context for with a html is used from; i.e. we lint a html file and try to work out what it is; aurelia's runtime has more to go on in this regard.

If the [html+ts] combo is a custom-element, then, to be used correctly anyway, the corresponding source file should have a class called SomethingCustomElement, if it doesn't them we'll have to assume the html file is part of a route and that the first exported class is the viewmodel.

So I'm going to go with:

"some-thing.html" -> "some-thing.[ts|js]" -> "SomeThing"

If "some-thing.[ts|js]" has class SomeThingCustomElement then use that (Custom Element) If "some-thing.[ts|js]" first export is class "SomeThing" then use that (Custom Element) If "some-thing.[ts|js]" first export is class "Anything" then use that (Route)

atsu85 commented 7 years ago

@MeirionHughes, please make the function that (decides if it is custom element or route view) configurable, so I could configure the linter based my naming conventions.

MeirionHughes commented 7 years ago

@atsu85 I can, but...

Can you make a gistrun of what you're doing differently? Your example in the first post doesn't work for me. https://gist.run/?id=c2a7e42b7d835923b2ec5f522d654d9b if I rename it to MyComponentCustomElement it works. if I change MyComponent to be the first export, it works. if I change it to class Anything as first export, it doesn't work as Custom Element.

The three rules I outlined above seem to be exactly how aurelia works by default.

MeirionHughes commented 7 years ago

I think I need to change how the linter is used; currently it comes at the project from the html files then tries to find view-models / source; there are quite a few ways for that to go wrong. see https://github.com/MeirionHughes/aurelia-template-lint/issues/151

Perhaps I need to try to come at it from the source files; specifically the main / app entry points and then the router module paths. This is how aurelia does it (unless you require a html file directly). :/

atsu85 commented 7 years ago

I think I need to change how the linter is used; currently it comes at the project from the html files then tries to find view-models / source;

Perhaps I need to try to come at it from the source files;

I agree, that current approach has uncovered corner-cases.

specifically the main / app entry points and then the router module paths.

But I'm not sure if it makes sense to over-complicate things like that. For example getViewStrategy method may dynamically compose html file name (the name might be computed using string concatenation, etc or even fetched from the server). Not sure how useful would the new approach be in this case. But to support @useView("foo.html") decorator, You could just scan all the classes (without trying to figure out what views/customElements are used from some or all application entrypoints).

Another corner-case that i suspect would be very difficult to cover is using compose tag, that could dynamically take any html template and viewModel.

Alexander-Taran commented 6 years ago

Probably can be closed Not sure what Is the current roadmap for linter. vscode/other ides only? cmdline tool?

EisenbergEffect commented 6 years ago

This will be covered by the work to incorporate the linter and extend the VS plugin and language server.