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

Build-time validation (linter) for Aurelia application #31

Closed
atsu85 opened this issue Mar 24, 2016 · 28 comments
Closed

Build-time validation (linter) for Aurelia application #31

atsu85 opened this issue Mar 24, 2016 · 28 comments

Comments

@atsu85
Copy link

atsu85 commented Mar 24, 2016

It would be nice if Aurelia would provide build-time correctness validation (linter) of the application. Here i don't mean JS errors, that could be detected with TypeScript, but other types of errors/problems.
For example

  • typos in binding expressions
    • check if view-model contains field/method that should exist in controller
    • check if application contains the ViewConverters and Signals used in expressions
      • (don't know if it is possible to detect, if given ViewConverter/Signal is available in the file where it is used)
  • typos in binding attributes for example value.one-way vs value.on-way
  • typos with incorrect <require from="./not/found"> element path
  • correct usage of template controller (You shouldn't add both repeat.for and if.bind to the same element)
  • check, that <content> element is not used inside dynamically created element, such as if.bind
  • using style attribute with expressions instead of css attribute (if I remember correctly, it cause problems for IE)
    • maybe it could detect some more problems that would work only on some browsers

Implementing some of the features mentioned above would also be very useful for other use-cases, such as a autocomplete/intellisense for IDE plugins (not to mention validation inside IDE plugins).

I'm sure that Aurelia team and community will find lots of more use-cases for it and I'd like to see what other people could come up with.

@patroza
Copy link

patroza commented Apr 2, 2016

Oh yes please! +1 (just ran into the if.bind vs repeat.for)

@EisenbergEffect
Copy link
Contributor

Yes, we would like to add these sorts of tools. We are building a base cli now as a first step in major tooling improvements. After that, we can add all sorts of useful capabilities.

@atsu85
Copy link
Author

atsu85 commented May 20, 2016

@niieani sent a pull request for Aurelia webpack plugin that among other things includes logic to iterate over all html files and to check that from attributes of require tags reference files, that exist in the project :)

For sure, this is specific to Aurelia webpack plugin and can't be directly reused as is for the linter, but (for people who don't feel comfortable in Node environment) that could be a good starting point to implement one part of the linter mentioned in this issue description:

typos with incorrect element path

Thanks @niieani!

@atsu85
Copy link
Author

atsu85 commented Jun 10, 2016

@MeirionHughes, I discovered by accident, that You already are building a linter for Aurelia, that detects some of the issues listed above.

@EisenbergEffect, @MeirionHughes, I guess the linter would gain much more attraction (potential contributors and bring fame to Aurelia) when it would be official subproject of Aurelia (hosted under aurelia github account). A blog post about it in official blog would be also super to let the community know about it.

@EisenbergEffect, do I understand correctly, that this project should eventually rely on linter project, not be integrated to this repo directly? PS. To be honest, I don't understand the clear purpose of this project any more, now that cli is being built instead and chrome plugin has been added meanwhile)

@EisenbergEffect
Copy link
Contributor

This project mostly contains some helpers that we use in the building of Aurelia itself.

Also...that linter looks awesome. We should definitely see how we can bring that to the broader community. @MeirionHughes I'd love to hear from you on where you think the project is at and what your vision is.

@atsu85
Copy link
Author

atsu85 commented Jun 10, 2016

@EisenbergEffect, @MeirionHughes has also a gulp plugin and a linting rules for obsolete tags and attributes (among other things) - they could be used to help developers update Aurelia webapps and plugins. For example content vs slot just to name one ;)

@EisenbergEffect
Copy link
Contributor

Nice!

@niieani
Copy link
Contributor

niieani commented Jun 11, 2016

Another idea: We could have the linter convert the View on-the-fly to a typed TypeScript object with source-mapping to the original .html that could then be type-checked by TypeScript's powerful type-checker, with the context of the ViewModel.

@MeirionHughes
Copy link

MeirionHughes commented Jun 11, 2016

Hi @EisenbergEffect. the linter came about because I consistently used self-closing tags and nothing complains about it; I've come from WPF + caliburn.micro, so self-closing is second nature for me. Basically, I wanted to create something that gave you warnings when nothing else did.

I've tried to flesh it out a little bit from that initial rule, but I think the project would benefit greatly if others could say what is and isn't allowed. @atsu85 that list above is useful, I will try to cover as much as possible.

I was thinking about doing a PR to add this into the Skeleton App (in gulp task to move the html). In theory it won't actually do anything different until the user edits files and makes mistakes (that are picked up by the rules).

One thing which may actually be rather nice is to introduce a common file (or add it to package.json) with information on the plugin. Something like:

"aurelia":{
  "obsoleteTags" : [
     {"tag":"content",
      "warning": "content has been replaced by slot"} 
  ],
  "obsoleteAttributes" : [
      { "attribute" : "replace", 
       "tag":"template", 
       "warning": "replace attribute on template is obsolete"}
   ]
}

I think should component/plugin authors do this, I would be able to parse all the packages and compose a list of obsolete tags / attributes, without having to hard code them in the Linter rule. Again, this is overlapping with the existing system - plugin authors should be warning that a tag / attribute is obsolete in the console log.

I thought about the possibility of actually loading up the template along with its view-model (by convention) and run it through Aurelia's binding engine. I could look for completely unmatched variables and warn about them. That said, I think at this point such a thing would be starting to overlap with protractor, debug-logging and the browser's debugger; i.e. I don't want to overlap too much with tools that are better at their job.

@atsu85
Copy link
Author

atsu85 commented Jun 11, 2016

have the linter convert the View on-the-fly to a typed TypeScript

@niieani, is Aurelia doing smth similar at runtime, but to JS instead of TS? (I'm not familiar with it and away from computer right now to investigate)

@atsu85
Copy link
Author

atsu85 commented Jun 11, 2016

I was thinking about doing a PR to add this into the Skeleton App

Actually i already sent the PR last night (only for skeleton-typescript as POC to start the discussion)

@MeirionHughes
Copy link

MeirionHughes commented Jun 11, 2016

Actually i already sent the PR last night

that defiantly would have to wait until I fix the template rule; complaining about nested templates is simply wrong. :D

@MeirionHughes
Copy link

MeirionHughes commented Jun 11, 2016

I think if you really wanted a super-duper linting engine on the real-time application, the logical course of action would be to have Aurelia actually use parse5 (angular 2 does); you could separate rules out into:

  • parse rules - basically check the parsing of the HTML document before its added to the dom/shadow dom. This is effectively what aurleia-template-linter does at the moment.
  • runtime rules - check things like bindings/variables are not orphaned, etc.

In principle there isn't anything stopping the use of parse5 or aurelia-template-lint be used in the browser; I just need to compile out to system.js module target. Might be useful during the development cycle.

However, at the moment my focus is to nail down as much of the parsing stuff, at gulp-build time.

@atsu85
Copy link
Author

atsu85 commented Jun 11, 2016

@MeirionHughes

that defiantly would have to wait until I fix the template rule

You could just remove that rule from default set of rules and there shouldnt be a problem

@MeirionHughes
Copy link

Its fixed, it actually checks for a few things:

  • root is template (unless root is html, then it disables itself)
  • don't allow more than one template per file (side-by-side)
  • don't allow template as immediate child of containers (table/select)

@gheoan
Copy link
Contributor

gheoan commented Jun 11, 2016

I wonder if there is an existing html linter that we could plug into. @MeirionHughes' work is great, but html templates are just html so a basic set of rules would come in handy and then we would have to only write aurelia specific rules instead of reinventing the wheel.

@gheoan
Copy link
Contributor

gheoan commented Jun 11, 2016

@niieani angular2 is doing something similar angular/angular#7482.

@MeirionHughes
Copy link

MeirionHughes commented Jun 12, 2016

@gheoan I would agree with you, and I did actually run through a few options prior to making a new one, but nothing could deal with the issue at hand.

aurelia-template-lint runs on-top of parse5 (html5 complaint) and effectively sits in-between the document parser and the generation of the element tree; So its super easy to have a rule that says... <template> cannot be child of <table>. That said, its not going to help you if you have exceptionally ill-formed code (un-closed attributes for example);

Also, one final thing: you could technically have aurelia use parse5 instead of using the browser's parser; at which point you can run the rules while the document fragment is parsed. IT might be something you'd do in a debug environment, perhaps. Also, its worth noting that this is what what angular2 does (they use parse5 and have checks too).

In any-case, I don't want to push anything on anyone; if it helps people, that's great. :)

@atsu85
Copy link
Author

atsu85 commented Jun 13, 2016

you could technically have aurelia use parse5 instead of using the browser's parser; at which point you can run the rules while the document fragment is parsed

I'm not sure how much would it benefit Aurelia. Maybe @EisenbergEffect could comment - one benefit could possibly be better error messages at runtime, that include the source line and position that probably isn't possible with native browser parsers. But I think that it is even more important to detect errors at build time instead of runtime, to catch all the problems as early as possible.

But thanks again @MeirionHughes, aurelia-template-lint with gulp plugin is great start and i encourage everyone to try it out. Even creating new rule was really easy :)

@atsu85
Copy link
Author

atsu85 commented Sep 5, 2016

@EisenbergEffect, what are the plans with this issue and potentially aurelia-template-lint (that has made great progress)?

@EisenbergEffect
Copy link
Contributor

Well @MeirionHughes has joined our core team :)

@atsu85
Copy link
Author

atsu85 commented Sep 5, 2016

Cool, what does it mean for Aurelia project? It would be nice if aurelia-template-lint was included in skeleton-navigation and CLI. What are the plans with that? I've used it for a long time, and after several PRs, it has become a fantastic tool I've added to all the projects I've worked with recently :)

@EisenbergEffect
Copy link
Contributor

I'd certainly love to incorporate it as a standard part of the setup. @MeirionHughes Have you tried using it with a CLI project yet? I'd love to work on how that experience might work and make a plan to bring that in.

@MeirionHughes
Copy link

@EisenbergEffect Not yet; I will try to use the CLI properly with a project either this week or next. I don't foresee there being much of an issue adding the lint though; the basic use case would be to add the lint call prior to sending to the bundler; which is no different to the gulp case (moving the html file to the dist).

There are some significant gaps in the typing check though; notably when the template requires anything else.

I basically need another level above my current linter, that tries to resolve the dependency tree. Then I can workout what the bindables are (from view or viewmodel), and their types (if defined in a view-model). With that I can tell if you're using a component correctly or not. Ditto with converters.

I also need to load up the app-config, so I can then load up the plugins and grab the resources (converters, components). Then I can definitively say something is missing and report typo errors on component/converter names.

I started on the changes before I went on holiday, so I need to pick it up again when I can get a good few hours spare.

@EisenbergEffect
Copy link
Contributor

Sounds great!

@arnederuwe
Copy link

arnederuwe commented Nov 25, 2016

FYI, I am currently using the aurelia-template-lint-gulp in my Aurelia CLI project, using TypeScript.
After npm installing aurelia-template-lint-gulp, I added a lintaurelia.ts file in the tasks folder that looks like this:

import * as gulp from 'gulp';
import * as linter from 'gulp-aurelia-template-lint';
var config = new (require('aurelia-template-lint').Config);

/// opt-in to static type checks:
config.useRuleAureliaBindingAccess = true;
config.reflectionOpts.sourceFileGlob = "src/**/*.ts";

function lintAurelia() {
    return gulp.src('src/**/*.html')
        .pipe(linter(config))
        .pipe(gulp.dest('output'));
}

export default gulp.series(lintAurelia);

Just a copy paste from the aurelia-template-lint-gulp project with some small modifications.
Next, in the build task, I call this method before transpiling:

import * as gulp from 'gulp';
import transpile from './transpile';
import processMarkup from './process-markup';
import processCSS from './process-css';
import {build} from 'aurelia-cli';
import * as project from '../aurelia.json';
import lint from './lintaurelia';

export default gulp.series(
  readProjectConfiguration,
  lint,
  gulp.parallel(
    transpile,
    processMarkup,
    processCSS
  ),
  writeBundles
);

so when performing an au build, I get all warnings in my console.

@Alexander-Taran
Copy link

Share the knowledge!!!

Stale since 2016

@EisenbergEffect
Copy link
Contributor

Closing since linting has been available through the above plugin for some time and we're currently working to port some of these ideas to the VS Code extension.

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

No branches or pull requests

8 participants