Skip to content
This repository was archived by the owner on Feb 9, 2020. It is now read-only.

Commit 0dd8b9c

Browse files
gary-bSomeKittens
authored andcommitted
fix(background.js) destroy child scopes
Destroyed scopes were not being removed from the scope tree. The background.js and inspectedApp code that handled the scope:destroy message was buggy, referencing undefined variables and also not tracking the descendants of the destroyed scopes. This code fixes both issues. All descendants are now present on the scope:destroy message broadcast throughout batarang. This should help clean up further tech debt in future.
1 parent 2f2e575 commit 0dd8b9c

File tree

3 files changed

+50
-7
lines changed

3 files changed

+50
-7
lines changed

background.js

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ function bufferOrForward(message, sender) {
1717
}
1818

1919
if (message !== 'refresh') {
20-
setIsHintFlag(message);
20+
addMessageHelperProperties(tabId, message);
2121
bufferData(tabId, message);
2222
}
2323
if (devToolsPort) {
@@ -32,14 +32,29 @@ function resetState(tabId) {
3232
};
3333
}
3434

35-
function setIsHintFlag(message) {
35+
function addMessageHelperProperties(tabId, message) {
36+
var scopes = data[tabId].scopes;
3637
var hintables = [
3738
'Controllers',
3839
'general',
3940
'Modules',
4041
'Events'
4142
];
4243
message.isHint = (hintables.indexOf(message.module) > -1);
44+
45+
if(message.event === 'scope:destroy'){
46+
message.data.subTree = getSubTree(scopes, message.data.id);
47+
}
48+
}
49+
50+
function getSubTree(scopes, id){
51+
var subTree = [id], scope;
52+
for(var i = 0; i < subTree.length; i++) {
53+
if(scope = scopes[subTree[i]]) {
54+
subTree.push.apply(subTree, scope.children);
55+
}
56+
}
57+
return subTree;
4358
}
4459

4560
function bufferData(tabId, message) {
@@ -62,9 +77,12 @@ function bufferData(tabId, message) {
6277
} else if (message.data.id && (scope = tabData.scopes[message.data.id])) {
6378
if (message.event === 'scope:destroy') {
6479
if (scope.parent) {
65-
scope.parent.children.splice(scope.parent.children.indexOf(child), 1);
80+
var parentScope = tabData.scopes[scope.parent];
81+
parentScope.children.splice(parentScope.children.indexOf(message.data.id), 1);
82+
}
83+
for(var i = 0; i < message.data.subTree.length; i++){
84+
delete tabData.scopes[message.data.subTree[i]];
6685
}
67-
delete scopes[message.data.id];
6886
} else if (message.event === 'model:change') {
6987
scope.models[message.data.path] = (typeof message.data.value === 'undefined') ?
7088
undefined : message.data.value;

panel/components/inspected-app/inspected-app.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,9 +92,12 @@ function inspectedAppService($rootScope, $q) {
9292
var scope = scopes[message.data.id];
9393
if (message.event === 'scope:destroy') {
9494
if (scope.parent) {
95-
scope.parent.children.splice(scope.parent.children.indexOf(child), 1);
95+
var parentScope = scopes[scope.parent];
96+
parentScope.children.splice(parentScope.children.indexOf(message.data.id), 1);
97+
}
98+
for(var i = 0; i < message.data.subTree.length; i++){
99+
delete scopes[message.data.subTree[i]];
96100
}
97-
delete scopes[message.data.id];
98101
} else if (message.event === 'model:change') {
99102
scope.models[message.data.path] = (typeof message.data.value === 'undefined') ?
100103
undefined : JSON.parse(message.data.value);

panel/components/inspected-app/inspected-app.spec.js

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,15 +68,37 @@ describe('inspectedApp', function() {
6868
expect(inspectedApp.scopes[id]).toEqual({ parent: parentId, children: [], models: {} });
6969
});
7070

71+
it('should track destruction of scopes', function () {
72+
var id = 2, parentId = 1;
73+
inspectedApp.scopes[id] = { parent: parentId };
74+
inspectedApp.scopes[parentId] = {
75+
children: [ 99, id, 100 ]
76+
};
77+
triggerAndFlush({ event: 'scope:destroy', data: { id: id, subTree: [id] } });
78+
79+
expect(inspectedApp.scopes.hasOwnProperty(id)).toBeFalsy();
80+
expect(inspectedApp.scopes[parentId].children).toEqual([99, 100]);
81+
});
82+
7183
it('should track destruction of scopes without throwing error when parent scope not present', function () {
7284
var id = 1;
7385
inspectedApp.scopes[id] = true;
74-
port.onMessage.trigger({ event: 'scope:destroy', data: { id: id } });
86+
port.onMessage.trigger({ event: 'scope:destroy', data: { id: id, subTree: [id] } });
7587

7688
expect($browser.defer.flush).not.toThrow();
7789
expect(inspectedApp.scopes.hasOwnProperty(id)).toBeFalsy();
7890
});
7991

92+
it('should delete subtree of destroyed scope', function () {
93+
inspectedApp.scopes[1] = true;
94+
inspectedApp.scopes[2] = true;
95+
inspectedApp.scopes[3] = true;
96+
inspectedApp.scopes[4] = true;
97+
98+
triggerAndFlush({ event: 'scope:destroy', data: { id: 1, subTree: [1, 3, 4] } });
99+
expect(inspectedApp.scopes).toEqual({2: true});
100+
});
101+
80102
it('should track model changes', function () {
81103
var id = 1;
82104
inspectedApp.scopes[id] = { models: {} };

0 commit comments

Comments
 (0)