-
Notifications
You must be signed in to change notification settings - Fork 6.7k
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: add lint rule to enforce coercion static properties for setters #17536
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM in general. I already ran this rule against my PR (a few instances are intentionally ignored due to the lint rule not being 100% correct; which is totally fine). These are the points we should improve though:
- Checking if the setter is already typed to work with coercion. i.e. either contains
null
orstring
in the type - We should not warn for abstract directives or vanilla classes if they use coercion
@crisbeto We chatted about this on Slack. do you want to look into these things as part of this PR or in follow-ups? Feel free to add merge ready based on what you prefer. |
I'll make the changes in this PR. |
24d0781
to
1e7ef91
Compare
Added the changes to skip abstract directives and to ignore setters which are |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. One minor comment
Adds a custom tslint rule to enforce that properties which use coercion in a setter also declare a static property to indicate the accepted types to ngtsc. Also handles inherited setters and properties coming from an interface being implemented (necessary to support mixins). Relates to angular#17528.
1e7ef91
to
f7cb86f
Compare
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Adds a custom tslint rule to enforce that properties which use coercion in a setter also declare a static property to indicate the accepted types to ngtsc. Also handles inherited setters and properties coming from an interface being implemented (necessary to support mixins).
Relates to #17528.