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

Add new rule no-runloop #1703

Merged
merged 4 commits into from Dec 20, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Expand Up @@ -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
Expand Down
71 changes: 71 additions & 0 deletions docs/rules/no-runloop.md
@@ -0,0 +1,71 @@
# ember/no-runloop

<!-- end auto-generated rule header -->

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:

```ts
lin-ll marked this conversation as resolved.
Show resolved Hide resolved
import Component from '@glimmer/component';
import { run } from '@ember/runloop';

export default class MyComponent extends Component {
constructor() {
super(...arguments);

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 array of strings, where the strings must be names of runloop functions.

```ts
lin-ll marked this conversation as resolved.
Show resolved Hide resolved
module.exports = {
rules: {
'ember/no-runloop': ['error', ['debounce', 'begin', 'end']],
lin-ll marked this conversation as resolved.
Show resolved Hide resolved
},
};
```

## 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
135 changes: 135 additions & 0 deletions lib/rules/no-runloop.js
@@ -0,0 +1,135 @@
'use strict';

const { getImportIdentifier } = require('../utils/import');
const { isIdentifier, isMemberExpression } = require('../utils/types');
lin-ll marked this conversation as resolved.
Show resolved Hide resolved

//------------------------------------------------------------------------------
// 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: 'array',
uniqueItems: true,
lin-ll marked this conversation as resolved.
Show resolved Hide resolved
items: { type: 'string', enum: EMBER_RUNLOOP_FUNCTIONS },
},
],
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] ?? [];
// Maps local names to imported names
const localMap = {};
lin-ll marked this conversation as resolved.
Show resolved Hide resolved

/**
* 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 fn of EMBER_RUNLOOP_FUNCTIONS) {
const importIdentifier = getImportIdentifier(node, '@ember/runloop', fn);
localMap[importIdentifier] = fn;
}
}
},

CallExpression(node) {
// Examples: run(...), later(...)
if (isIdentifier(node.callee)) {
const name = node.callee.name;
const runloopFn = localMap[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 (isMemberExpression(node.callee) && isIdentifier(node.callee.object)) {
const objectName = node.callee.object.name;
const objectRunloopFn = localMap[objectName];

if (objectRunloopFn === 'run' && isIdentifier(node.callee.property)) {
const runloopFn = node.callee.property.name;

if (
EMBER_RUNLOOP_FUNCTIONS.includes(runloopFn) &&
runloopFn !== 'run' &&
!allowList.includes(runloopFn)
) {
report(node, runloopFn, `${objectName}.${runloopFn}`);
}
}
}
},
};
},
};
148 changes: 148 additions & 0 deletions tests/lib/rules/no-runloop.js
@@ -0,0 +1,148 @@
// ------------------------------------------------------------------------------
// 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: [['run']],
},
{
code: `
import { run as foobar } from '@ember/runloop';
foobar();
`,
options: [['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: [['later']],
},
],
invalid: [
{
code: `
import { run } from '@ember/runloop';
run();
`,
output: null,
errors: [{ messageId: 'main' }],
lin-ll marked this conversation as resolved.
Show resolved Hide resolved
},
{
code: `
import { run as foobar } from '@ember/runloop';
foobar();
`,
output: null,
errors: [{ messageId: 'main' }],
},
{
code: `
import { later } from '@ember/runloop';
later();
`,
output: null,
errors: [{ messageId: 'lifelineReplacement' }],
},
{
code: `
import { later as foobar } from '@ember/runloop';
foobar();
`,
output: null,
errors: [{ messageId: 'lifelineReplacement' }],
},
{
code: `
import { run, later } from '@ember/runloop';
run();
later();
`,
output: null,
options: [['run']],
errors: [{ messageId: 'lifelineReplacement' }],
},
{
code: `
import { run, later } from '@ember/runloop';
run();
later();
`,
output: null,
options: [['later']],
errors: [{ messageId: 'main' }],
},
// chaining off of `run`
{
code: `
import { run } from '@ember/runloop';
run.later();
`,
output: null,
errors: [{ messageId: 'lifelineReplacement' }],
},
{
code: `
import { run } from '@ember/runloop';
run.begin();
`,
output: null,
errors: [{ messageId: 'main' }],
},
{
code: `
import { run as foobar } from '@ember/runloop';
foobar.schedule();
`,
output: null,
errors: [{ messageId: 'lifelineReplacement' }],
},
],
});