Skip to content

Commit

Permalink
Fix NetworkController Redux selectors
Browse files Browse the repository at this point in the history
When running tests, we are seeing the following warning:

> An input selector returned a different result when passed same
> arguments. This means your output selector will likely run more
> frequently than intended.

This is happening because the "input selector" in
`selectAvailableNetworkClientIds` returns a new object reference (returned
by `getNetworkConfigurations`) each time it is called. This violates the
Redux guidelines outlined here:
<https://redux.js.org/usage/deriving-data-selectors#optimizing-selectors-with-memoization>.

To fix this, we split off the "input selector" into its own selector
function which we then memoize.
  • Loading branch information
mcmire committed Jan 23, 2025
1 parent dcc0df0 commit eb7baeb
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 5 deletions.
2 changes: 1 addition & 1 deletion eslint-warning-thresholds.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
"jest/prefer-strict-equal": 2,
"jsdoc/check-tag-names": 375,
"jsdoc/require-returns": 25,
"jsdoc/tag-lines": 335,
"jsdoc/tag-lines": 334,
"n/no-unsupported-features/node-builtins": 4,
"n/prefer-global/text-encoder": 4,
"n/prefer-global/text-decoder": 4,
Expand Down
4 changes: 4 additions & 0 deletions packages/network-controller/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- Bump `@metamask/base-controller` from `^7.0.0` to `^7.1.0` ([#5079](https://github.com/MetaMask/core/pull/5079))

### Fixed

- Fix `selectAvailableNetworkClientIds` so that it is properly memoized ([#5193](https://github.com/MetaMask/core/pull/5193))

## [22.1.1]

### Changed
Expand Down
41 changes: 37 additions & 4 deletions packages/network-controller/src/NetworkController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -589,6 +589,16 @@ export function getDefaultNetworkControllerState(): NetworkState {
};
}

/**
* Redux selector for getting all network configurations from NetworkController
* state, keyed by chain ID.
*
* @param state - NetworkController state
* @returns All registered network configurations, keyed by chain ID.
*/
const selectNetworkConfigurationsByChainId = (state: NetworkState) =>
state.networkConfigurationsByChainId;

/**
* Get a list of all network configurations.
*
Expand All @@ -602,8 +612,22 @@ export function getNetworkConfigurations(
}

/**
* Get a list of all available client IDs from a list of
* network configurations
* Redux selector for getting a list of all network configurations from
* NetworkController state.
*
* @param state - NetworkController state
* @returns A list of all available network configurations
*/
const selectNetworkConfigurations = createSelector(
selectNetworkConfigurationsByChainId,
(networkConfigurationsByChainId) =>
Object.values(networkConfigurationsByChainId),
);

/**
* Get a list of all available network client IDs from a list of network
* configurations.
*
* @param networkConfigurations - The array of network configurations
* @returns A list of all available client IDs
*/
Expand All @@ -617,8 +641,15 @@ export function getAvailableNetworkClientIds(
);
}

/**
* Redux selector for getting a list of all available network client IDs
* from NetworkController state.
*
* @param state - NetworkController state
* @returns A list of all available network client IDs.
*/
export const selectAvailableNetworkClientIds = createSelector(
[getNetworkConfigurations],
selectNetworkConfigurations,
getAvailableNetworkClientIds,
);

Expand Down Expand Up @@ -773,7 +804,9 @@ function validateNetworkControllerState(state: NetworkState) {
const networkConfigurationEntries = Object.entries(
state.networkConfigurationsByChainId,
);
const networkClientIds = selectAvailableNetworkClientIds(state);
const networkClientIds = getAvailableNetworkClientIds(
getNetworkConfigurations(state),
);

if (networkConfigurationEntries.length === 0) {
throw new Error(
Expand Down
19 changes: 19 additions & 0 deletions packages/network-controller/tests/NetworkController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import {
NetworkController,
RpcEndpointType,
selectAvailableNetworkClientIds,
selectNetworkConfigurations,
} from '../src/NetworkController';
import type { NetworkClientConfiguration, Provider } from '../src/types';
import { NetworkClientType } from '../src/types';
Expand Down Expand Up @@ -13024,6 +13025,24 @@ describe('getNetworkConfigurations', () => {
});
});

describe('selectNetworkConfigurations', () => {
it('returns network configurations available in the state', () => {
const state = getDefaultNetworkControllerState();

expect(selectNetworkConfigurations(state)).toStrictEqual(
Object.values(state.networkConfigurationsByChainId),
);
});

it('is memoized', () => {
const state = getDefaultNetworkControllerState();

expect(selectNetworkConfigurations(state)).toBe(
selectNetworkConfigurations(state),
);
});
});

describe('getAvailableNetworkClientIds', () => {
it('returns network client ids available in the state', () => {
const networkConfigurations = [
Expand Down

0 comments on commit eb7baeb

Please sign in to comment.