Skip to content

Commit

Permalink
feat: new rule no-this-assign-in-render (#146)
Browse files Browse the repository at this point in the history
Fixes #90
  • Loading branch information
43081j committed Nov 28, 2022
1 parent b1e901b commit 950ef6d
Show file tree
Hide file tree
Showing 4 changed files with 275 additions and 1 deletion.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
36 changes: 36 additions & 0 deletions docs/rules/no-this-assign-in-render.md
Original file line number Diff line number Diff line change
@@ -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.
125 changes: 125 additions & 0 deletions src/rules/no-this-assign-in-render.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
/**
* @fileoverview Disallows assignments to members of `this` in render methods
* @author James Garbutt <https://github.com/43081j>
*/

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;
113 changes: 113 additions & 0 deletions src/test/rules/no-this-assign-in-render_test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
/**
* @fileoverview Disallows assignments to members of `this` in render methods
* @author James Garbutt <https://github.com/43081j>
*/

//------------------------------------------------------------------------------
// 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
}
]
}
]
});

0 comments on commit 950ef6d

Please sign in to comment.