Skip to content

Commit 6ce4766

Browse files
authored
Merge pull request #28 from chadhietala/add-destructuring
Adding generic way to detect destructured Ember object
2 parents 4a371b2 + 08c09c5 commit 6ce4766

File tree

5 files changed

+71
-14
lines changed

5 files changed

+71
-14
lines changed

lib/rules/no-lifecycle-events.js

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,24 @@ function isEmber(node) {
88
return node.name === 'Ember';
99
}
1010

11+
const { collectObjectPatternBindings } = require('../utils/destructed-binding');
12+
const { getEmberImportBinding } = require('../utils/imports');
13+
1114
module.exports = {
1215
meta: {
1316
message: MESSAGE
1417
},
1518
create(context) {
19+
var emberImportBinding;
20+
let destructedBindings = [];
1621
return {
22+
ObjectPattern(node) {
23+
if (emberImportBinding) {
24+
destructedBindings = destructedBindings.concat(collectObjectPatternBindings(node, {
25+
[emberImportBinding]: ['on']
26+
}));
27+
}
28+
},
1729

1830
ImportDeclaration(node) {
1931
if (node.source.value === '@ember/object/evented') {
@@ -25,6 +37,10 @@ module.exports = {
2537
}
2638
},
2739

40+
ImportDefaultSpecifier(node) {
41+
emberImportBinding = getEmberImportBinding(node);
42+
},
43+
2844
FunctionExpression(node) {
2945
if (node.parent.property && isOn(node.parent.property)) {
3046
context.report(node, MESSAGE);
@@ -39,15 +55,9 @@ module.exports = {
3955

4056
CallExpression(node) {
4157
if (isOn(node.callee)) {
42-
context.getScope().variables.forEach((v) => {
43-
if (isOn(v)) {
44-
v.defs.forEach((defNode) => {
45-
if (defNode.node.type === 'VariableDeclarator' && isEmber(defNode.node.init)) {
46-
context.report(node, MESSAGE);
47-
}
48-
});
49-
}
50-
});
58+
if (destructedBindings.includes(node.callee.name)) {
59+
context.report(node, MESSAGE);
60+
}
5161
}
5262
}
5363
};

lib/rules/no-side-effect-cp.js

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
const { getCaller, cleanCaller } = require('../utils/caller');
22
const isCPGetter = require('../utils/computed-property');
3-
const collectImportBindings = require('../utils/imports');
3+
const { getEmberImportBinding, collectImportBindings } = require('../utils/imports');
4+
const { collectObjectPatternBindings } = require('../utils/destructed-binding');
45

5-
const SIDE_EFFECTS = ['this.send', 'this.sendAction', 'this.sendEvent', 'Em.sendEvent', 'Ember.sendEvent', 'this.trigger', 'this.set', 'Ember.set', 'Em.set', 'this.setProperties', 'Ember.setProperties', 'Em.setProperties'];
6+
let SIDE_EFFECTS = ['this.send', 'this.sendAction', 'this.sendEvent', 'Em.sendEvent', 'Ember.sendEvent', 'this.trigger', 'this.set', 'Ember.set', 'Em.set', 'this.setProperties', 'Ember.setProperties', 'Em.setProperties'];
67

78
const MESSAGE = 'Do not send events or actions in Computed Properties. This will cause data flow issues in the application, where the accessing of a property causes some side-effect. You should only send actions on behalf of user initiated events. Please see the following guide for more infromation: https://github.com/chadhietala/ember-best-practices/blob/master/guides/rules/no-action-cp.md';
89

@@ -12,8 +13,7 @@ module.exports = {
1213
},
1314
create(context) {
1415
let inCPGettter = false;
15-
let bindings;
16-
16+
let bindings, emberImportBinding;
1717
return {
1818
ImportDeclaration(node) {
1919
bindings = collectImportBindings(node, {
@@ -22,6 +22,18 @@ module.exports = {
2222
});
2323
},
2424

25+
ObjectPattern(node) {
26+
if (emberImportBinding) {
27+
SIDE_EFFECTS = SIDE_EFFECTS.concat(collectObjectPatternBindings(node, {
28+
[emberImportBinding]: ['set', 'setProperties', 'sendEvent']
29+
}));
30+
}
31+
},
32+
33+
ImportDefaultSpecifier(node) {
34+
emberImportBinding = getEmberImportBinding(node);
35+
},
36+
2537
FunctionExpression(node) {
2638
if (isCPGetter(node)) {
2739
inCPGettter = true;

lib/utils/destructed-binding.js

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
const { get } = require('./get');
2+
3+
function collectObjectPatternBindings(node, initialObjToBinding) {
4+
let identifiers = Object.keys(initialObjToBinding);
5+
let objBindingName = get(node, 'parent.init.name');
6+
let bindingIndex = identifiers.indexOf(objBindingName);
7+
if (bindingIndex > -1) {
8+
let binding = identifiers[bindingIndex];
9+
return node.properties.filter(({ key }) => {
10+
return initialObjToBinding[binding].includes(key.name);
11+
}).map(({ value }) => {
12+
return value.name;
13+
});
14+
}
15+
16+
return [];
17+
}
18+
19+
module.exports = {
20+
collectObjectPatternBindings
21+
};

lib/utils/imports.js

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
module.exports = function collectImportBindings(node, imports) {
1+
const { get } = require('./get');
2+
3+
function collectImportBindings(node, imports) {
24
let sources = Object.keys(imports);
35
let sourceName = sources[sources.indexOf(node.source.value)];
46
let importedBindings = imports[sourceName];
@@ -10,4 +12,15 @@ module.exports = function collectImportBindings(node, imports) {
1012
}
1113

1214
return [];
15+
}
16+
17+
function getEmberImportBinding(node) {
18+
if (get(node, 'parent.source.raw') === '\'ember\'') {
19+
return node.local.name;
20+
}
21+
}
22+
23+
module.exports = {
24+
collectImportBindings,
25+
getEmberImportBinding
1326
};

tests/lib/rules/no-lifecycle-events.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ ruleTester.run('no-lifecycle-events', rule, {
4040
},
4141
{
4242
code: `
43+
import Ember from 'ember';
4344
const { on } = Ember;
4445
export default Ember.Component({
4546
registerFocus: on('didInsertElement', function() {

0 commit comments

Comments
 (0)