diff --git a/README.md b/README.md index 68910d7..598c6b7 100644 --- a/README.md +++ b/README.md @@ -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 | | diff --git a/docs/rules/no-attributes-during-construction.md b/docs/rules/no-attributes-during-construction.md new file mode 100644 index 0000000..9bb1780 --- /dev/null +++ b/docs/rules/no-attributes-during-construction.md @@ -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'; + } +} +``` diff --git a/lib/index.js b/lib/index.js index ce05425..72ee544 100644 --- a/lib/index.js +++ b/lib/index.js @@ -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'), diff --git a/lib/rules/no-attributes-during-construction.js b/lib/rules/no-attributes-during-construction.js new file mode 100644 index 0000000..694d5f3 --- /dev/null +++ b/lib/rules/no-attributes-during-construction.js @@ -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, + }); + } + } + } + } + }, + }; + }, +}; diff --git a/lib/util/component.js b/lib/util/component.js index be1d870..691b18a 100644 --- a/lib/util/component.js +++ b/lib/util/component.js @@ -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 = { diff --git a/test/lib/rules/consistent-component-name.js b/test/lib/rules/consistent-component-name.js index 28b356b..a9a23f4 100644 --- a/test/lib/rules/consistent-component-name.js +++ b/test/lib/rules/consistent-component-name.js @@ -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', }, { @@ -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 {} + `, }, ], }); diff --git a/test/lib/rules/no-attributes-during-construction.js b/test/lib/rules/no-attributes-during-construction.js new file mode 100644 index 0000000..798411b --- /dev/null +++ b/test/lib/rules/no-attributes-during-construction.js @@ -0,0 +1,184 @@ +/* + * 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 { RuleTester } = require('eslint'); + +const { ESLINT_TEST_CONFIG } = require('../shared'); +const rule = require('../../../lib/rules/no-attributes-during-construction'); + +const ruleTester = new RuleTester(ESLINT_TEST_CONFIG); + +ruleTester.run('no-attributes-during-construction', rule, { + valid: [ + { + code: ` + class LightningElement {} + class Test extends LightningElement { + constructor() { + this.tabIndex = '0'; + } + } + `, + }, + { + code: ` + import { LightningElement } from 'lwc'; + class Test extends LightningElement { + tabIndex; + constructor() { + this.tabIndex = '0'; + } + } + `, + }, + { + code: ` + import { LightningElement as Component } from 'lwc'; + class Test extends Component { + title; + constructor() { + this.title = 'test'; + } + } + `, + }, + { + code: ` + import { LightningElement } from 'lwc'; + class Test extends LightningElement { + ariaDescribedBy = 'foo'; + constructor() { + this.ariaDescribedBy = 'test'; + } + } + `, + }, + { + code: ` + import { track, LightningElement } from 'lwc'; + class Test extends LightningElement { + @track role = []; + constructor() { + this.role = ['test']; + } + } + `, + }, + { + code: ` + import { LightningElement } from 'lwc'; + class Test extends LightningElement { + get role() {} + constructor() { + this.role = 'test'; + } + } + `, + }, + { + code: ` + import { LightningElement } from 'lwc'; + class Test extends LightningElement { + set hidden(val) {} + constructor() { + this.hidden = 'test'; + } + } + `, + }, + { + code: ` + import { LightningElement } from 'lwc'; + const foo = {}; + class Test extends LightningElement { + constructor() { + foo.hidden = 'test'; + } + } + `, + }, + ], + invalid: [ + { + code: ` + import { LightningElement } from 'lwc'; + class Test extends LightningElement { + constructor() { + this.tabIndex = '0'; + } + } + `, + errors: [ + { + message: + 'Invariant violation: Setting "tabIndex" 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 "tabIndex" as a property on the class.', + type: 'AssignmentExpression', + column: 25, + line: 5, + }, + ], + }, + { + code: ` + import { LightningElement as Component } from 'lwc'; + class Test extends Component { + constructor() { + this.title = 'test'; + } + } + `, + errors: [ + { + message: + 'Invariant violation: Setting "title" 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 "title" as a property on the class.', + type: 'AssignmentExpression', + column: 25, + line: 5, + }, + ], + }, + { + code: ` + import { LightningElement } from 'lwc'; + class Test extends LightningElement { + constructor() { + this.ariaDescribedBy = 'test'; + } + } + `, + errors: [ + { + message: + 'Invariant violation: Setting "ariaDescribedBy" 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 "ariaDescribedBy" as a property on the class.', + type: 'AssignmentExpression', + column: 25, + line: 5, + }, + ], + }, + { + code: ` + import { track, LightningElement } from 'lwc'; + class Test extends LightningElement { + constructor() { + this.role = ['test']; + } + } + `, + errors: [ + { + message: + 'Invariant violation: Setting "role" 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 "role" as a property on the class.', + type: 'AssignmentExpression', + column: 25, + line: 5, + }, + ], + }, + ], +});