diff --git a/README.md b/README.md index b5aa064bc8..6fd59e211d 100644 --- a/README.md +++ b/README.md @@ -155,6 +155,7 @@ module.exports = { | [no-incorrect-calls-with-inline-anonymous-functions](docs/rules/no-incorrect-calls-with-inline-anonymous-functions.md) | disallow inline anonymous functions as arguments to `debounce`, `once`, and `scheduleOnce` | ✅ | | | | [no-invalid-debug-function-arguments](docs/rules/no-invalid-debug-function-arguments.md) | disallow usages of Ember's `assert()` / `warn()` / `deprecate()` functions that have the arguments passed in the wrong order. | ✅ | | | | [no-restricted-property-modifications](docs/rules/no-restricted-property-modifications.md) | disallow modifying the specified properties | | 🔧 | | +| [no-runloop](docs/rules/no-runloop.md) | disallow usage of `@ember/runloop` functions | | | | | [require-fetch-import](docs/rules/require-fetch-import.md) | enforce explicit import for `fetch()` | | | | ### Routes diff --git a/docs/rules/no-runloop.md b/docs/rules/no-runloop.md new file mode 100644 index 0000000000..ea3cb0116a --- /dev/null +++ b/docs/rules/no-runloop.md @@ -0,0 +1,76 @@ +# ember/no-runloop + + + +Ember's runloop functions are not lifecycle-aware and don't ensure that an object's async is cleaned up. It is recommended to use [`ember-lifeline`](https://ember-lifeline.github.io/ember-lifeline/), [`ember-concurrency`](http://ember-concurrency.com/docs/introduction/), or [`@ember/destroyable`](https://rfcs.emberjs.com/id/0580-destroyables/) instead. + +## Rule Details + +This rule disallows usage of `@ember/runloop` functions. + +## Examples + +Example of **incorrect** code for this rule: + +```js +import Component from '@glimmer/component'; +import { run } from '@ember/runloop'; + +export default class MyComponent extends Component { + constructor(...args) { + super(...args); + + run.later(() => { + this.set('date', new Date()); + }, 500); + } +} +``` + +Example of **correct** code for this rule using `ember-lifeline`: + +```js +import Component from '@glimmer/component'; +import { runTask, runDisposables } from 'ember-lifeline'; + +export default class MyComponent extends Component { + constructor(...args) { + super(...args); + + runTask( + this, + () => { + this.set('date', new Date()); + }, + 500 + ); + } + + willDestroy(...args) { + super.willDestroy(...args); + + runDisposables(this); + } +} +``` + +## Configuration + +If you have `@ember/runloop` functions that you wish to allow, you can configure this rule to allow specific methods. The configuration takes an object with the `allowList` property, which is an array of strings where the strings must be names of runloop functions. + +```js +module.exports = { + rules: { + 'ember/no-runloop': [ + 'error', + { + allowList: ['debounce', 'begin', 'end'], + }, + ], + }, +}; +``` + +## References + +- [require-lifeline](https://github.com/ember-best-practices/eslint-plugin-ember-best-practices/blob/master/guides/rules/require-ember-lifeline.md) - a rule that was originally implemented in eslint-plugin-ember-best-practices diff --git a/lib/rules/no-runloop.js b/lib/rules/no-runloop.js new file mode 100644 index 0000000000..0ea3d45b6a --- /dev/null +++ b/lib/rules/no-runloop.js @@ -0,0 +1,146 @@ +'use strict'; + +//------------------------------------------------------------------------------ +// General rule - Don’t use runloop functions +//------------------------------------------------------------------------------ + +/** + * Map of runloop functions to ember-lifeline recommended replacements + */ +const RUNLOOP_TO_LIFELINE_MAP = Object.freeze({ + later: 'runTask', + next: 'runTask', + debounce: 'debounceTask', + schedule: 'scheduleTask', + throttle: 'throttleTask', +}); + +const ERROR_MESSAGE = + "Don't use @ember/runloop functions. Use ember-lifeline, ember-concurrency, or @ember/destroyable instead."; + +// https://api.emberjs.com/ember/3.24/classes/@ember%2Frunloop +const EMBER_RUNLOOP_FUNCTIONS = [ + 'begin', + 'bind', + 'cancel', + 'debounce', + 'end', + 'join', + 'later', + 'next', + 'once', + 'run', + 'schedule', + 'scheduleOnce', + 'throttle', +]; + +/** @type {import('eslint').Rule.RuleModule} */ +module.exports = { + meta: { + type: 'problem', + docs: { + description: 'disallow usage of `@ember/runloop` functions', + category: 'Miscellaneous', + url: 'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/no-runloop.md', + }, + fixable: null, + schema: [ + { + type: 'object', + properties: { + allowList: { + type: 'array', + uniqueItems: true, + items: { + type: 'string', + enum: EMBER_RUNLOOP_FUNCTIONS, + minItems: 1, + }, + }, + }, + additionalProperties: false, + }, + ], + messages: { + main: ERROR_MESSAGE, + lifelineReplacement: `${ERROR_MESSAGE} For this case, you can replace \`{{actualMethodUsed}}\` with \`{{lifelineEquivalent}}\` from ember-lifeline.`, + }, + }, + + create(context) { + // List of allowed runloop functions + const allowList = context.options[0]?.allowList ?? []; + // Maps local names to imported names of imports + const localToImportedNameMap = {}; + + /** + * Reports a node with usage of a disallowed runloop function + * @param {Node} node + * @param {String} [runloopFn] the name of the runloop function that is not allowed + * @param {String} [localName] the locally used name of the runloop function + */ + const report = function (node, runloopFn, localName) { + if (Object.keys(RUNLOOP_TO_LIFELINE_MAP).includes(runloopFn)) { + // If there is a recommended lifeline replacement, include the suggestion + context.report({ + node, + messageId: 'lifelineReplacement', + data: { + actualMethodUsed: localName, + lifelineEquivalent: RUNLOOP_TO_LIFELINE_MAP[runloopFn], + }, + }); + } else { + // Otherwise, show a generic error message + context.report({ node, messageId: 'main' }); + } + }; + + return { + ImportDeclaration(node) { + if (node.source.value === '@ember/runloop') { + for (const spec of node.specifiers) { + if (spec.type === 'ImportSpecifier') { + const importedName = spec.imported.name; + if (EMBER_RUNLOOP_FUNCTIONS.includes(importedName)) { + localToImportedNameMap[spec.local.name] = importedName; + } + } + } + } + }, + + CallExpression(node) { + // Examples: run(...), later(...) + if (node.callee.type === 'Identifier') { + const name = node.callee.name; + const runloopFn = localToImportedNameMap[name]; + const isNotAllowed = runloopFn && !allowList.includes(runloopFn); + if (isNotAllowed) { + report(node, runloopFn, name); + } + } + + // runloop functions (aside from run itself) can chain onto `run`, so we need to check for this + // Examples: run.later(...), run.schedule(...) + if (node.callee.type === 'MemberExpression' && node.callee.object?.type === 'Identifier') { + const objectName = node.callee.object.name; + const objectRunloopFn = localToImportedNameMap[objectName]; + + if (objectRunloopFn === 'run' && node.callee.property?.type === 'Identifier') { + const runloopFn = node.callee.property.name; + + if ( + EMBER_RUNLOOP_FUNCTIONS.includes(runloopFn) && + runloopFn !== 'run' && + !allowList.includes(runloopFn) + ) { + report(node, runloopFn, `${objectName}.${runloopFn}`); + } + } + } + }, + }; + }, +}; diff --git a/tests/lib/rules/no-runloop.js b/tests/lib/rules/no-runloop.js new file mode 100644 index 0000000000..953a420822 --- /dev/null +++ b/tests/lib/rules/no-runloop.js @@ -0,0 +1,193 @@ +// ------------------------------------------------------------------------------ +// Requirements +// ------------------------------------------------------------------------------ + +const rule = require('../../../lib/rules/no-runloop'); +const RuleTester = require('eslint').RuleTester; + +// ------------------------------------------------------------------------------ +// Tests +// ------------------------------------------------------------------------------ + +const eslintTester = new RuleTester({ + parserOptions: { ecmaVersion: 2020, sourceType: 'module' }, +}); + +eslintTester.run('no-runloop', rule, { + valid: [ + ` + run(); + later(); + `, + ` + import { run } from 'foobar'; + run(); + `, + ` + import { run } from '@ember/runloop'; + this.run(); + runTask(); + runRun(); + `, + { + code: ` + import { run } from '@ember/runloop'; + run(); + later(); + `, + options: [{ allowList: ['run'] }], + }, + { + code: ` + import { run as foobar } from '@ember/runloop'; + foobar(); + `, + options: [{ allowList: ['run'] }], + }, + ` + import run from '@ember/runloop'; + run(); + `, + ` + import { run } from '@ember/runloop'; + run.run(); + run.foobar(); + `, + ` + import { later } from '@ember/runloop'; + later.run(); + later.foobar(); + `, + { + code: ` + import { run } from '@ember/runloop'; + run.later(); + `, + options: [{ allowList: ['later'] }], + }, + ], + invalid: [ + { + code: ` + import { run } from '@ember/runloop'; + run(); + `, + output: null, + errors: [ + { + messageId: 'main', + type: 'CallExpression', + }, + ], + }, + { + code: ` + import { run as foobar } from '@ember/runloop'; + foobar(); + `, + output: null, + errors: [ + { + messageId: 'main', + type: 'CallExpression', + }, + ], + }, + { + code: ` + import { later } from '@ember/runloop'; + later(); + `, + output: null, + errors: [ + { + messageId: 'lifelineReplacement', + type: 'CallExpression', + }, + ], + }, + { + code: ` + import { later as foobar } from '@ember/runloop'; + foobar(); + `, + output: null, + errors: [ + { + messageId: 'lifelineReplacement', + type: 'CallExpression', + }, + ], + }, + { + code: ` + import { run, later } from '@ember/runloop'; + run(); + later(); + `, + output: null, + options: [{ allowList: ['run'] }], + errors: [ + { + messageId: 'lifelineReplacement', + type: 'CallExpression', + }, + ], + }, + { + code: ` + import { run, later } from '@ember/runloop'; + run(); + later(); + `, + output: null, + options: [{ allowList: ['later'] }], + errors: [ + { + messageId: 'main', + type: 'CallExpression', + }, + ], + }, + // chaining off of `run` + { + code: ` + import { run } from '@ember/runloop'; + run.later(); + `, + output: null, + errors: [ + { + messageId: 'lifelineReplacement', + type: 'CallExpression', + }, + ], + }, + { + code: ` + import { run } from '@ember/runloop'; + run.begin(); + `, + output: null, + errors: [ + { + messageId: 'main', + type: 'CallExpression', + }, + ], + }, + { + code: ` + import { run as foobar } from '@ember/runloop'; + foobar.schedule(); + `, + output: null, + errors: [ + { + messageId: 'lifelineReplacement', + type: 'CallExpression', + }, + ], + }, + ], +});