diff --git a/README.md b/README.md index 57c7bfedd8..b4a0fe49ab 100644 --- a/README.md +++ b/README.md @@ -127,6 +127,7 @@ The `--fix` option on the command line automatically fixes problems reported by | | Rule ID | Description | |:---|:--------|:------------| +| | [avoid-leaking-callbacks-in-ember-objects](./docs/rules/avoid-leaking-callbacks-in-ember-objects.md) | Avoids callback memory leaks | | :white_check_mark: | [avoid-leaking-state-in-ember-objects](./docs/rules/avoid-leaking-state-in-ember-objects.md) | Avoids state leakage | | | [computed-property-getters](./docs/rules/computed-property-getters.md) | Enforce the consistent use of getters in computed properties | diff --git a/docs/rules/avoid-leaking-callbacks-in-ember-objects.md b/docs/rules/avoid-leaking-callbacks-in-ember-objects.md new file mode 100644 index 0000000000..6a671a3699 --- /dev/null +++ b/docs/rules/avoid-leaking-callbacks-in-ember-objects.md @@ -0,0 +1,41 @@ + +# Avoid callback leaks in Ember objects +## Rule `avoid-leaking-callbacks-in-ember-objects` + +Callback leaks are memory leaks that occur due to state being caught in a callback function that is never released from memory. + +Since callback functions for things like event listeners and interval timers are retained by reference elsewhere, you must be careful to unregister them when no longer needed or ensure that the context they're registered with is destroyed. + + +Examples of **incorrect** code for this rule: + +```js +export default Ember.Component.extend({ + didInsertElement() { + if (this.get('onScroll')) { + window.addEventListener('scroll', (...args) => this.get('onScroll')(...args)); + } + } +}); +``` + +Examples of **correct** code for this rule: + +```js +export default Ember.Component.extend({ + didInsertElement() { + if (this.get('onScroll')) { + this._onScrollHandler = (...args) => this.get('onScroll')(...args); + window.addEventListener('scroll', this._onScrollHandler); + } + }, + + willDestroy() { + window.removeEventListener('scroll', this._onScrollHandler); + } +}); +``` + +## Further Reading + +https://github.com/ember-best-practices/memory-leak-examples/blob/master/exercises/exercise-2.md diff --git a/lib/index.js b/lib/index.js index a1baf5e51a..6f4b87351f 100644 --- a/lib/index.js +++ b/lib/index.js @@ -4,6 +4,7 @@ module.exports = { rules: { 'alias-model-in-controller': require('./rules/alias-model-in-controller'), + 'avoid-leaking-callbacks-in-ember-objects': require('./rules/avoid-leaking-callbacks-in-ember-objects'), 'avoid-leaking-state-in-components': require('./rules/avoid-leaking-state-in-components'), 'avoid-leaking-state-in-ember-objects': require('./rules/avoid-leaking-state-in-ember-objects'), 'avoid-using-needs-in-controllers': require('./rules/avoid-using-needs-in-controllers'), diff --git a/lib/recommended-rules.js b/lib/recommended-rules.js index 06bb6a365c..4f8d758334 100644 --- a/lib/recommended-rules.js +++ b/lib/recommended-rules.js @@ -6,6 +6,7 @@ */ module.exports = { "ember/alias-model-in-controller": "off", + "ember/avoid-leaking-callbacks-in-ember-objects": "error", "ember/avoid-leaking-state-in-components": "off", "ember/avoid-leaking-state-in-ember-objects": "error", "ember/avoid-using-needs-in-controllers": "error", @@ -50,4 +51,4 @@ module.exports = { "ember/routes-segments-snake-case": "error", "ember/use-brace-expansion": "error", "ember/use-ember-get-and-set": "off" -} +} \ No newline at end of file diff --git a/lib/rules/avoid-leaking-callbacks-in-ember-objects.js b/lib/rules/avoid-leaking-callbacks-in-ember-objects.js new file mode 100644 index 0000000000..3c1a6e414a --- /dev/null +++ b/lib/rules/avoid-leaking-callbacks-in-ember-objects.js @@ -0,0 +1,65 @@ +'use strict'; + +const utils = require('../utils/utils'); + +const ERROR_MESSAGE = 'All `addEventListener` calls should have a corresponding `removeEventListener` to avoid leaking callbacks.'; +//------------------------------------------------------------------------------ +// Ember object rule - Avoid callback memory leaks +// Callback leaks are memory leaks that occur due to state being caught +// in a callback function that is never released from memory. +//------------------------------------------------------------------------------ + +module.exports = { + meta: { + docs: { + description: 'Avoids callback memory leaks', + category: 'Ember Object', + url: 'https://github.com/ember-best-practices/memory-leak-examples/blob/master/exercises/exercise-2.md' + }, + fixable: null, // or "code" or "whitespace" + ERROR_MESSAGE, + }, + + + create(context) { + const report = function (node) { + context.report(node, ERROR_MESSAGE); + }; + + const addedListeners = []; + const removedListeners = []; + return { + 'Program:exit': function () { + addedListeners.forEach((a) => { + const idx = removedListeners.findIndex(r => r.el === a.el && r.event === a.event); + + if (idx < 0) { + // No removeEventListener + report(a.node); + } + }); + }, + + CallExpression(node) { + if (utils.getPropertyValue(node, 'callee.property.name') === 'addEventListener') { + const listener = { + el: node.callee.object.name, + event: node.arguments[0].value, + node + }; + + addedListeners.push(listener); + } + + if (utils.getPropertyValue(node, 'callee.property.name') === 'removeEventListener') { + const listener = { + el: node.callee.object.name, + event: node.arguments[0].value + }; + + removedListeners.push(listener); + } + } + }; + }, +}; diff --git a/tests/lib/rules/avoid-leaking-callbacks-in-ember-objects.js b/tests/lib/rules/avoid-leaking-callbacks-in-ember-objects.js new file mode 100644 index 0000000000..41af8accda --- /dev/null +++ b/tests/lib/rules/avoid-leaking-callbacks-in-ember-objects.js @@ -0,0 +1,141 @@ +// ------------------------------------------------------------------------------ +// Requirements +// ------------------------------------------------------------------------------ + +const rule = require('../../../lib/rules/avoid-leaking-callbacks-in-ember-objects'); +const RuleTester = require('eslint').RuleTester; + +const { ERROR_MESSAGE } = rule; +// ------------------------------------------------------------------------------ +// Tests +// ------------------------------------------------------------------------------ + + +const eslintTester = new RuleTester({ + parserOptions: { + ecmaVersion: 2018, + sourceType: 'module' + } +}); +eslintTester.run('avoid-leaking-callbacks-in-ember-objects', rule, { + valid: [ + { + code: ` +import Component from '@ember/component'; + +export default Component.extend({ + didInsertElement() { + if (this.get('onScroll')) { + this._onScrollHandler = (...args) => this.get('onScroll')(...args); + window.addEventListener('scroll', this._onScrollHandler); + } + }, + + willDestroyElement() { + window.removeEventListener("scroll", this._onScrollHandler); + this._super(...arguments); + } +}); + `, + }, + { + code: ` +import Component from '@ember/component'; + +export default Component.extend({});`, + }, + { + code: ` +export default Ember.Service.extend({ + init() { + if (this.get('onScroll')) { + this._onScrollHandler = (...args) => this.get('onScroll')(...args); + window.addEventListener('scroll', this._onScrollHandler); + } + }, + + willDestroy() { + window.removeEventListener("scroll", this._onScrollHandler); + this._super(...arguments); + } +});`, + }, + ], + invalid: [ + { + code: ` +import Component from '@ember/component'; + +export default Component.extend({ + didInsertElement() { + if (this.get('onScroll')) { + window.addEventListener('scroll', (...args) => this.get('onScroll')(...args)); + } + } +});`, + output: null, + errors: [{ + message: ERROR_MESSAGE, + }], + }, + { + code: ` +import Component from '@ember/component'; +export default Component.extend({ + init() { + if (this.get('onScroll')) { + window.addEventListener('scroll', (...args) => this.get('onScroll')(...args)); + } + }, + + didInsertElement() { + if (this.get("onResize")) { + window.addEventListener("resize", (...args) => this.get("onResize")(...args)); + } + } + +}); + `, + output: null, + errors: [{ + message: ERROR_MESSAGE, + }, + { + message: ERROR_MESSAGE, + }], + }, + { + code: ` +import Component from '@ember/component'; + +export default Component.extend({ + init() { + if (this.get('onScroll')) { + this._onScrollHandler = (...args) => this.get('onScroll')(...args); + window.addEventListener('scroll', this._onScrollHandler); + } + }, + + + didInsertElement() { + if (this.get('onResize')) { + window.addEventListener('resize', (...args) => this.get('onResize')(...args)); + } + }, + + willDestroyElement() { + window.removeEventListener("scroll", this._onScrollHandler); + this._super(...arguments); + } + +}); + `, + output: null, + errors: [{ + message: ERROR_MESSAGE, + }], + }, + + + ], +});