Skip to content

Commit

Permalink
feat: no-attributes-during-construction (#61)
Browse files Browse the repository at this point in the history
  • Loading branch information
ekashida authored Mar 12, 2021
1 parent c0f3c69 commit ca33119
Show file tree
Hide file tree
Showing 7 changed files with 483 additions and 18 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ To choose from three configuration settings, install the [`eslint-config-lwc`](h
| [lwc/no-api-reassignments](./docs/rules/no-api-reassignments.md) | prevent public property reassignments | |
| [lwc/no-deprecated](./docs/rules/no-deprecated.md) | disallow usage of deprecated LWC APIs | |
| [lwc/no-document-query](./docs/rules/no-document-query.md) | disallow DOM query at the document level | |
| [lwc/no-attributes-during-construction](./docs/rules/no-attributes-during-construction.md) | disallow setting attributes during construction | |
| [lwc/no-leading-uppercase-api-name](./docs/rules/no-leading-uppercase-api-name.md) | ensure public property doesn't start with an upper-case character | |
| [lwc/no-unexpected-wire-adapter-usages](./docs/rules/no-unexpected-wire-adapter-usages.md) | enforce wire adapters to be used with `wire` decorator | |
| [lwc/no-unknown-wire-adapters](./docs/rules/no-unknown-wire-adapters.md) | disallow usage of unknown wire adapters | |
Expand Down
88 changes: 88 additions & 0 deletions docs/rules/no-attributes-during-construction.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
# Disallow setting attributes during construction (no-attributes-during-construction)

The `LightningElement` base class extended by LWC component classes defines several properties that, when set, renders
attributes on its host element. This behavior mimics the native browser behavior.

By [specification](https://html.spec.whatwg.org/multipage/custom-elements.html#custom-element-conformance), constructors
must not cause the host element to gain attributes. This rule prevents set operations in the constructor method that
violate this restriction.

## Caveats

This rule only knows about `LightningElement` properties that implement this behavior (e.g., `hidden`, `id`, `role`,
`tabIndex`, `title`, etc) and will not detect custom implementations that may set attributes during construction time:

```js
import { LightningElement } from 'lwc';

export default class Test extends LightningElement {
set foo(val) {
this.setAttribute('foo', val);
}

constructor() {
this.foo = 'this causes the element to gain the foo attribute during construction';
}
}
```

This rule will not detect violations on component classes that do not directly inherit from `LightningElement`:

```js
import { LightningElement } from 'lwc';

class Base extends LightningElement {}

export default class Test extends Base {
constructor() {
this.title = 'this causes the element to gain the foo attribute during construction';
}
}
```

## Examples

### Invalid

The following example is setting the `title` property which the `LightningElement` base class provides by default and
this renders the `title` attribute on the host element.

```js
import { LightningElement } from 'lwc';

export default class Test extends LightningElement {
constructor() {
this.title = 'this causes the element to gain the title attribute during construction';
}
}
```

### Valid

The following example does not set the value of `title` inside the constructor.

```js
import { LightningElement } from 'lwc';

export default class Test extends LightningElement {
connectedCallback() {
this.title = 'this causes the element to gain the title attribute upon connection';
}
}
```

The following example overrides the `title` property which the `LightningElement` base class provides by default, with a
`title` property that, when set, does not render an attribute on the host element.

```js
import { LightningElement } from 'lwc';

export default class Test extends LightningElement {
title = 'this custom property overrides the one in LightningElement';

constructor() {
this.title =
'this does not cause the element to gain the title attribute during construction';
}
}
```
1 change: 1 addition & 0 deletions lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ const rules = {
'no-api-reassignments': require('./rules/no-api-reassignments'),
'no-async-await': require('./rules/no-async-await'),
'no-async-operation': require('./rules/no-async-operation'),
'no-attributes-during-construction': require('./rules/no-attributes-during-construction'),
'no-deprecated': require('./rules/no-deprecated'),
'no-document-query': require('./rules/no-document-query'),
'no-dupe-class-members': require('./rules/no-dupe-class-members'),
Expand Down
152 changes: 152 additions & 0 deletions lib/rules/no-attributes-during-construction.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
/*
* Copyright (c) 2018, salesforce.com, inc.
* All rights reserved.
* SPDX-License-Identifier: MIT
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT
*/
'use strict';

const { isComponent } = require('../util/component');
const { docUrl } = require('../util/doc-url');

// https://github.com/salesforce/lwc/blob/6e6ad1011887d842e3f03546c6a81e9cee057611/packages/%40lwc/shared/src/aria.ts#L10-L67
const ARIA_PROPERTY_NAMES = [
'ariaActiveDescendant',
'ariaAtomic',
'ariaAutoComplete',
'ariaBusy',
'ariaChecked',
'ariaColCount',
'ariaColIndex',
'ariaColSpan',
'ariaControls',
'ariaCurrent',
'ariaDescribedBy',
'ariaDetails',
'ariaDisabled',
'ariaErrorMessage',
'ariaExpanded',
'ariaFlowTo',
'ariaHasPopup',
'ariaHidden',
'ariaInvalid',
'ariaKeyShortcuts',
'ariaLabel',
'ariaLabelledBy',
'ariaLevel',
'ariaLive',
'ariaModal',
'ariaMultiLine',
'ariaMultiSelectable',
'ariaOrientation',
'ariaOwns',
'ariaPlaceholder',
'ariaPosInSet',
'ariaPressed',
'ariaReadOnly',
'ariaRelevant',
'ariaRequired',
'ariaRoleDescription',
'ariaRowCount',
'ariaRowIndex',
'ariaRowSpan',
'ariaSelected',
'ariaSetSize',
'ariaSort',
'ariaValueMax',
'ariaValueMin',
'ariaValueNow',
'ariaValueText',
'role',
];

// https://github.com/salesforce/lwc/blob/6e6ad1011887d842e3f03546c6a81e9cee057611/packages/%40lwc/engine-core/src/framework/attributes.ts#L9-L20
const LWC_DEFAULT_PROPERTY_NAMES = [
'accessKey',
'dir',
'draggable',
'hidden',
'id',
'lang',
'spellcheck',
'tabIndex',
'title',
];

const PROPERTIES_THAT_REFLECT = new Set([...ARIA_PROPERTY_NAMES, ...LWC_DEFAULT_PROPERTY_NAMES]);

function getAccessorMethodNames(bodyItems) {
return bodyItems
.filter(
({ computed, kind, type }) =>
type === 'MethodDefinition' && (kind === 'get' || kind === 'set') && !computed,
)
.map(({ key }) => key.name);
}

function getClassPropertyNames(bodyItems) {
return bodyItems
.filter(({ computed, type }) => type === 'ClassProperty' && !computed)
.map(({ key }) => key.name);
}

function getAssignmentExpressions(bodyItems) {
return bodyItems
.filter(
({ expression, type }) =>
type === 'ExpressionStatement' && expression.type === 'AssignmentExpression',
)
.map(({ expression }) => expression);
}

module.exports = {
meta: {
type: 'problem',
docs: {
description: 'no attributes during construction',
category: 'LWC',
recommended: true,
url: docUrl('no-attributes-during-construction'),
},
schema: [],
},

create(context) {
return {
'MethodDefinition[kind=constructor]': function (ctorMethod) {
const classBody = ctorMethod.parent;

// classBody.parent is ClassDeclaration or ClassExpression
if (!isComponent(classBody.parent, context)) {
return;
}

const blockStatement = ctorMethod.value.body;
const assignmentExpressions = getAssignmentExpressions(blockStatement.body);

if (assignmentExpressions.length !== 0) {
const classPropertyNameSet = new Set(getClassPropertyNames(classBody.body));
const accessorMethodNameSet = new Set(getAccessorMethodNames(classBody.body));

for (const expression of assignmentExpressions) {
const { left } = expression;
const { computed, object, property } = left;
if (object.type === 'ThisExpression' && !computed) {
const { name } = property;
if (
PROPERTIES_THAT_REFLECT.has(name) &&
!classPropertyNameSet.has(name) &&
!accessorMethodNameSet.has(name)
) {
context.report({
message: `Invariant violation: Setting "${name}" in the constructor results in a rendered attribute during construction. Change the name of this property, move this assignment to another lifecycle method, or override the default behavior by defining "${name}" as a property on the class.`,
node: expression,
});
}
}
}
}
},
};
},
};
22 changes: 17 additions & 5 deletions lib/util/component.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,26 @@
* @param {Context} context The Eslint context.
*/
function isComponent(node, context) {
if (!node.superClass) {
const { superClass } = node;

if (!superClass) {
return false;
}

// Checking if the class extends `Element` covers 99% of the LWC cases. We can make this safer
// in the future by ensuring that the `superClass` identifier is imported from engine.
const sourceCode = context.getSourceCode();
return sourceCode.getText(node.superClass) === 'LightningElement';
const program = context.getAncestors(node).find(({ type }) => type === 'Program');

const importDeclaration = program.body
.filter(({ type }) => type === 'ImportDeclaration')
.find(({ source }) => source.value === 'lwc');

if (importDeclaration) {
const importSpecifier = importDeclaration.specifiers.find(
({ imported }) => imported.name === 'LightningElement',
);
return superClass.name === importSpecifier.local.name;
}

return false;
}

module.exports = {
Expand Down
53 changes: 40 additions & 13 deletions test/lib/rules/consistent-component-name.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@ const ruleTester = new RuleTester(ESLINT_TEST_CONFIG);
ruleTester.run('consistent-class-name', rule, {
valid: [
{
code: `export class Foo extends LightningElement {}`,
code: `
import { LightningElement } from 'lwc';
export class Foo extends LightningElement {}
`,
filename: 'foo.js',
},
{
Expand All @@ -26,52 +29,76 @@ ruleTester.run('consistent-class-name', rule, {
],
invalid: [
{
code: `export default class extends LightningElement {}`,
code: `
import { LightningElement } from 'lwc';
export default class extends LightningElement {}
`,
filename: 'foo.js',
errors: [
{
type: 'ClassDeclaration',
column: 16,
column: 32,
message: 'Lightning component class should be named "Foo".',
},
],
output: 'export default class Foo extends LightningElement {}',
output: `
import { LightningElement } from 'lwc';
export default class Foo extends LightningElement {}
`,
},
{
code: `export default class Foo extends LightningElement {}`,
code: `
import { LightningElement } from 'lwc';
export default class Foo extends LightningElement {}
`,
filename: 'bar.js',
errors: [
{
column: 16,
column: 32,
type: 'ClassDeclaration',
message: 'Lightning component class should be named "Bar".',
},
],
output: 'export default class Bar extends LightningElement {}',
output: `
import { LightningElement } from 'lwc';
export default class Bar extends LightningElement {}
`,
},
{
code: `export default class ComplexName extends LightningElement {}`,
code: `
import { LightningElement } from 'lwc';
export default class ComplexName extends LightningElement {}
`,
filename: 'superComplexName.js',
errors: [
{
column: 16,
column: 32,
type: 'ClassDeclaration',
message: 'Lightning component class should be named "SuperComplexName".',
},
],
output: 'export default class SuperComplexName extends LightningElement {}',
output: `
import { LightningElement } from 'lwc';
export default class SuperComplexName extends LightningElement {}
`,
},
{
code: `export default class complexName extends LightningElement {}`,
code: `
import { LightningElement } from 'lwc';
export default class complexName extends LightningElement {}
`,
filename: 'complexName.js',
errors: [
{
column: 16,
column: 32,
type: 'ClassDeclaration',
message: 'Lightning component class should be named "ComplexName".',
},
],
output: 'export default class ComplexName extends LightningElement {}',
output: `
import { LightningElement } from 'lwc';
export default class ComplexName extends LightningElement {}
`,
},
],
});
Loading

0 comments on commit ca33119

Please sign in to comment.