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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(component-change-detection): add change detection strategy rule #737

Merged
merged 2 commits into from Feb 5, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
79 changes: 79 additions & 0 deletions src/componentChangeDetectionRule.ts
@@ -0,0 +1,79 @@
import { Utils } from 'tslint/lib';
import * as Lint from 'tslint';
import * as ts from 'typescript';
import { getDecoratorArgument, getDecoratorName } from './util/utils';
import { sprintf } from 'sprintf-js';
import { NgWalker } from './angular';

export class Rule extends Lint.Rules.AbstractRule {
public static metadata: Lint.IRuleMetadata = {
ruleName: 'component-change-detection',
type: 'functionality',
description: 'Enforce the preferred component change detection type as ChangeDetectionStrategy.OnPush.',
descriptionDetails: Utils.dedent`
See more at https://angular.io/api/core/ChangeDetectionStrategy
`,
options: null,
optionsDescription: 'Not configurable.',
rationale: Utils.dedent`
By using OnPush for change detection, Angular will only run a change detection cycle when that
components inputs or outputs change
`,
typescriptOnly: true
};

static CHANGE_DETECTION_INVALID_FAILURE =
'The changeDetection value of the component "%s" should be set to ChangeDetectionStrategy.OnPush';

apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
return this.applyWithWalker(new ComponentChangeDetectionValidatorWalker(sourceFile, this));
}
}

export class ComponentChangeDetectionValidatorWalker extends NgWalker {
constructor(sourceFile: ts.SourceFile, private rule: Rule) {
super(sourceFile, rule.getOptions());
}

visitClassDeclaration(node: ts.ClassDeclaration) {
ts.createNodeArray(node.decorators).forEach(this.validateDecorator.bind(this, node.name!.text));
super.visitClassDeclaration(node);
}

private validateDecorator(className: string, decorator: ts.Decorator) {
const argument = getDecoratorArgument(decorator)!;
const name = getDecoratorName(decorator);

// Run only for Components
if (name === 'Component') {
this.validateComponentChangeDetection(className, decorator, argument);
}
}

private validateComponentChangeDetection(className: string, decorator: ts.Decorator, arg: ts.Node) {
if (!ts.isObjectLiteralExpression(arg)) {
return;
}

const changeDetectionAssignment = arg.properties
.filter(prop => ts.isPropertyAssignment(prop) && this.validateProperty(prop))
.map(prop => (ts.isPropertyAssignment(prop) ? prop.initializer : undefined))
.filter(Boolean)[0] as ts.PropertyAccessExpression;

if (!changeDetectionAssignment) {
this.addFailureAtNode(decorator, sprintf(Rule.CHANGE_DETECTION_INVALID_FAILURE, className));
} else {
if (!this.validateChangeDetectionType(changeDetectionAssignment!.name.escapedText as string)) {
this.addFailureAtNode(changeDetectionAssignment, sprintf(Rule.CHANGE_DETECTION_INVALID_FAILURE, className));
}
}
}

private validateProperty(p: ts.PropertyAssignment): boolean {
return ts.isIdentifier(p.name) && p.name.text === 'changeDetection';
}

private validateChangeDetectionType(changeDetectionValue: string): boolean {
return changeDetectionValue === 'OnPush';
}
}
1 change: 1 addition & 0 deletions src/index.ts
Expand Up @@ -39,6 +39,7 @@ export { Rule as UsePipeDecoratorRule } from './usePipeDecoratorRule';
export { Rule as UsePipeTransformInterfaceRule } from './usePipeTransformInterfaceRule';
export { Rule as UseViewEncapsulationRule } from './useViewEncapsulationRule';
export { Rule as RelativePathExternalResourcesRule } from './relativeUrlPrefixRule';
export { Rule as ComponentChangeDetectionRule } from './componentChangeDetectionRule';

export * from './angular';

Expand Down
47 changes: 47 additions & 0 deletions test/componentChangeDetectionRule.spec.ts
@@ -0,0 +1,47 @@
import { assertSuccess, assertAnnotated } from './testHelper';

describe('component-change-detection', () => {
describe('invalid component change detection', () => {
it('should fail when component used without preferred change detection type', () => {
let source = `
@Component({
changeDetection: ChangeDetectionStrategy.Default
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
})
class Test {}
`;
assertAnnotated({
ruleName: 'component-change-detection',
message: 'The changeDetection value of the component "Test" should be set to ChangeDetectionStrategy.OnPush',
source
});
});

it('should fail when component change detection is not set', () => {
let source = `
@Component({
~~~~~~~~~~~~
selector: 'foo'
})
~~
class Test {}`;
assertAnnotated({
ruleName: 'component-change-detection',
message: 'The changeDetection value of the component "Test" should be set to ChangeDetectionStrategy.OnPush',
source
});
});
});

describe('valid component selector', () => {
it('should succeed when a valid change detection strategy is set on @Component', () => {
let source = `
@Component({
changeDetection: ChangeDetectionStrategy.OnPush
})
class Test {}
`;
assertSuccess('component-change-detection', source);
});
});
});