refactor: accumulator spread in reduce (#37435)

n3ps · web-flow · commit 28ff526b7625 · 2025-11-03T23:03:43.000Z
## **Description** Improve readability and performance Spreading the accumulator can generally be [slower](https://www.richsnapp.com/article/2019/06-09-reduce-spread-anti-pattern) than a simple for loop, not to mention less readable [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/37435?quickstart=1) ## **Changelog**  CHANGELOG entry: null ## **Related issues** Fixes: ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings**  ### **Before**  ### **After**  ## **Pre-merge author checklist** - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.  --- > [!NOTE] > Refactors multiple reducers using spread into explicit loops/Object.assign for building maps, arrays, and initial state across controllers, selectors, UI, tests, and constants. > > - **Core/Controllers** > - `app/scripts/constants/sentry-state.ts`: Replace `reduce` + spread with loop and `Object.assign` to build `flattenedBackgroundStateMask`. > - `app/scripts/controller-init/assets/network-enablement-controller-init.ts`: Build `allEnabledNetworks` via loop and `Object.assign`. > - `app/scripts/controllers/metametrics-controller.ts`: Rework `#buildValidTraits` to use `for...of` accumulation; preserve validation and return shape. > - `app/scripts/lib/ComposableObservableStore.js`: Use `Object.assign` when merging child states in `getFlatState`. > - **Shared/Constants** > - `shared/constants/tokens.js`: Build `STATIC_MAINNET_TOKEN_LIST` via loop; switch to named export after declaration. > - **Selectors** > - `shared/modules/selectors/networks.ts`: Build `clientIdsByChain` via loop; in `getNetworksByScopes`, use `concat` instead of array spread. > - `ui/selectors/multichain/networks.ts`: Construct EVM multichain config map via loop. > - `ui/selectors/selectors.js`: Mutate accumulator object in accounts reduce instead of returning new object each iteration. > - **UI Components** > - `ui/components/app/contact-list/contact-list.component.js`: Group contacts by letter using explicit loops and pre-allocated buckets. > - `ui/components/multichain/.../asset-picker-modal-network.tsx`: Initialize and update `checkedChainIds` via loops; rework "select all" toggle to build a new map. > - **Tests** > - `test/e2e/fixture-builder.js`: Build `optionalScopes` for multichain test dapp using a loop. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 41a2b7f. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup>
diff --git a/app/scripts/constants/sentry-state.ts b/app/scripts/constants/sentry-state.ts
@@ -396,14 +396,11 @@ export const SENTRY_BACKGROUND_STATE = {
   },
 };
 
-const flattenedBackgroundStateMask = Object.values(
-  SENTRY_BACKGROUND_STATE,
-).reduce((partialBackgroundState, controllerState: object) => {
-  return {
-    ...partialBackgroundState,
-    ...controllerState,
-  };
-}, {});
+const flattenedBackgroundStateMask: Record<string, unknown> = {};
+
+for (const controllerState of Object.values(SENTRY_BACKGROUND_STATE)) {
+  Object.assign(flattenedBackgroundStateMask, controllerState);
+}
 
 // This describes the subset of Redux state attached to errors sent to Sentry
 // These properties have some potential to be useful for debugging, and they do
diff --git a/app/scripts/controller-init/assets/network-enablement-controller-init.ts b/app/scripts/controller-init/assets/network-enablement-controller-init.ts
@@ -187,11 +187,11 @@ export const NetworkEnablementControllerInit: ControllerInitFunction<
       );
       ///: END:ONLY_INCLUDE_IF
 
-      const allEnabledNetworks = Object.values(
-        controller.state.enabledNetworkMap,
-      ).reduce((acc, curr) => {
-        return { ...acc, ...curr };
-      }, {});
+      const allEnabledNetworks = {};
+
+      for (const network of Object.values(controller.state.enabledNetworkMap)) {
+        Object.assign(allEnabledNetworks, network);
+      }
 
       if (Object.keys(allEnabledNetworks).length === 1) {
         const chainId = Object.keys(allEnabledNetworks)[0];
diff --git a/app/scripts/controllers/metametrics-controller.ts b/app/scripts/controllers/metametrics-controller.ts
@@ -1471,27 +1471,21 @@ export default class MetaMetricsController extends BaseController<
   #buildValidTraits(
     userTraits: Partial<MetaMetricsUserTraits>,
   ): MetaMetricsUserTraits {
-    return Object.entries(userTraits).reduce(
-      (validTraits: MetaMetricsUserTraits, [key, value]) => {
-        if (this.#isValidTraitDate(value)) {
-          return {
-            ...validTraits,
-            [key]: value.toISOString(),
-          };
-        } else if (this.#isValidTrait(value)) {
-          return {
-            ...validTraits,
-            [key]: value,
-          };
-        }
-
+    const validTraits: Record<string, string> = {};
+
+    for (const [key, value] of Object.entries(userTraits)) {
+      if (this.#isValidTraitDate(value)) {
+        validTraits[key] = value.toISOString();
+      } else if (this.#isValidTrait(value)) {
+        (validTraits as Record<string, typeof value>)[key] = value;
+      } else {
         console.warn(
           `MetaMetricsController: "${key}" value is not a valid trait type`,
         );
-        return validTraits;
-      },
-      {},
-    );
+      }
+    }
+
+    return validTraits;
   }
 
   /**
diff --git a/app/scripts/lib/ComposableObservableStore.js b/app/scripts/lib/ComposableObservableStore.js
@@ -107,13 +107,13 @@ export default class ComposableObservableStore extends ObservableStore {
     if (!this.config) {
       return {};
     }
-    let flatState = {};
+    const flatState = {};
     for (const key of Object.keys(this.config)) {
       const controller = this.config[key];
       const state = controller.getState
         ? controller.getState()
         : controller.state;
-      flatState = { ...flatState, ...state };
+      Object.assign(flatState, state);
     }
     return flatState;
   }
diff --git a/shared/constants/tokens.js b/shared/constants/tokens.js
@@ -25,21 +25,19 @@ export const LISTED_CONTRACT_ADDRESSES = Object.keys(contractMap).map(
  * @property {boolean} [isERC721] - True when the asset is a ERC721 token.
  */
 
-export const STATIC_MAINNET_TOKEN_LIST = Object.keys(contractMap).reduce(
-  (acc, base) => {
-    const { logo, ...tokenMetadata } = contractMap[base];
-    return {
-      ...acc,
-      [base.toLowerCase()]: {
-        ...tokenMetadata,
-        address: base.toLowerCase(),
-        iconUrl: `images/contract/${logo}`,
-        aggregators: [],
-      },
-    };
-  },
-  {},
-);
+const STATIC_MAINNET_TOKEN_LIST = {};
+
+for (const base of Object.keys(contractMap)) {
+  const { logo, ...tokenMetadata } = contractMap[base];
+  STATIC_MAINNET_TOKEN_LIST[base.toLowerCase()] = {
+    ...tokenMetadata,
+    address: base.toLowerCase(),
+    iconUrl: `images/contract/${logo}`,
+    aggregators: [],
+  };
+}
+
+export { STATIC_MAINNET_TOKEN_LIST };
 
 export const TOKEN_API_METASWAP_CODEFI_URL =
   'https://token.api.cx.metamask.io/tokens/';
diff --git a/shared/modules/selectors/networks.ts b/shared/modules/selectors/networks.ts
@@ -72,18 +72,18 @@ export const getNetworkConfigurationsByChainId = (
 export const selectDefaultNetworkClientIdsByChainId = createSelector(
   getNetworkConfigurationsByChainId,
   (networkConfigurationsByChainId) => {
-    return Object.entries(networkConfigurationsByChainId).reduce<
-      Record<Hex, NetworkClientId>
-    >(
-      (obj, [chainId, networkConfiguration]) => ({
-        ...obj,
-        [chainId]:
-          networkConfiguration.rpcEndpoints[
-            networkConfiguration.defaultRpcEndpointIndex
-          ].networkClientId,
-      }),
-      {},
-    );
+    const clientIdsByChain: Record<Hex, NetworkClientId> = {};
+
+    for (const [chainId, networkConfiguration] of Object.entries(
+      networkConfigurationsByChainId,
+    )) {
+      clientIdsByChain[chainId as Hex] =
+        networkConfiguration.rpcEndpoints[
+          networkConfiguration.defaultRpcEndpointIndex
+        ].networkClientId;
+    }
+
+    return clientIdsByChain;
   },
 );
 
@@ -366,21 +366,18 @@ export const getNetworksByScopes = createSelector(
               name: network.name,
             }));
 
-          return [...result, ...evmNetworks];
+          return result.concat(evmNetworks);
         }
 
         const matchingNetwork = nonTestNetworks.find(
           (network) => network.caipChainId === scope,
         );
 
         if (matchingNetwork) {
-          return [
-            ...result,
-            {
-              chainId: matchingNetwork.chainId,
-              name: matchingNetwork.name,
-            },
-          ];
+          return result.concat({
+            chainId: matchingNetwork.chainId,
+            name: matchingNetwork.name,
+          });
         }
 
         return result;
diff --git a/test/e2e/fixture-builder.js b/test/e2e/fixture-builder.js
@@ -586,18 +586,16 @@ class FixtureBuilder {
   withPermissionControllerConnectedToMultichainTestDappWithTwoAccounts({
     scopes = ['eip155:1337'],
   }) {
-    const optionalScopes = scopes
-      .map((scope) => ({
-        [scope]: {
-          accounts: [
-            `${scope}:0x5cfe73b6021e818b776b421b1c4db2474086a7e1`,
-            `${scope}:0x09781764c08de8ca82e156bbf156a3ca217c7950`,
-          ],
-        },
-      }))
-      .reduce((acc, curr) => {
-        return { ...acc, ...curr };
-      }, {});
+    const optionalScopes = {};
+
+    for (const scope of scopes) {
+      optionalScopes[scope] = {
+        accounts: [
+          `${scope}:0x5cfe73b6021e818b776b421b1c4db2474086a7e1`,
+          `${scope}:0x09781764c08de8ca82e156bbf156a3ca217c7950`,
+        ],
+      };
+    }
 
     const subjects = {
       [DAPP_URL]: {
diff --git a/ui/components/app/contact-list/contact-list.component.js b/ui/components/app/contact-list/contact-list.component.js
@@ -75,27 +75,24 @@ export default class ContactList extends PureComponent {
       internalAccounts,
     );
 
-    const unsortedContactsByLetter = searchForContacts().reduce(
-      (obj, contact) => {
-        const firstLetter = contact.name[0].toUpperCase();
-
-        const isDuplicate =
-          (duplicateContactMap.get(contact.name.trim().toLowerCase()) ?? [])
-            .length > 1;
-
-        return {
-          ...obj,
-          [firstLetter]: [
-            ...(obj[firstLetter] || []),
-            {
-              ...contact,
-              isDuplicate,
-            },
-          ],
-        };
-      },
-      {},
-    );
+    const unsortedContactsByLetter = {};
+
+    for (const contact of searchForContacts()) {
+      const firstLetter = contact.name[0].toUpperCase();
+
+      const isDuplicate =
+        (duplicateContactMap.get(contact.name.trim().toLowerCase()) ?? [])
+          .length > 1;
+
+      if (!unsortedContactsByLetter[firstLetter]) {
+        unsortedContactsByLetter[firstLetter] = [];
+      }
+
+      unsortedContactsByLetter[firstLetter].push({
+        ...contact,
+        isDuplicate,
+      });
+    }
 
     const letters = Object.keys(unsortedContactsByLetter).sort();
 
diff --git a/ui/components/multichain/asset-picker-amount/asset-picker-modal/asset-picker-modal-network.tsx b/ui/components/multichain/asset-picker-amount/asset-picker-modal/asset-picker-modal-network.tsx
@@ -124,32 +124,35 @@ export const AssetPickerModalNetwork = ({
   // Initialized with the selectedChainIds if provided
   const [checkedChainIds, setCheckedChainIds] = useState<
     Record<string, boolean>
-  >(
-    networksList?.reduce(
-      (acc, { chainId }) => ({
-        ...acc,
-        [chainId]: selectedChainIds
-          ? selectedChainIds.includes(chainId)
-          : false,
-      }),
-      {},
-    ) ?? {},
-  );
+  >(() => {
+    if (!networksList) {
+      return {};
+    }
+
+    const initialState: Record<string, boolean> = {};
+
+    for (const { chainId } of networksList) {
+      initialState[chainId] = selectedChainIds
+        ? selectedChainIds.includes(chainId)
+        : false;
+    }
+
+    return initialState;
+  });
 
   // Reset checkedChainIds if selectedChainIds change in parent component
   useEffect(() => {
-    networksList &&
-      setCheckedChainIds(
-        networksList.reduce(
-          (acc, { chainId }) => ({
-            ...acc,
-            [chainId]: selectedChainIds
-              ? selectedChainIds.includes(chainId)
-              : false,
-          }),
-          {},
-        ),
-      );
+    if (networksList) {
+      const updatedState: Record<string, boolean> = {};
+
+      for (const { chainId } of networksList) {
+        updatedState[chainId] = selectedChainIds
+          ? selectedChainIds.includes(chainId)
+          : false;
+      }
+
+      setCheckedChainIds(updatedState);
+    }
   }, [networksList, selectedChainIds]);
 
   const handleToggleNetwork = useCallback((chainId: string) => {
@@ -161,15 +164,14 @@ export const AssetPickerModalNetwork = ({
 
   // Toggles all networks to be checked or unchecked
   const handleToggleAllNetworks = useCallback(() => {
-    setCheckedChainIds(
-      Object.keys(checkedChainIds)?.reduce(
-        (agg, chainId) => ({
-          ...agg,
-          [chainId]: !Object.values(checkedChainIds).every((v) => v),
-        }),
-        {},
-      ),
-    );
+    const toggledState: Record<string, boolean> = {};
+    const allChecked = Object.values(checkedChainIds).every((v) => v);
+
+    for (const chainId of Object.keys(checkedChainIds)) {
+      toggledState[chainId] = !allChecked;
+    }
+
+    setCheckedChainIds(toggledState);
   }, [checkedChainIds]);
 
   return (
diff --git a/ui/selectors/multichain/networks.ts b/ui/selectors/multichain/networks.ts
@@ -198,18 +198,21 @@ export const getMultichainNetworkConfigurationsByChainId =
       // There's a fallback for EVM network names/nicknames, in case the network
       // does not have a name/nickname the fallback is the first rpc endpoint url.
       // TODO: Update toMultichainNetworkConfigurationsByChainId to handle this case.
-      const evmNetworks = Object.entries(networkConfigurationsByChainId).reduce(
-        (acc, [, network]) => ({
-          ...acc,
-          [toEvmCaipChainId(network.chainId)]: {
-            ...toMultichainNetworkConfiguration(network),
-            name:
-              network.name ||
-              network.rpcEndpoints[network.defaultRpcEndpointIndex].url,
-          },
-        }),
-        {},
-      );
+      const evmNetworks: Record<
+        CaipChainId,
+        InternalMultichainNetworkConfiguration
+      > = {};
+
+      for (const [, network] of Object.entries(
+        networkConfigurationsByChainId,
+      )) {
+        evmNetworks[toEvmCaipChainId(network.chainId)] = {
+          ...toMultichainNetworkConfiguration(network),
+          name:
+            network.name ||
+            network.rpcEndpoints[network.defaultRpcEndpointIndex].url,
+        };
+      }
 
       const networks = {
         ///: BEGIN:ONLY_INCLUDE_IF(multichain)
diff --git a/ui/selectors/selectors.js b/ui/selectors/selectors.js
@@ -497,10 +497,8 @@ export const getMetaMaskAccounts = createDeepEqualSelector(
         };
       }
 
-      return {
-        ...accounts,
-        [internalAccount.address]: account,
-      };
+      accounts[internalAccount.address] = account;
+      return accounts;
     }, {}),
 );
 /**

Original file line number	Diff line number	Diff line change
`@@ -107,13 +107,13 @@ export default class ComposableObservableStore extends ObservableStore {`
`107`	`107`	`if (!this.config) {`
`108`	`108`	`return {};`
`109`	`109`	`}`
`110`		`- let flatState = {};`
	`110`	`+ const flatState = {};`
`111`	`111`	`for (const key of Object.keys(this.config)) {`
`112`	`112`	`const controller = this.config[key];`
`113`	`113`	`const state = controller.getState`
`114`	`114`	`? controller.getState()`
`115`	`115`	`: controller.state;`
`116`		`- flatState = { ...flatState, ...state };`
	`116`	`+ Object.assign(flatState, state);`
`117`	`117`	`}`
`118`	`118`	`return flatState;`
`119`	`119`	`}`