diff --git a/README.md b/README.md index b2d9d34..7e3e983 100644 --- a/README.md +++ b/README.md @@ -69,12 +69,12 @@ If you want more fine-grained configuration, you can instead add a snippet like - [lit/no-template-arrow](docs/rules/no-template-arrow.md) - [lit/no-template-bind](docs/rules/no-template-bind.md) - [lit/no-template-map](docs/rules/no-template-map.md) +- [lit/no-this-assign-in-render](docs/rules/no-this-assign-in-render.md) - [lit/no-useless-template-literals](docs/rules/no-useless-template-literals.md) - [lit/no-value-attribute](docs/rules/no-value-attribute.md) - [lit/prefer-nothing](docs/rules/prefer-nothing.md) - [lit/quoted-expressions](docs/rules/quoted-expressions.md) - ## Shareable configurations ### Recommended diff --git a/docs/rules/no-this-assign-in-render.md b/docs/rules/no-this-assign-in-render.md new file mode 100644 index 0000000..6be569f --- /dev/null +++ b/docs/rules/no-this-assign-in-render.md @@ -0,0 +1,36 @@ +# Disallows assignments to members of `this` in render methods (no-this-assign-in-render) + +Assignments to `this` in the render method of a `LitElement` are generally a +mistake since it should ideally act like a pure function. + +Property updates should usually happen in the other lifecycle methods +(e.g. `updated`) or in event handlers. + +## Rule Details + +This rule disallows assigning to `this` in the render method. + +The following patterns are considered warnings: + +```ts +render() { + this.prop = 5; +} +``` + +The following patterns are not warnings: + +```ts +render() { + const prop = 5; +} + +updated() { + this.prop = 5; +} +``` + +## When Not To Use It + +If you have non-observed properties you wish to update per render, you may +want to disable this rule. diff --git a/src/rules/no-this-assign-in-render.ts b/src/rules/no-this-assign-in-render.ts new file mode 100644 index 0000000..cd6b6a3 --- /dev/null +++ b/src/rules/no-this-assign-in-render.ts @@ -0,0 +1,125 @@ +/** + * @fileoverview Disallows assignments to members of `this` in render methods + * @author James Garbutt + */ + +import {Rule} from 'eslint'; +import * as ESTree from 'estree'; + +//------------------------------------------------------------------------------ +// Rule Definition +//------------------------------------------------------------------------------ + +const rule: Rule.RuleModule = { + meta: { + docs: { + description: + 'Disallows assignments to members of `this` in render methods', + category: 'Best Practices', + recommended: true, + url: 'https://github.com/43081j/eslint-plugin-lit/blob/master/docs/rules/no-this-assign-in-render.md' + }, + schema: [], + messages: { + noThis: + 'Members of `this` should not be assigned to in the render ' + + 'method. It is likely you should do this elsewhere instead ' + + '(e.g. in `updated`)' + } + }, + + create(context): Rule.RuleListener { + let inRender = false; + let inComponent = false; + + /** + * Class entered + * + * @param {ESTree.Class} node Node entered + * @return {void} + */ + function classEnter(node: ESTree.Class): void { + if ( + !node.superClass || + node.superClass.type !== 'Identifier' || + node.superClass.name !== 'LitElement' + ) { + return; + } + + inComponent = true; + } + + /** + * Class exited + * + * @return {void} + */ + function classExit(): void { + inComponent = false; + } + + /** + * Method entered + * + * @param {ESTree.MethodDefinition} node Node entered + * @return {void} + */ + function methodEnter(node: ESTree.MethodDefinition): void { + if ( + !inComponent || + node.kind !== 'method' || + node.static === true || + node.key.type !== 'Identifier' || + node.key.name !== 'render' + ) { + return; + } + + inRender = true; + } + + /** + * Method exited + * + * @return {void} + */ + function methodExit(): void { + inRender = false; + } + + /** + * Assignment expression entered + * + * @param {ESTree.AssignmentExpression} node Node entered + * @return {void} + */ + function assignmentFound(node: ESTree.AssignmentExpression): void { + if (!inRender) { + return; + } + + context.report({ + node, + messageId: 'noThis' + }); + } + + return { + ClassExpression: (node: ESTree.Node): void => + classEnter(node as ESTree.Class), + ClassDeclaration: (node: ESTree.Node): void => + classEnter(node as ESTree.Class), + 'ClassExpression:exit': classExit, + 'ClassDeclaration:exit': classExit, + MethodDefinition: (node: ESTree.Node): void => + methodEnter(node as ESTree.MethodDefinition), + 'MethodDefinition:exit': methodExit, + 'AssignmentExpression:has([left] ThisExpression)': ( + node: ESTree.Node + ): void => assignmentFound(node as ESTree.AssignmentExpression) + }; + } +}; + +export = rule; diff --git a/src/test/rules/no-this-assign-in-render_test.ts b/src/test/rules/no-this-assign-in-render_test.ts new file mode 100644 index 0000000..998a9c2 --- /dev/null +++ b/src/test/rules/no-this-assign-in-render_test.ts @@ -0,0 +1,113 @@ +/** + * @fileoverview Disallows assignments to members of `this` in render methods + * @author James Garbutt + */ + +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +import rule = require('../../rules/no-this-assign-in-render'); +import {RuleTester} from 'eslint'; + +//------------------------------------------------------------------------------ +// Tests +//------------------------------------------------------------------------------ + +const ruleTester = new RuleTester({ + parserOptions: { + sourceType: 'module', + ecmaVersion: 2015 + } +}); + +ruleTester.run('no-this-assign-in-render', rule, { + valid: [ + 'const x = 808;', + 'class Foo { }', + `class Foo { + render() { + this.prop = 5; + } + }`, + `class Foo { + render() { + this.deep.prop = 5; + } + }`, + `class Foo extends LitElement { + render() { + const x = this.prop; + } + }`, + `class Foo extends LitElement { + render() { + const x = 5; + } + }`, + `class Foo extends LitElement { + static render() { + this.foo = 5; + } + }` + ], + + invalid: [ + { + code: `class Foo extends LitElement { + render() { + this.prop = 'foo'; + } + }`, + errors: [ + { + messageId: 'noThis', + line: 3, + column: 11 + } + ] + }, + { + code: `class Foo extends LitElement { + render() { + this.deep.prop = 'foo'; + } + }`, + errors: [ + { + messageId: 'noThis', + line: 3, + column: 11 + } + ] + }, + { + code: `const foo = class extends LitElement { + render() { + this.prop = 'foo'; + } + }`, + errors: [ + { + messageId: 'noThis', + line: 3, + column: 11 + } + ] + }, + { + code: `class Foo extends LitElement { + render() { + this['prop'] = 'foo'; + } + }`, + errors: [ + { + messageId: 'noThis', + line: 3, + column: 11 + } + ] + } + ] +});