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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: new rule no-this-assign-in-render #146

Merged
merged 1 commit into from
Nov 28, 2022
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
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
}
]
}
]
});