Skip to content

build: add linting for classList usage #17388

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

Merged
merged 1 commit into from
Oct 14, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/material-experimental/mdc-button/button-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,15 @@ export class MatButtonBase extends _MatButtonBaseMixin implements CanDisable, Ca
@Optional() @Inject(ANIMATION_MODULE_TYPE) public _animationMode?: string) {
super(elementRef);

const classList = (elementRef.nativeElement as HTMLElement).classList;

// 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);
Copy link
Member Author

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.

pair.mdcClasses.forEach(className => {
classList.add(className);
});
}
}
}
Expand Down
46 changes: 46 additions & 0 deletions tools/tslint-rules/classListSignaturesRule.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import * as ts from 'typescript';
import * as Lint from 'tslint';

/**
* Rule that catches cases where `classList` is used in a way
* that won't work in all browsers that we support.
*/
export class Rule extends Lint.Rules.TypedRule {
applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] {
return this.applyWithWalker(new Walker(sourceFile, this.getOptions(), program));
}
}

class Walker extends Lint.ProgramAwareRuleWalker {
visitPropertyAccessExpression(propertyAccess: ts.PropertyAccessExpression) {
const parent = propertyAccess.parent;

// We only care about property accesses inside of calls.
if (!ts.isCallExpression(parent)) {
return;
}

// We only care about these method names.
const name = propertyAccess.name.text;
if (name !== 'add' && name !== 'remove' && name !== 'toggle' && name !== 'replace') {
return;
}

const symbol = this.getTypeChecker().getTypeAtLocation(propertyAccess.expression).symbol;

if (symbol && symbol.name === 'DOMTokenList') {
const args = parent.arguments;

if (name === 'replace') {
this.addFailureAtNode(propertyAccess,
'This method is not supported in iOS Safari. Use `add` and `remove` instead.');
} else if (args.length > 1 || (args.length === 1 && ts.isSpreadElement(args[0]))) {
this.addFailureAtNode(propertyAccess,
'Passing in multiple arguments into this method is not supported in some browsers. ' +
'Use the single argument signature instead.');
}
}

super.visitPropertyAccessExpression(propertyAccess);
}
}
1 change: 1 addition & 0 deletions tslint.json
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@
"rxjs-imports": true,
"require-breaking-change-version": true,
"static-query": true,
"class-list-signatures": true,
"no-host-decorator-in-concrete": [
true,
"HostBinding",
Expand Down