Skip to content
This repository was archived by the owner on Apr 4, 2025. It is now read-only.

feat: update codelyzer and add rules #257

Merged
merged 1 commit into from
Nov 23, 2017
Merged

Conversation

mgechev
Copy link
Member

@mgechev mgechev commented Nov 3, 2017

codelyzer@^4.0.0 is out 🎉! It introduces support for @angular/compiler version 5 and few new rules such as:

  • angular-whitespace - checks whether we have presence of whitespace
    in the template to make it more readable, for instance
    in {{ expr }}, check-interpolation will check if expr is
    surrounded by whitespace. This rule also introduces auto fixes.
  • no-output-on-prefix - warns if outputs are prefixed with on this
    is based on
    https://angular.io/guide/styleguide#dont-prefix-output-properties.

Tell me what you think about angular-whitespace and let me know if it
is too opinionated. As part of angular-whitespace we're planning a few
more features.

Also, since invoke-injectable is supposed to be caught by tsc, this rule has been deprecated.

Fixes angular/angular-cli#8463

Codelyzer@^4.0.0 is out! It introduces few new rules such as:

- `angular-whitespace` - checks whether we have presence of whitespace
in the template to make it more readable, for instance
in `{{ expr }}`, `check-interpolation` will check if `expr` is
surrounded by whitespace. This rule also introduces auto fixes.
- `no-output-on-prefix` - warns if outputs are prefixed with `on` this
is based on
https://angular.io/guide/styleguide#dont-prefix-output-properties.

Tell me what you think about `angular-whitespace` and let me know if it
is too opinionated. As part of `angular-whitespace` we're planning a few
more features.
@mgechev mgechev changed the title chore: update codelyzer and add rules feat: update codelyzer and add rules Nov 3, 2017
@cyrilletuzi
Copy link
Contributor

The update of codelyzer, along with a tslint one, is already proposed in #253

I don't think it's a good idea to enable the new angular-whitespace by default. Existing project would now fail on lint, and it's more a user preference.

@akkumar
Copy link
Contributor

akkumar commented Nov 14, 2017

The build seems to have failed with the following message.

    The following commit should always have a scope:
      1471daf feat: update codelyzer and add rules
  1 commits were found invalid...

May be - this needs an 'amend'ed commit again with the right syntax with the component name in it ?

Looking at the git history - we can try something like -

"feat(@schematics/angular)" or "fix(@schematics/angular)"

Eg:

2017-10-25 11:41 Alex Rickabaugh           o feat(@schematics/angular): update Angular version to 5.0.0

Would be good to have this fix in .

cc @mgechev

@moritz-h
Copy link

I don't think it's a good idea to enable the new angular-whitespace by default. Existing project would now fail on lint, and it's more a user preference.

Yes, I found this discussion, right because of this behaviour. 😄

@mgechev
When looking at https://angular.io/guide/styleguide there is not a single {{ expr }}. Everything is written as {{expr}}.

@mgechev
Copy link
Member Author

mgechev commented Nov 30, 2017

Yes, the style guide doesn't specify a recommended way of using the interpolation syntax. The fact that we're using {{expr}} doesn't mean that this is our recommendation. We don't want to introduce syntax conventions.

As mentioned above, this rule introduces an autofix so projects can be easily aligned to it. Although I believe wrapping an expression with whitespace characters is more readable I don't insist to enforce this coding style. If majority thinks that it's not a good idea, I don't mind excluding the rule.

clydin pushed a commit that referenced this pull request Dec 1, 2017
… rule

While updating to Codelyzer 4.0, #257 also added a new `angular-whitespace` rule.

This must be reverted because :
- it is a breaking change : existing project are now failing on lint ;
- it is opinionated (it is not in official Angular style guide) and so should not be forced ;
- it is the contrary of what is done in the official Angular doc and in the style guide itself.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1.5.0 ng new should have codelyzer 4.0.x
6 participants