From 80121256100a98b46652043897bf3bef0c6f5029 Mon Sep 17 00:00:00 2001 From: Rajasegar Date: Sat, 25 May 2019 07:07:32 +0530 Subject: [PATCH 1/9] [NEW RULE] no-callback-leaks Added new rule and tests to avoid callback memory leaks --- .../no-callback-leaks-in-ember-objects.js | 72 +++++++++ .../no-callback-leaks-in-ember-objects.js | 152 ++++++++++++++++++ 2 files changed, 224 insertions(+) create mode 100644 lib/rules/no-callback-leaks-in-ember-objects.js create mode 100644 tests/lib/rules/no-callback-leaks-in-ember-objects.js diff --git a/lib/rules/no-callback-leaks-in-ember-objects.js b/lib/rules/no-callback-leaks-in-ember-objects.js new file mode 100644 index 0000000000..32e8293ad2 --- /dev/null +++ b/lib/rules/no-callback-leaks-in-ember-objects.js @@ -0,0 +1,72 @@ +'use strict'; + +// const ember = require('../utils/ember'); +const utils = require('../utils/utils'); + + +//------------------------------------------------------------------------------ +// 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', + recommended: true, + url: 'https://github.com/ember-best-practices/memory-leak-examples/blob/master/exercises/exercise-2.md' + }, + fixable: null, // or "code" or "whitespace" + schema: [{ + type: 'array', + items: { type: 'string' }, + }], + }, + + + create(context) { + const report = function (node) { + const message = "Event listeners and interval timers must be unregistered or ensure that the context they're registered with is destroyed, when no longer needed."; + context.report(node, message); + }; + + const addedListeners = []; + const removedListeners = []; + return { + 'ExportDefaultDeclaration:exit': function (node) { + addedListeners.forEach((a) => { + const idx = removedListeners.findIndex(r => r.el === a.el); + + if (idx < 0) { + // No removeEventListener + report(node); + } + }); + }, + + CallExpression(node) { + // console.log(node.callee.property); + if (utils.getPropertyValue(node, 'callee.property.name') === 'addEventListener') { + // if (node.callee.property.name === 'addEventListener') { + const listener = { + el: node.callee.object.name, + event: node.arguments[0].value, + }; + + 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/no-callback-leaks-in-ember-objects.js b/tests/lib/rules/no-callback-leaks-in-ember-objects.js new file mode 100644 index 0000000000..cb397a9b9b --- /dev/null +++ b/tests/lib/rules/no-callback-leaks-in-ember-objects.js @@ -0,0 +1,152 @@ +// ------------------------------------------------------------------------------ +// Requirements +// ------------------------------------------------------------------------------ + +const rule = require('../../../lib/rules/no-callback-leaks-in-ember-objects'); +const RuleTester = require('eslint').RuleTester; + +// ------------------------------------------------------------------------------ +// Tests +// ------------------------------------------------------------------------------ + + +const message = "Event listeners and interval timers must be unregistered or ensure that the context they're registered with is destroyed, when no longer needed."; + +const eslintTester = new RuleTester({ + parserOptions: { + ecmaVersion: 2018, + sourceType: 'module' + } +}); +eslintTester.run('no-callback-leaks-in-ember-objects', rule, { + valid: [ + { + code: ` +export default Ember.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({ + 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: ` + export default Ember.Component.extend({ + didInsertElement() { + if (this.get("onScroll")) { + window.addEventListener("scroll", (...args) => this.get("onScroll")(...args)); + } + } + });`, + output: null, + errors: [{ + message, + }], + }, + { + 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, + }], + }, + { + code: ` +export default Ember.Service.extend({ + init() { + if (this.get('onScroll')) { + window.addEventListener('scroll', (...args) => this.get('onScroll')(...args)); + } + } +}); + `, + output: null, + errors: [{ + message, + }], + }, + { + code: ` +import Component from '@ember/component'; + +export default Component.extend({ + didInsertElement() { + if (this.get('onScroll')) { + window.addEventListener('scroll', (...args) => this.get('onScroll')(...args)); + } + }, + + willDestroyElement() { + this._super(...arguments); + } + +}); + `, + output: null, + errors: [{ + message, + }], + }, + + + ], +}); From 707e9a8ed69e39fd4b69318b7b0a0aee7fe9f5ff Mon Sep 17 00:00:00 2001 From: Rajasegar Chandran Date: Wed, 29 May 2019 06:38:18 +0530 Subject: [PATCH 2/9] [DOCS] Adding doc page for the rule and ref in README --- README.md | 1 + .../no-callback-leaks-in-ember-objects.md | 41 +++++++++++++++++++ 2 files changed, 42 insertions(+) create mode 100644 docs/rules/no-callback-leaks-in-ember-objects.md diff --git a/README.md b/README.md index 0bc38a0154..e2c4abfceb 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 | |:---|:--------|:------------| | :white_check_mark: | [avoid-leaking-state-in-ember-objects](./docs/rules/avoid-leaking-state-in-ember-objects.md) | Avoids state leakage | +| :white_check_mark: | [no-callback-leaks-in-ember-objects](./docs/rules/no-callback-leaks-in-ember-objects.md) | Avoids callback memory leaks | ### Testing diff --git a/docs/rules/no-callback-leaks-in-ember-objects.md b/docs/rules/no-callback-leaks-in-ember-objects.md new file mode 100644 index 0000000000..7de4720ffe --- /dev/null +++ b/docs/rules/no-callback-leaks-in-ember-objects.md @@ -0,0 +1,41 @@ + +# Avoid callback leaks in Ember objects +## Rule `no-callback-leaks-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 From 87bed74f33bf5abe92baeb0a54a5e10f382bfcab Mon Sep 17 00:00:00 2001 From: Rajasegar Chandran Date: Mon, 3 Jun 2019 11:43:49 +0530 Subject: [PATCH 3/9] [CHORE] Refactoring the rule and tests based on review comments 1. Moving error messages to exports 2. Removing commented code 3. Changing the erro message to new string 4. Ran yarn update --- lib/recommended-rules.js | 3 ++- lib/rules/no-callback-leaks-in-ember-objects.js | 14 ++++---------- .../rules/no-callback-leaks-in-ember-objects.js | 11 +++++------ 3 files changed, 11 insertions(+), 17 deletions(-) diff --git a/lib/recommended-rules.js b/lib/recommended-rules.js index 06bb6a365c..e8063cfe6c 100644 --- a/lib/recommended-rules.js +++ b/lib/recommended-rules.js @@ -17,6 +17,7 @@ module.exports = { "ember/new-module-imports": "error", "ember/no-attrs-in-components": "error", "ember/no-attrs-snapshot": "error", + "ember/no-callback-leaks-in-ember-objects": "error", "ember/no-capital-letters-in-routes": "error", "ember/no-deeply-nested-dependent-keys-with-each": "off", "ember/no-duplicate-dependent-keys": "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/no-callback-leaks-in-ember-objects.js b/lib/rules/no-callback-leaks-in-ember-objects.js index 32e8293ad2..f4ae9d4a36 100644 --- a/lib/rules/no-callback-leaks-in-ember-objects.js +++ b/lib/rules/no-callback-leaks-in-ember-objects.js @@ -3,7 +3,7 @@ // const ember = require('../utils/ember'); 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 @@ -19,23 +19,19 @@ module.exports = { url: 'https://github.com/ember-best-practices/memory-leak-examples/blob/master/exercises/exercise-2.md' }, fixable: null, // or "code" or "whitespace" - schema: [{ - type: 'array', - items: { type: 'string' }, - }], + ERROR_MESSAGE, }, create(context) { const report = function (node) { - const message = "Event listeners and interval timers must be unregistered or ensure that the context they're registered with is destroyed, when no longer needed."; - context.report(node, message); + context.report(node, ERROR_MESSAGE); }; const addedListeners = []; const removedListeners = []; return { - 'ExportDefaultDeclaration:exit': function (node) { + 'Program:exit': function (node) { addedListeners.forEach((a) => { const idx = removedListeners.findIndex(r => r.el === a.el); @@ -47,9 +43,7 @@ module.exports = { }, CallExpression(node) { - // console.log(node.callee.property); if (utils.getPropertyValue(node, 'callee.property.name') === 'addEventListener') { - // if (node.callee.property.name === 'addEventListener') { const listener = { el: node.callee.object.name, event: node.arguments[0].value, diff --git a/tests/lib/rules/no-callback-leaks-in-ember-objects.js b/tests/lib/rules/no-callback-leaks-in-ember-objects.js index cb397a9b9b..6a3210abe2 100644 --- a/tests/lib/rules/no-callback-leaks-in-ember-objects.js +++ b/tests/lib/rules/no-callback-leaks-in-ember-objects.js @@ -5,13 +5,12 @@ const rule = require('../../../lib/rules/no-callback-leaks-in-ember-objects'); const RuleTester = require('eslint').RuleTester; +const { ERROR_MESSAGE } = rule; // ------------------------------------------------------------------------------ // Tests // ------------------------------------------------------------------------------ -const message = "Event listeners and interval timers must be unregistered or ensure that the context they're registered with is destroyed, when no longer needed."; - const eslintTester = new RuleTester({ parserOptions: { ecmaVersion: 2018, @@ -90,7 +89,7 @@ export default Ember.Service.extend({ });`, output: null, errors: [{ - message, + message: ERROR_MESSAGE, }], }, { @@ -106,7 +105,7 @@ export default Component.extend({ });`, output: null, errors: [{ - message, + message: ERROR_MESSAGE, }], }, { @@ -121,7 +120,7 @@ export default Ember.Service.extend({ `, output: null, errors: [{ - message, + message: ERROR_MESSAGE, }], }, { @@ -143,7 +142,7 @@ export default Component.extend({ `, output: null, errors: [{ - message, + message: ERROR_MESSAGE, }], }, From 4fb3db266ce275273313652b0e4a04bc0f068ad0 Mon Sep 17 00:00:00 2001 From: Rajasegar Chandran Date: Mon, 3 Jun 2019 11:48:05 +0530 Subject: [PATCH 4/9] [CHORE] Renaming rules and assets 1. Renaming the rule to `avoid-leaking-callbacks-in-ember-objects` --- ...ber-objects.md => avoid-leaking-callbacks-in-ember-objects.md} | 0 ...ber-objects.js => avoid-leaking-callbacks-in-ember-objects.js} | 0 ...ber-objects.js => avoid-leaking-callbacks-in-ember-objects.js} | 0 3 files changed, 0 insertions(+), 0 deletions(-) rename docs/rules/{no-callback-leaks-in-ember-objects.md => avoid-leaking-callbacks-in-ember-objects.md} (100%) rename lib/rules/{no-callback-leaks-in-ember-objects.js => avoid-leaking-callbacks-in-ember-objects.js} (100%) rename tests/lib/rules/{no-callback-leaks-in-ember-objects.js => avoid-leaking-callbacks-in-ember-objects.js} (100%) diff --git a/docs/rules/no-callback-leaks-in-ember-objects.md b/docs/rules/avoid-leaking-callbacks-in-ember-objects.md similarity index 100% rename from docs/rules/no-callback-leaks-in-ember-objects.md rename to docs/rules/avoid-leaking-callbacks-in-ember-objects.md diff --git a/lib/rules/no-callback-leaks-in-ember-objects.js b/lib/rules/avoid-leaking-callbacks-in-ember-objects.js similarity index 100% rename from lib/rules/no-callback-leaks-in-ember-objects.js rename to lib/rules/avoid-leaking-callbacks-in-ember-objects.js diff --git a/tests/lib/rules/no-callback-leaks-in-ember-objects.js b/tests/lib/rules/avoid-leaking-callbacks-in-ember-objects.js similarity index 100% rename from tests/lib/rules/no-callback-leaks-in-ember-objects.js rename to tests/lib/rules/avoid-leaking-callbacks-in-ember-objects.js From 4cabe4cf2b138e25bd2e9407e5a7c9530af0f10e Mon Sep 17 00:00:00 2001 From: Rajasegar Chandran Date: Mon, 3 Jun 2019 14:19:01 +0530 Subject: [PATCH 5/9] [CHORE] Fixing rule names in all the places 1. Updating rule names in Readme 2. Adding rule to index.js 3. Adding node information for proper error reporting by using addEventListener nodes 4. Fixing test cases and removing duplicates --- README.md | 2 +- lib/index.js | 1 + ...void-leaking-callbacks-in-ember-objects.js | 7 +-- tests/__snapshots__/recommended.js.snap | 1 + ...void-leaking-callbacks-in-ember-objects.js | 46 +++++++++++-------- 5 files changed, 33 insertions(+), 24 deletions(-) diff --git a/README.md b/README.md index 4ba00f3953..b0d8ffdb85 100644 --- a/README.md +++ b/README.md @@ -129,7 +129,7 @@ The `--fix` option on the command line automatically fixes problems reported by |:---|:--------|:------------| | :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 | -| :white_check_mark: | [no-callback-leaks-in-ember-objects](./docs/rules/no-callback-leaks-in-ember-objects.md) | Avoids callback memory leaks | +| :white_check_mark: | [avoid-leaking-callbacks-in-ember-objects](./docs/rules/avoid-leaking-callbacks-in-ember-objects.md) | Avoids callback memory leaks | ### Testing diff --git a/lib/index.js b/lib/index.js index a1baf5e51a..3f45450ddf 100644 --- a/lib/index.js +++ b/lib/index.js @@ -47,6 +47,7 @@ module.exports = { 'routes-segments-snake-case': require('./rules/routes-segments-snake-case'), 'use-brace-expansion': require('./rules/use-brace-expansion'), 'use-ember-get-and-set': require('./rules/use-ember-get-and-set'), + 'avoid-leaking-callbacks-in-ember-objects': require('./rules/avoid-leaking-callbacks-in-ember-objects'), }, configs: { base: require('./config/base'), diff --git a/lib/rules/avoid-leaking-callbacks-in-ember-objects.js b/lib/rules/avoid-leaking-callbacks-in-ember-objects.js index f4ae9d4a36..9e894be8c9 100644 --- a/lib/rules/avoid-leaking-callbacks-in-ember-objects.js +++ b/lib/rules/avoid-leaking-callbacks-in-ember-objects.js @@ -31,13 +31,13 @@ module.exports = { const addedListeners = []; const removedListeners = []; return { - 'Program:exit': function (node) { + 'Program:exit': function () { addedListeners.forEach((a) => { - const idx = removedListeners.findIndex(r => r.el === a.el); + const idx = removedListeners.findIndex(r => r.el === a.el && r.event === a.event); if (idx < 0) { // No removeEventListener - report(node); + report(a.node); } }); }, @@ -47,6 +47,7 @@ module.exports = { const listener = { el: node.callee.object.name, event: node.arguments[0].value, + node }; addedListeners.push(listener); diff --git a/tests/__snapshots__/recommended.js.snap b/tests/__snapshots__/recommended.js.snap index 4e0a2655a4..8f5922fd2e 100644 --- a/tests/__snapshots__/recommended.js.snap +++ b/tests/__snapshots__/recommended.js.snap @@ -22,5 +22,6 @@ Array [ "require-super-in-init", "routes-segments-snake-case", "use-brace-expansion", + "avoid-leaking-callbacks-in-ember-objects", ] `; diff --git a/tests/lib/rules/avoid-leaking-callbacks-in-ember-objects.js b/tests/lib/rules/avoid-leaking-callbacks-in-ember-objects.js index 6a3210abe2..c89b1b833f 100644 --- a/tests/lib/rules/avoid-leaking-callbacks-in-ember-objects.js +++ b/tests/lib/rules/avoid-leaking-callbacks-in-ember-objects.js @@ -2,7 +2,7 @@ // Requirements // ------------------------------------------------------------------------------ -const rule = require('../../../lib/rules/no-callback-leaks-in-ember-objects'); +const rule = require('../../../lib/rules/avoid-leaking-callbacks-in-ember-objects'); const RuleTester = require('eslint').RuleTester; const { ERROR_MESSAGE } = rule; @@ -17,7 +17,7 @@ const eslintTester = new RuleTester({ sourceType: 'module' } }); -eslintTester.run('no-callback-leaks-in-ember-objects', rule, { +eslintTester.run('avoid-leaking-callbacks-in-ember-objects', rule, { valid: [ { code: ` @@ -80,20 +80,6 @@ export default Ember.Service.extend({ invalid: [ { code: ` - export default Ember.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({ @@ -110,17 +96,28 @@ export default Component.extend({ }, { code: ` -export default Ember.Service.extend({ +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, }], }, { @@ -128,13 +125,22 @@ export default Ember.Service.extend({ import Component from '@ember/component'; export default Component.extend({ - didInsertElement() { + init() { if (this.get('onScroll')) { - window.addEventListener('scroll', (...args) => this.get('onScroll')(...args)); + 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); } From 3d4cf727f39126432ee335b430b412ac928b03f8 Mon Sep 17 00:00:00 2001 From: Rajasegar Chandran Date: Mon, 3 Jun 2019 14:27:53 +0530 Subject: [PATCH 6/9] [CHORE] Updated rule name in the docs --- docs/rules/avoid-leaking-callbacks-in-ember-objects.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/rules/avoid-leaking-callbacks-in-ember-objects.md b/docs/rules/avoid-leaking-callbacks-in-ember-objects.md index 7de4720ffe..6a671a3699 100644 --- a/docs/rules/avoid-leaking-callbacks-in-ember-objects.md +++ b/docs/rules/avoid-leaking-callbacks-in-ember-objects.md @@ -1,6 +1,6 @@ # Avoid callback leaks in Ember objects -## Rule `no-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. From db6d2dff417266fa14c841edd5669c70527fd818 Mon Sep 17 00:00:00 2001 From: Rajasegar Chandran Date: Mon, 3 Jun 2019 14:31:13 +0530 Subject: [PATCH 7/9] [CHORE] Yarn update task for callback leaks rule Ran yarn update to update the new rule names in readme and recommended rules --- README.md | 18 +++++++++--------- lib/recommended-rules.js | 2 +- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index b0d8ffdb85..47009b883b 100644 --- a/README.md +++ b/README.md @@ -104,6 +104,15 @@ The `--fix` option on the command line automatically fixes problems reported by | :wrench: | [use-ember-get-and-set](./docs/rules/use-ember-get-and-set.md) | Enforces usage of Ember.get and Ember.set | +### Ember Object + +| | Rule ID | Description | +|:---|:--------|:------------| +| :white_check_mark: | [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 | + + ### Possible Errors | | Rule ID | Description | @@ -123,15 +132,6 @@ The `--fix` option on the command line automatically fixes problems reported by | :white_check_mark: | [routes-segments-snake-case](./docs/rules/routes-segments-snake-case.md) | Enforces usage of snake_cased dynamic segments in routes | -### Ember Object - -| | Rule ID | Description | -|:---|:--------|:------------| -| :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 | -| :white_check_mark: | [avoid-leaking-callbacks-in-ember-objects](./docs/rules/avoid-leaking-callbacks-in-ember-objects.md) | Avoids callback memory leaks | - - ### Testing | | Rule ID | Description | diff --git a/lib/recommended-rules.js b/lib/recommended-rules.js index e8063cfe6c..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", @@ -17,7 +18,6 @@ module.exports = { "ember/new-module-imports": "error", "ember/no-attrs-in-components": "error", "ember/no-attrs-snapshot": "error", - "ember/no-callback-leaks-in-ember-objects": "error", "ember/no-capital-letters-in-routes": "error", "ember/no-deeply-nested-dependent-keys-with-each": "off", "ember/no-duplicate-dependent-keys": "error", From 08625d0ca9c32dc3fa8f5123e3155e52b93748f6 Mon Sep 17 00:00:00 2001 From: Rajasegar Chandran Date: Tue, 4 Jun 2019 12:55:15 +0530 Subject: [PATCH 8/9] [CHORE] Addressing review comments - phase 2 1. Restoing Ember Object section in Readme 2. Rearranging the new rule in lib/index based on alphabetical order 3. Removing commented code in the rule 4. Removing the recommended flag from the rule 5. Removing the rule from snapshot recommended 6. Removing the duplicate valid test case with Ember.Component.extend --- README.md | 18 +++++++++--------- lib/index.js | 2 +- ...avoid-leaking-callbacks-in-ember-objects.js | 2 -- tests/__snapshots__/recommended.js.snap | 1 - ...avoid-leaking-callbacks-in-ember-objects.js | 16 ---------------- 5 files changed, 10 insertions(+), 29 deletions(-) diff --git a/README.md b/README.md index 47009b883b..01b4c8f3e9 100644 --- a/README.md +++ b/README.md @@ -104,15 +104,6 @@ The `--fix` option on the command line automatically fixes problems reported by | :wrench: | [use-ember-get-and-set](./docs/rules/use-ember-get-and-set.md) | Enforces usage of Ember.get and Ember.set | -### Ember Object - -| | Rule ID | Description | -|:---|:--------|:------------| -| :white_check_mark: | [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 | - - ### Possible Errors | | Rule ID | Description | @@ -132,6 +123,15 @@ The `--fix` option on the command line automatically fixes problems reported by | :white_check_mark: | [routes-segments-snake-case](./docs/rules/routes-segments-snake-case.md) | Enforces usage of snake_cased dynamic segments in routes | +### Ember Object + +| | Rule ID | Description | +|:---|:--------|:------------| +| :white_check_mark: | [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 | + + ### Testing | | Rule ID | Description | diff --git a/lib/index.js b/lib/index.js index 3f45450ddf..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'), @@ -47,7 +48,6 @@ module.exports = { 'routes-segments-snake-case': require('./rules/routes-segments-snake-case'), 'use-brace-expansion': require('./rules/use-brace-expansion'), 'use-ember-get-and-set': require('./rules/use-ember-get-and-set'), - 'avoid-leaking-callbacks-in-ember-objects': require('./rules/avoid-leaking-callbacks-in-ember-objects'), }, configs: { base: require('./config/base'), diff --git a/lib/rules/avoid-leaking-callbacks-in-ember-objects.js b/lib/rules/avoid-leaking-callbacks-in-ember-objects.js index 9e894be8c9..3c1a6e414a 100644 --- a/lib/rules/avoid-leaking-callbacks-in-ember-objects.js +++ b/lib/rules/avoid-leaking-callbacks-in-ember-objects.js @@ -1,6 +1,5 @@ 'use strict'; -// const ember = require('../utils/ember'); const utils = require('../utils/utils'); const ERROR_MESSAGE = 'All `addEventListener` calls should have a corresponding `removeEventListener` to avoid leaking callbacks.'; @@ -15,7 +14,6 @@ module.exports = { docs: { description: 'Avoids callback memory leaks', category: 'Ember Object', - recommended: true, url: 'https://github.com/ember-best-practices/memory-leak-examples/blob/master/exercises/exercise-2.md' }, fixable: null, // or "code" or "whitespace" diff --git a/tests/__snapshots__/recommended.js.snap b/tests/__snapshots__/recommended.js.snap index 8f5922fd2e..4e0a2655a4 100644 --- a/tests/__snapshots__/recommended.js.snap +++ b/tests/__snapshots__/recommended.js.snap @@ -22,6 +22,5 @@ Array [ "require-super-in-init", "routes-segments-snake-case", "use-brace-expansion", - "avoid-leaking-callbacks-in-ember-objects", ] `; diff --git a/tests/lib/rules/avoid-leaking-callbacks-in-ember-objects.js b/tests/lib/rules/avoid-leaking-callbacks-in-ember-objects.js index c89b1b833f..41af8accda 100644 --- a/tests/lib/rules/avoid-leaking-callbacks-in-ember-objects.js +++ b/tests/lib/rules/avoid-leaking-callbacks-in-ember-objects.js @@ -21,22 +21,6 @@ eslintTester.run('avoid-leaking-callbacks-in-ember-objects', rule, { valid: [ { code: ` -export default Ember.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({ From eb30a54f28c9550fca414d222e6c22620edbe83b Mon Sep 17 00:00:00 2001 From: Rajasegar Chandran Date: Thu, 6 Jun 2019 14:16:59 +0530 Subject: [PATCH 9/9] [CHORE] Review comments - phase 3 1. Removing white checkmark for no callback leaks rules in README --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 01b4c8f3e9..b4a0fe49ab 100644 --- a/README.md +++ b/README.md @@ -127,7 +127,7 @@ The `--fix` option on the command line automatically fixes problems reported by | | Rule ID | Description | |:---|:--------|:------------| -| :white_check_mark: | [avoid-leaking-callbacks-in-ember-objects](./docs/rules/avoid-leaking-callbacks-in-ember-objects.md) | Avoids callback memory leaks | +| | [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 |