From 40792ac949fb1c6a72a449bfc922e181e7da36d0 Mon Sep 17 00:00:00 2001 From: Matt Lewis Date: Mon, 4 Feb 2019 15:45:12 +0700 Subject: [PATCH] feat(component-change-detection): add change detection strategy rule Closes #135 --- src/componentChangeDetectionRule.ts | 113 ++++++++++++++++++++++ src/index.ts | 1 + test/componentChangeDetectionRule.spec.ts | 49 ++++++++++ 3 files changed, 163 insertions(+) create mode 100644 src/componentChangeDetectionRule.ts create mode 100644 test/componentChangeDetectionRule.spec.ts diff --git a/src/componentChangeDetectionRule.ts b/src/componentChangeDetectionRule.ts new file mode 100644 index 000000000..08fd9070c --- /dev/null +++ b/src/componentChangeDetectionRule.ts @@ -0,0 +1,113 @@ +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'; + +const OPTION_DEFAULT = 'Default'; +const OPTION_ON_PUSH = 'OnPush'; + +export type ChangeDetectionType = 'Default' | 'OnPush'; + +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 (Default or OnPush).', + descriptionDetails: Utils.dedent` + See more at https://angular.io/api/core/ChangeDetectionStrategy + `, + optionExamples: [[true, OPTION_DEFAULT], [true, OPTION_ON_PUSH]], + options: { + items: [ + { + enum: [OPTION_DEFAULT, OPTION_ON_PUSH] + } + ], + maxLength: 1, + minLength: 1, + type: 'array' + }, + optionsDescription: Utils.dedent` + Options accepts one obligatory item as an array: + 1. \`${OPTION_DEFAULT}\` or \`${OPTION_ON_PUSH}\` to force components to use that type of change detection + `, + 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.%s'; + + type: ChangeDetectionType; + + constructor(options: Lint.IOptions) { + super(options); + const args = this.getOptions().ruleArguments; + this.type = args[0]; + } + + isEnabled(): boolean { + const { + metadata: { + options: { maxLength, minLength } + } + } = Rule; + const { length } = this.ruleArguments; + + return super.isEnabled() && length >= minLength && length <= maxLength; + } + + 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); + + // Do not run component rules for directives + if (name === 'Component') { + this.validateChangeDetection(className, decorator, argument); + } + } + + private validateChangeDetection(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, this.rule.type)); + } else { + const changeDetectionValue = changeDetectionAssignment!.name.escapedText as string; + + if (this.rule.type !== changeDetectionValue) { + this.addFailureAtNode(changeDetectionAssignment, sprintf(Rule.CHANGE_DETECTION_INVALID_FAILURE, className, this.rule.type)); + } + } + } + + private validateProperty(p: ts.PropertyAssignment): boolean { + return ts.isIdentifier(p.name) && p.name.text === 'changeDetection'; + } +} diff --git a/src/index.ts b/src/index.ts index d9dd0f6cd..c3e49e849 100644 --- a/src/index.ts +++ b/src/index.ts @@ -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'; diff --git a/test/componentChangeDetectionRule.spec.ts b/test/componentChangeDetectionRule.spec.ts new file mode 100644 index 000000000..c7080de63 --- /dev/null +++ b/test/componentChangeDetectionRule.spec.ts @@ -0,0 +1,49 @@ +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, + options: ['OnPush'] + }); + }); + + 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, + options: ['OnPush'] + }); + }); + }); + + 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, ['OnPush']); + }); + }); +});