From e59d146f5394d81be5426ab940b9b332dc9e2b79 Mon Sep 17 00:00:00 2001 From: Russell Date: Fri, 24 Nov 2017 15:28:26 -0800 Subject: [PATCH] Bug 1419369 - NetMonitor: Stop using ImmutableJS in ui reducer; r=Honza MozReview-Commit-ID: IxxOmakX4Fr --HG-- extra : rebase_source : c39fa317d25d137ad756216d5015e9739b568f01 --- .../src/components/RequestListHeader.js | 2 +- .../src/components/RequestListItem.js | 45 +++++----- .../client/netmonitor/src/middleware/prefs.js | 10 ++- devtools/client/netmonitor/src/reducers/ui.js | 82 ++++++++++++------- .../src/request-list-header-context-menu.js | 11 ++- .../netmonitor/src/utils/create-store.js | 13 ++- .../test/browser_net_columns_last_column.js | 14 +++- .../test/browser_net_columns_pref.js | 20 +++-- .../test/browser_net_columns_reset.js | 2 +- .../test/browser_net_columns_showhide.js | 10 ++- 10 files changed, 131 insertions(+), 78 deletions(-) diff --git a/devtools/client/netmonitor/src/components/RequestListHeader.js b/devtools/client/netmonitor/src/components/RequestListHeader.js index abf292ad982d..084c0cc74f58 100644 --- a/devtools/client/netmonitor/src/components/RequestListHeader.js +++ b/devtools/client/netmonitor/src/components/RequestListHeader.js @@ -107,7 +107,7 @@ class RequestListHeader extends Component { className: "devtools-toolbar requests-list-headers", onContextMenu: this.onContextMenu }, - HEADERS.filter((header) => columns.get(header.name)).map((header) => { + HEADERS.filter((header) => columns[header.name]).map((header) => { let name = header.name; let boxName = header.boxName || name; let label = header.noLocalization diff --git a/devtools/client/netmonitor/src/components/RequestListItem.js b/devtools/client/netmonitor/src/components/RequestListItem.js index 59c666f6ff14..f10e48e3bf79 100644 --- a/devtools/client/netmonitor/src/components/RequestListItem.js +++ b/devtools/client/netmonitor/src/components/RequestListItem.js @@ -7,7 +7,6 @@ const { Component, createFactory } = require("devtools/client/shared/vendor/react"); const dom = require("devtools/client/shared/vendor/react-dom-factories"); const PropTypes = require("devtools/client/shared/vendor/react-prop-types"); -const I = require("devtools/client/shared/vendor/immutable"); const { propertiesEqual } = require("../utils/request-utils"); const { RESPONSE_HEADERS } = require("../constants"); @@ -100,7 +99,7 @@ class RequestListItem extends Component { shouldComponentUpdate(nextProps) { return !propertiesEqual(UPDATED_REQ_ITEM_PROPS, this.props.item, nextProps.item) || !propertiesEqual(UPDATED_REQ_PROPS, this.props, nextProps) || - !I.is(this.props.columns, nextProps.columns); + this.props.columns !== nextProps.columns; } componentDidUpdate(prevProps) { @@ -141,32 +140,32 @@ class RequestListItem extends Component { onContextMenu, onMouseDown, }, - columns.get("status") && RequestListColumnStatus({ item }), - columns.get("method") && RequestListColumnMethod({ item }), - columns.get("file") && RequestListColumnFile({ item }), - columns.get("protocol") && RequestListColumnProtocol({ item }), - columns.get("scheme") && RequestListColumnScheme({ item }), - columns.get("domain") && RequestListColumnDomain({ item, - onSecurityIconMouseDown }), - columns.get("remoteip") && RequestListColumnRemoteIP({ item }), - columns.get("cause") && RequestListColumnCause({ item, onCauseBadgeMouseDown }), - columns.get("type") && RequestListColumnType({ item }), - columns.get("cookies") && RequestListColumnCookies({ connector, item }), - columns.get("setCookies") && RequestListColumnSetCookies({ connector, item }), - columns.get("transferred") && RequestListColumnTransferredSize({ item }), - columns.get("contentSize") && RequestListColumnContentSize({ item }), - columns.get("startTime") && + columns.status && RequestListColumnStatus({ item }), + columns.method && RequestListColumnMethod({ item }), + columns.file && RequestListColumnFile({ item }), + columns.protocol && RequestListColumnProtocol({ item }), + columns.scheme && RequestListColumnScheme({ item }), + columns.domain && RequestListColumnDomain({ item, + onSecurityIconMouseDown }), + columns.remoteip && RequestListColumnRemoteIP({ item }), + columns.cause && RequestListColumnCause({ item, onCauseBadgeMouseDown }), + columns.type && RequestListColumnType({ item }), + columns.cookies && RequestListColumnCookies({ connector, item }), + columns.setCookies && RequestListColumnSetCookies({ connector, item }), + columns.transferred && RequestListColumnTransferredSize({ item }), + columns.contentSize && RequestListColumnContentSize({ item }), + columns.startTime && RequestListColumnStartTime({ item, firstRequestStartedMillis }), - columns.get("endTime") && + columns.endTime && RequestListColumnEndTime({ item, firstRequestStartedMillis }), - columns.get("responseTime") && + columns.responseTime && RequestListColumnResponseTime({ item, firstRequestStartedMillis }), - columns.get("duration") && RequestListColumnDuration({ item }), - columns.get("latency") && RequestListColumnLatency({ item }), - ...RESPONSE_HEADERS.filter(header => columns.get(header)).map( + columns.duration && RequestListColumnDuration({ item }), + columns.latency && RequestListColumnLatency({ item }), + ...RESPONSE_HEADERS.filter(header => columns[header]).map( header => RequestListColumnResponseHeader({ item, header }), ), - columns.get("waterfall") && + columns.waterfall && RequestListColumnWaterfall({ item, firstRequestStartedMillis, onWaterfallMouseDown }), ) diff --git a/devtools/client/netmonitor/src/middleware/prefs.js b/devtools/client/netmonitor/src/middleware/prefs.js index d2e5f9115674..99039b085ea9 100644 --- a/devtools/client/netmonitor/src/middleware/prefs.js +++ b/devtools/client/netmonitor/src/middleware/prefs.js @@ -42,9 +42,13 @@ function prefsMiddleware(store) { break; case TOGGLE_COLUMN: case RESET_COLUMNS: - let visibleColumns = [...store.getState().ui.columns] - .filter(([column, shown]) => shown) - .map(([column, shown]) => column); + let visibleColumns = []; + let columns = store.getState().ui.columns; + for (let column in columns) { + if (columns[column]) { + visibleColumns.push(column); + } + } Services.prefs.setCharPref( "devtools.netmonitor.visibleColumns", JSON.stringify(visibleColumns)); break; diff --git a/devtools/client/netmonitor/src/reducers/ui.js b/devtools/client/netmonitor/src/reducers/ui.js index 5936034966de..8797f1e59261 100644 --- a/devtools/client/netmonitor/src/reducers/ui.js +++ b/devtools/client/netmonitor/src/reducers/ui.js @@ -4,7 +4,6 @@ "use strict"; -const I = require("devtools/client/shared/vendor/immutable"); const Services = require("Services"); const { CLEAR_REQUESTS, @@ -44,65 +43,92 @@ const cols = { latency: false, waterfall: true, }; -const Columns = I.Record( - Object.assign( +function Columns() { + return Object.assign( cols, RESPONSE_HEADERS.reduce((acc, header) => Object.assign(acc, { [header]: false }), {}) - ) -); + ); +} -const UI = I.Record({ - columns: new Columns(), - detailsPanelSelectedTab: PANELS.HEADERS, - networkDetailsOpen: false, - persistentLogsEnabled: Services.prefs.getBoolPref("devtools.netmonitor.persistlog"), - browserCacheDisabled: Services.prefs.getBoolPref("devtools.cache.disabled"), - statisticsOpen: false, - waterfallWidth: null, -}); +function UI(initialState = {}) { + return { + columns: Columns(), + detailsPanelSelectedTab: PANELS.HEADERS, + networkDetailsOpen: false, + persistentLogsEnabled: Services.prefs.getBoolPref("devtools.netmonitor.persistlog"), + browserCacheDisabled: Services.prefs.getBoolPref("devtools.cache.disabled"), + statisticsOpen: false, + waterfallWidth: null, + ...initialState, + }; +} function resetColumns(state) { - return state.set("columns", new Columns()); + return { + ...state, + columns: Columns() + }; } function resizeWaterfall(state, action) { - return state.set("waterfallWidth", action.width); + return { + ...state, + waterfallWidth: action.width + }; } function openNetworkDetails(state, action) { - return state.set("networkDetailsOpen", action.open); + return { + ...state, + networkDetailsOpen: action.open + }; } function enablePersistentLogs(state, action) { - return state.set("persistentLogsEnabled", action.enabled); + return { + ...state, + persistentLogsEnabled: action.enabled + }; } function disableBrowserCache(state, action) { - return state.set("browserCacheDisabled", action.disabled); + return { + ...state, + browserCacheDisabled: action.disabled + }; } function openStatistics(state, action) { - return state.set("statisticsOpen", action.open); + return { + ...state, + statisticsOpen: action.open + }; } function setDetailsPanelTab(state, action) { - return state.set("detailsPanelSelectedTab", action.id); + return { + ...state, + detailsPanelSelectedTab: action.id + }; } function toggleColumn(state, action) { let { column } = action; - if (!state.has(column)) { + if (!state.columns.hasOwnProperty(column)) { return state; } - let newState = state.withMutations(columns => { - columns.set(column, !state.get(column)); - }); - return newState; + return { + ...state, + columns: { + ...state.columns, + [column]: !state.columns[column] + } + }; } -function ui(state = new UI(), action) { +function ui(state = UI(), action) { switch (action.type) { case CLEAR_REQUESTS: return openNetworkDetails(state, { open: false }); @@ -124,7 +150,7 @@ function ui(state = new UI(), action) { case SELECT_REQUEST: return openNetworkDetails(state, { open: true }); case TOGGLE_COLUMN: - return state.set("columns", toggleColumn(state.columns, action)); + return toggleColumn(state, action); case WATERFALL_RESIZE: return resizeWaterfall(state, action); default: diff --git a/devtools/client/netmonitor/src/request-list-header-context-menu.js b/devtools/client/netmonitor/src/request-list-header-context-menu.js index 1549d3bfe8ae..aa5c2de853eb 100644 --- a/devtools/client/netmonitor/src/request-list-header-context-menu.js +++ b/devtools/client/netmonitor/src/request-list-header-context-menu.js @@ -33,7 +33,13 @@ class RequestListHeaderContextMenu { } get visibleColumns() { - return [...this.columns].filter(([_, shown]) => shown); + let visible = []; + for (let column in this.columns) { + if (this.columns[column]) { + visible.push(column); + } + } + return visible; } /** @@ -44,7 +50,8 @@ class RequestListHeaderContextMenu { let subMenu = { timings: [], responseHeaders: [] }; let onlyOneColumn = this.visibleColumns.length === 1; - for (let [column, shown] of this.columns) { + for (let column in this.columns) { + let shown = this.columns[column]; let label = nonLocalizedHeaders.includes(column) ? stringMap[column] || column : L10N.getStr(`netmonitor.toolbar.${stringMap[column] || column}`); diff --git a/devtools/client/netmonitor/src/utils/create-store.js b/devtools/client/netmonitor/src/utils/create-store.js index 49fd73849522..af1941da1141 100644 --- a/devtools/client/netmonitor/src/utils/create-store.js +++ b/devtools/client/netmonitor/src/utils/create-store.js @@ -33,7 +33,7 @@ function configureStore(connector) { requests: new Requests(), sort: new Sort(), timingMarkers: new TimingMarkers(), - ui: new UI({ + ui: UI({ columns: getColumnState() }), }; @@ -55,16 +55,15 @@ function configureStore(connector) { * Get column state from preferences. */ function getColumnState() { - let columns = new Columns(); + let columns = Columns(); let visibleColumns = getPref("devtools.netmonitor.visibleColumns"); - for (let [col] of columns) { - columns = columns.withMutations((state) => { - state.set(col, visibleColumns.includes(col)); - }); + const state = {}; + for (let col in columns) { + state[col] = visibleColumns.includes(col); } - return columns; + return state; } /** diff --git a/devtools/client/netmonitor/test/browser_net_columns_last_column.js b/devtools/client/netmonitor/test/browser_net_columns_last_column.js index 890a00229913..28c0c28e4a6a 100644 --- a/devtools/client/netmonitor/test/browser_net_columns_last_column.js +++ b/devtools/client/netmonitor/test/browser_net_columns_last_column.js @@ -13,9 +13,17 @@ add_task(function* () { let { document, store, parent } = monitor.panelWin; - for (let [column, shown] of store.getState().ui.columns) { - let visibleColumns = [...store.getState().ui.columns] - .filter(([_, visible]) => visible); + let initialColumns = store.getState().ui.columns; + for (let column in initialColumns) { + let shown = initialColumns[column]; + + let columns = store.getState().ui.columns; + let visibleColumns = []; + for (let c in columns) { + if (columns[c]) { + visibleColumns.push(c); + } + } if (visibleColumns.length === 1) { if (!shown) { diff --git a/devtools/client/netmonitor/test/browser_net_columns_pref.js b/devtools/client/netmonitor/test/browser_net_columns_pref.js index 3e0e6149cab5..3b141f457eda 100644 --- a/devtools/client/netmonitor/test/browser_net_columns_pref.js +++ b/devtools/client/netmonitor/test/browser_net_columns_pref.js @@ -24,15 +24,23 @@ add_task(function* () { yield hideColumn("status"); yield hideColumn("contentSize"); - ok(!Services.prefs.getCharPref("devtools.netmonitor.visibleColumns").includes("status"), - "Pref should be synced for status"); - ok(!Services.prefs.getCharPref("devtools.netmonitor.visibleColumns") - .includes("contentSize"), "Pref should be synced for contentSize"); + let visibleColumns = JSON.parse( + Services.prefs.getCharPref("devtools.netmonitor.visibleColumns") + ); + + ok(!visibleColumns.includes("status"), + "Pref should be synced for status"); + ok(!visibleColumns.includes("contentSize"), + "Pref should be synced for contentSize"); yield showColumn("status"); - ok(Services.prefs.getCharPref("devtools.netmonitor.visibleColumns").includes("status"), - "Pref should be synced for status"); + visibleColumns = JSON.parse( + Services.prefs.getCharPref("devtools.netmonitor.visibleColumns") + ); + + ok(visibleColumns.includes("status"), + "Pref should be synced for status"); function* hideColumn(column) { info(`Clicking context-menu item for ${column}`); diff --git a/devtools/client/netmonitor/test/browser_net_columns_reset.js b/devtools/client/netmonitor/test/browser_net_columns_reset.js index a4820819d563..5ba099bd32ab 100644 --- a/devtools/client/netmonitor/test/browser_net_columns_reset.js +++ b/devtools/client/netmonitor/test/browser_net_columns_reset.js @@ -24,7 +24,7 @@ add_task(function* () { parent.document.querySelector("#request-list-header-reset-columns").click(); - is(JSON.stringify(prefBefore), JSON.stringify(Prefs.visibleColumns), + ok(JSON.stringify(prefBefore) === JSON.stringify(Prefs.visibleColumns), "Reset columns item should reset columns pref"); function* hideColumn(column) { diff --git a/devtools/client/netmonitor/test/browser_net_columns_showhide.js b/devtools/client/netmonitor/test/browser_net_columns_showhide.js index 7df89e6ed8fe..97a6269312b1 100644 --- a/devtools/client/netmonitor/test/browser_net_columns_showhide.js +++ b/devtools/client/netmonitor/test/browser_net_columns_showhide.js @@ -13,8 +13,9 @@ add_task(function* () { let { document, store, parent } = monitor.panelWin; - for (let [column, shown] of store.getState().ui.columns) { - if (shown) { + let columns = store.getState().ui.columns; + for (let column in columns) { + if (columns[column]) { yield testVisibleColumnContextMenuItem(column, document, parent); yield testHiddenColumnContextMenuItem(column, document, parent); } else { @@ -23,8 +24,9 @@ add_task(function* () { } } - for (let [column, shown] of store.getState().ui.columns) { - if (shown) { + columns = store.getState().ui.columns; + for (let column in columns) { + if (columns[column]) { yield testVisibleColumnContextMenuItem(column, document, parent); // Right click on the white-space for the context menu to appear // and toggle column visibility