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] Friendlier checking of validator directive names #397

Closed
RandomShaper opened this issue Aug 17, 2017 · 8 comments · Fixed by #407
Closed

[feature] Friendlier checking of validator directive names #397

RandomShaper opened this issue Aug 17, 2017 · 8 comments · Fixed by #407

Comments

@RandomShaper
Copy link

Angular builtin validators are suffixed with Validator, not Directive.

Would it be possible to allow directives to follow that, as long as they implement the Validator interface?

@wKoza
Copy link
Collaborator

wKoza commented Aug 17, 2017

it seems to make sense. I'm ok.

@mgechev
Copy link
Owner

mgechev commented Aug 17, 2017

What do you think about extending the properties of directive-class-suffix to allow an array of suffixes? The drawback is that its not context dependent validation but the rule will be more configurable.

An alternative is to use Directive or Validator by default (check the class implements Validator and if it does require Validator suffix) and fallback to other configuration options if provided.

@wKoza
Copy link
Collaborator

wKoza commented Aug 17, 2017

I had thought about that, but as you say, its not context dependent. I prefer the second option even if it's not extensible.
We should also manage the case of AsyncValidator.

@RandomShaper
Copy link
Author

Yes, the second option seems the way to go.

And in addition to AsyncValidator, suffix ValueAccesor for the ControlValueAccessor interface.

I've checked the Angular API for more interesting directive naming cases and I don't see any other naming schema of interest for developers. Either they begin with Ng (which seems to give them permission to skip the naming guidelines) or are too core to think someone will need to do anything similar.

And if someone does, well, after all you are not following the guidelines, so a lint is what you deserve. :)

@wKoza
Copy link
Collaborator

wKoza commented Aug 18, 2017

mhm, i'm not in favour of implementing a rule for a ControlValueAccessor . For me, it's just a Component with a specific behaviour. @mgechev , an opinion ?

@RandomShaper
Copy link
Author

The check would be for directives implementing that interface, allowing/forcing them to have the right suffix in their name.

That is, leaving components alone.

@mgechev
Copy link
Owner

mgechev commented Sep 8, 2017

@wKoza, this sounds good.

@mgechev
Copy link
Owner

mgechev commented Sep 8, 2017

Will take this one right now.

@mgechev mgechev self-assigned this Sep 8, 2017
mgechev added a commit that referenced this issue Sep 8, 2017
- Allow `Validator` suffix if any of the implemented interfaces by a
  directive ends with `Validator`.
- Allow multiple suffixes for a directive.

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

Successfully merging a pull request may close this issue.

3 participants