Bug 1239436 - Remove in-place mutation of trees' expanded sets in reducers. r=fitzgen

This commit is contained in:
Lin Clark 2016-03-16 13:19:00 +01:00
parent 37f4efccea
commit 367e617129
2 changed files with 11 additions and 30 deletions

View File

@ -3,6 +3,7 @@
* You can obtain one at http://mozilla.org/MPL/2.0/. */ * You can obtain one at http://mozilla.org/MPL/2.0/. */
"use strict"; "use strict";
const Immutable = require("devtools/client/shared/vendor/immutable");
const { immutableUpdate, assert } = require("devtools/shared/DevToolsUtils"); const { immutableUpdate, assert } = require("devtools/shared/DevToolsUtils");
const { actions, diffingState, viewState } = require("../constants"); const { actions, diffingState, viewState } = require("../constants");
const { snapshotIsDiffable } = require("../utils"); const { snapshotIsDiffable } = require("../utils");
@ -82,7 +83,7 @@ handlers[actions.TAKE_CENSUS_DIFF_END] = function (diffing, action) {
census: { census: {
report: action.report, report: action.report,
parentMap: action.parentMap, parentMap: action.parentMap,
expanded: new Set(), expanded: Immutable.Set(),
inverted: action.inverted, inverted: action.inverted,
filter: action.filter, filter: action.filter,
display: action.display, display: action.display,
@ -105,11 +106,7 @@ handlers[actions.EXPAND_DIFFING_CENSUS_NODE] = function (diffing, { node }) {
assert(diffing.census.report, "Should have a census report"); assert(diffing.census.report, "Should have a census report");
assert(diffing.census.expanded, "Should have a census's expanded set"); assert(diffing.census.expanded, "Should have a census's expanded set");
// Warning: mutable operations couched in an immutable update ahead :'( This const expanded = diffing.census.expanded.add(node.id);
// at least lets us use referential equality on the census model itself,
// even though the expanded set is mutated in place.
const expanded = diffing.census.expanded;
expanded.add(node.id);
const census = immutableUpdate(diffing.census, { expanded }); const census = immutableUpdate(diffing.census, { expanded });
return immutableUpdate(diffing, { census }); return immutableUpdate(diffing, { census });
}; };
@ -122,10 +119,7 @@ handlers[actions.COLLAPSE_DIFFING_CENSUS_NODE] = function (diffing, { node }) {
assert(diffing.census.report, "Should have a census report"); assert(diffing.census.report, "Should have a census report");
assert(diffing.census.expanded, "Should have a census's expanded set"); assert(diffing.census.expanded, "Should have a census's expanded set");
// Warning: mutable operations couched in an immutable update ahead :'( See const expanded = diffing.census.expanded.delete(node.id);
// above comment in the EXPAND_DIFFING_CENSUS_NODE handler.
const expanded = diffing.census.expanded;
expanded.delete(node.id);
const census = immutableUpdate(diffing.census, { expanded }); const census = immutableUpdate(diffing.census, { expanded });
return immutableUpdate(diffing, { census }); return immutableUpdate(diffing, { census });
}; };

View File

@ -3,6 +3,7 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
"use strict"; "use strict";
const Immutable = require("devtools/client/shared/vendor/immutable");
const { immutableUpdate, assert } = require("devtools/shared/DevToolsUtils"); const { immutableUpdate, assert } = require("devtools/shared/DevToolsUtils");
const { const {
actions, actions,
@ -74,7 +75,7 @@ handlers[actions.TAKE_CENSUS_END] = function (snapshots, { id,
const census = { const census = {
report, report,
parentMap, parentMap,
expanded: new Set(), expanded: Immutable.Set(),
display, display,
filter, filter,
}; };
@ -96,11 +97,7 @@ handlers[actions.EXPAND_CENSUS_NODE] = function (snapshots, { id, node }) {
assert(snapshot.census.report, "Should have a census report"); assert(snapshot.census.report, "Should have a census report");
assert(snapshot.census.expanded, "Should have a census's expanded set"); assert(snapshot.census.expanded, "Should have a census's expanded set");
// Warning: mutable operations couched in an immutable update ahead :'( This const expanded = snapshot.census.expanded.add(node.id);
// at least lets us use referential equality on the census model itself,
// even though the expanded set is mutated in place.
const expanded = snapshot.census.expanded;
expanded.add(node.id);
const census = immutableUpdate(snapshot.census, { expanded }); const census = immutableUpdate(snapshot.census, { expanded });
return immutableUpdate(snapshot, { census }); return immutableUpdate(snapshot, { census });
}); });
@ -116,10 +113,7 @@ handlers[actions.COLLAPSE_CENSUS_NODE] = function (snapshots, { id, node }) {
assert(snapshot.census.report, "Should have a census report"); assert(snapshot.census.report, "Should have a census report");
assert(snapshot.census.expanded, "Should have a census's expanded set"); assert(snapshot.census.expanded, "Should have a census's expanded set");
// Warning: mutable operations couched in an immutable update ahead :'( See const expanded = snapshot.census.expanded.delete(node.id);
// above comment in the EXPAND_CENSUS_NODE handler.
const expanded = snapshot.census.expanded;
expanded.delete(node.id);
const census = immutableUpdate(snapshot.census, { expanded }); const census = immutableUpdate(snapshot.census, { expanded });
return immutableUpdate(snapshot, { census }); return immutableUpdate(snapshot, { census });
}); });
@ -224,7 +218,7 @@ handlers[actions.FETCH_DOMINATOR_TREE_END] = function (snapshots, { id, root })
const dominatorTree = immutableUpdate(snapshot.dominatorTree, { const dominatorTree = immutableUpdate(snapshot.dominatorTree, {
state: dominatorTreeState.LOADED, state: dominatorTreeState.LOADED,
root, root,
expanded: new Set(), expanded: Immutable.Set(),
}); });
return immutableUpdate(snapshot, { dominatorTree }); return immutableUpdate(snapshot, { dominatorTree });
@ -241,11 +235,7 @@ handlers[actions.EXPAND_DOMINATOR_TREE_NODE] = function (snapshots, { id, node }
assert(snapshot.dominatorTree.expanded, assert(snapshot.dominatorTree.expanded,
"Should have the dominator tree's expanded set"); "Should have the dominator tree's expanded set");
// Warning: mutable operations couched in an immutable update ahead :'( This const expanded = snapshot.dominatorTree.expanded.add(node.nodeId);
// at least lets us use referential equality on the dominatorTree model itself,
// even though the expanded set is mutated in place.
const expanded = snapshot.dominatorTree.expanded;
expanded.add(node.nodeId);
const dominatorTree = immutableUpdate(snapshot.dominatorTree, { expanded }); const dominatorTree = immutableUpdate(snapshot.dominatorTree, { expanded });
return immutableUpdate(snapshot, { dominatorTree }); return immutableUpdate(snapshot, { dominatorTree });
}); });
@ -261,10 +251,7 @@ handlers[actions.COLLAPSE_DOMINATOR_TREE_NODE] = function (snapshots, { id, node
assert(snapshot.dominatorTree.expanded, assert(snapshot.dominatorTree.expanded,
"Should have the dominator tree's expanded set"); "Should have the dominator tree's expanded set");
// Warning: mutable operations couched in an immutable update ahead :'( See const expanded = snapshot.dominatorTree.expanded.delete(node.nodeId);
// above comment in the EXPAND_DOMINATOR_TREE_NODE handler.
const expanded = snapshot.dominatorTree.expanded;
expanded.delete(node.nodeId);
const dominatorTree = immutableUpdate(snapshot.dominatorTree, { expanded }); const dominatorTree = immutableUpdate(snapshot.dominatorTree, { expanded });
return immutableUpdate(snapshot, { dominatorTree }); return immutableUpdate(snapshot, { dominatorTree });
}); });