Skip to content
Open
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 docs/rules/no-this-assign-in-render.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,5 +32,5 @@ updated() {

## When Not To Use It

If you have non-observed properties you wish to update per render, you may
If you have non-observed properties you wish to update per render via computed property accessors, you may
want to disable this rule.
50 changes: 44 additions & 6 deletions src/rules/no-this-assign-in-render.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import {Rule} from 'eslint';
import * as ESTree from 'estree';
import {isLitClass} from '../util';
import {getPropertyMap, isLitClass, PropertyMapEntry} from '../util';

//------------------------------------------------------------------------------
// Rule Definition
Expand All @@ -31,6 +31,7 @@ const rule: Rule.RuleModule = {
create(context): Rule.RuleListener {
let inRender = false;
let inComponent = false;
let propertyMap: ReadonlyMap<string, PropertyMapEntry> | null = null;

/**
* Class entered
Expand All @@ -43,6 +44,12 @@ const rule: Rule.RuleModule = {
return;
}

const props = getPropertyMap(node);

if (props) {
propertyMap = props;
}

inComponent = true;
}

Expand All @@ -52,6 +59,7 @@ const rule: Rule.RuleModule = {
* @return {void}
*/
function classExit(): void {
propertyMap = null;
inComponent = false;
}

Expand Down Expand Up @@ -84,21 +92,51 @@ const rule: Rule.RuleModule = {
inRender = false;
}

/**
* Walk left side assignment members and return first non-member
*
* @param {ESTree.MemberExpression} member Member entered
* @return {ESTree.Node}
*/
function walkMembers(member: ESTree.MemberExpression): ESTree.Node {
if (member.object.type === 'MemberExpression') {
return walkMembers(member.object);
} else {
return member.object;
}
}

/**
* Left side of an assignment expr found
*
* @param {Rule.Node} node Node entered
* @return {void}
*/
function assignmentFound(node: Rule.Node): void {
if (!inRender) {
if (!inRender ||
!propertyMap ||
node.type !== 'MemberExpression') {
return;
}

context.report({
node: node.parent,
messageId: 'noThis'
});
const nonMember = walkMembers(node) as Rule.Node;
if (nonMember.type === 'ThisExpression') {
const parent = nonMember.parent as ESTree.MemberExpression;

let propertyName = '';
if (parent.property.type === 'Identifier' && !parent.computed) {
propertyName = parent.property.name;
} else if (parent.property.type === 'Literal') {
propertyName = String(parent.property.value);
}

if (propertyMap.has(propertyName) || parent.computed) {
context.report({
node: node.parent,
messageId: 'noThis'
});
}
}
}

return {
Expand Down
94 changes: 90 additions & 4 deletions src/test/rules/no-this-assign-in-render_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,18 @@ ruleTester.run('no-this-assign-in-render', rule, {
this.deep.prop = 5;
}
}`,
`class Foo {
static get properties() {
return { prop: { type: Number } };
}
render() {
this.deep.prop = 5;
}
}`,
`class Foo extends LitElement {
static get properties() {
return { x: { type: Number } };
}
render() {
const x = this.prop;
}
Expand All @@ -54,96 +65,171 @@ ruleTester.run('no-this-assign-in-render', rule, {
}
}`,
`class Foo extends LitElement {
static get properties() {
return { foo: { type: Number } };
}
static render() {
this.foo = 5;
}
}`,
`class Foo extends LitElement {
static get properties() {
return { prop: { type: Number } };
}
render() {
let x;
x = this.prop;
}
}`,
`class Foo extends LitElement {
static get properties() {
return { x: { type: Number } };
}
render() {
let x;
x = 5;
}
}`,
`class Foo extends LitElement {
static get properties() {
return { foo: { type: Number } };
}
render() {
let x;
x = this.foo || 123;
}
}`,
`class Foo extends LitElement {
static get properties() {
return { prop: { type: Number } };
}
render() {
const x = {};
x[this.prop] = 123;
}
}`,
`class Foo extends LitElement {
static get properties() {
return { prop: { type: Number } };
}
render() {
const x = () => ({});
x(this.prop).y = 123;
}
}`,
`class Foo extends LitElement {
static get properties() {
return { prop: { type: Number } };
}
render() {
this.unreactive = 123;
}
}`,
`class Foo extends LitElement {
static get properties() {
return { prop: { type: Number } };
}
render() {
this.unreactive.prop = 123;
}
}`
],

invalid: [
{
code: `class Foo extends LitElement {
static get properties() {
return { prop: { type: String } };
}
render() {
this.prop = 'foo';
}
}`,
errors: [
{
messageId: 'noThis',
line: 3,
line: 6,
column: 11
}
]
},
{
code: `class Foo extends LitElement {
static get properties() {
return { deep: { type: Object } };
}
render() {
this.deep.prop = 'foo';
}
}`,
errors: [
{
messageId: 'noThis',
line: 3,
line: 6,
column: 11
}
]
},
{
code: `const foo = class extends LitElement {
static get properties() {
return { prop: { type: String } };
}
render() {
this.prop = 'foo';
}
}`,
errors: [
{
messageId: 'noThis',
line: 3,
line: 6,
column: 11
}
]
},
{
code: `class Foo extends LitElement {
static get properties() {
return { prop: { type: String } };
}
render() {
this['prop'] = 'foo';
}
}`,
errors: [
{
messageId: 'noThis',
line: 3,
line: 6,
column: 11
}
]
},
{
code: `@customElement('foo')
class Foo extends FooElement {
@property({ type: String })
prop = '';
render() {
this['prop'] = 'foo';
}
}`,
parser,
parserOptions,
errors: [
{
messageId: 'noThis',
line: 6,
column: 11
}
]
},
{
code: `class Foo extends LitElement {
render() {
const x = 'prop';
this[x] = 'foo';
}
}`,
errors: [
{
messageId: 'noThis',
Expand Down