-
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 linting for classList usage #17388
Conversation
721ad03
to
d71b933
Compare
Once in a while we hit an issue because we use some of the `classList` methods in a way that isn't supported in all browsers (e.g. passing multiple parameters to `add` in angular#17378). These changes add a simple lint rule to catch these cases earlier.
d71b933
to
596fa24
Compare
// For each of the variant selectors that is present in the button's host | ||
// attributes, add the correct corresponding MDC classes. | ||
for (const pair of HOST_SELECTOR_MDC_CLASS_PAIR) { | ||
if (this._hasHostAttributes(pair.selector)) { | ||
(elementRef.nativeElement as HTMLElement).classList.add(...pair.mdcClasses); |
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.
Looks like we had one of these already that we hadn't noticed.
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. Good use on the type checker to ensure we don't incorrectly add failures for similarly named methods.
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. |
Once in a while we hit an issue because we use some of the
classList
methods in a way that isn't supported in all browsers (e.g. passing multiple parameters toadd
in #17378). These changes add a simple lint rule to catch these cases earlier.