Skip to content

Commit c671bf3

Browse files
MajorLiftmetamaskbot
MajorLift
and
metamaskbot
authored
chore: Bump reselect to ^5.1.1 for heterogenously-typed selectors support (#29094)
## Motivation #### Homogenous selector types: ```ts const selectorOne = (state: State) => state.something; const selectorTwo = (state: State) => state.other; createSelector([selectorOne, selectorTwo], () => ...); ``` #### Heterogenous selector types: ```ts const selectorOne = (state: { something: string }) => state.something; const selectorTwo = (state: { other: number }) => state.other; createSelector([selectorOne, selectorTwo], () => ...); ``` Support for heterogenous typing is essential for selectors to function properly, because selectors must be both mergeable and atomic. Without heterogenous selectors, these becoming conflicting objectives. - **Mergeable:** - Without heterogenous typing for selectors, the size of the state type for all selectors tend to inflate. This is because a selector's state must be a supertype for the intersection of the state types of all of its merged selectors, including selectors nested in the definitions of merged selectors. - Eventually, many selectors end up being defined with a state type that is close to the entire Redux state. - Paradoxically, selectors that only need access to very few properties end up needing to have the widest state type, because they tend to be merged into the most selectors across several nested levels. - **Atomic:** - When selectors are actually invoked, including in test files, it's not always practical to prepare and pass in a very large state object. - It's both safer and more convenient to restrict the state being passed into the selector to the minimum size required for the selector to function. - This requirement becomes incompatible with the mergeability requirement if all selectors must share a common state type. Enabling merged selectors to accept different, even disjoint state types resolves this issue. > [!NOTE] > See the [`MultichainState`](https://github.com/MetaMask/metamask-extension/blob/4f970df0acec3e3bc80da08373aa3b16f23aae41/ui/selectors/multichain.ts#L53) type for an example of a bloated selector state type which will become unnecessary with this update. ## **Description** - `[email protected]` supports heterogenous typing for selector inputs to `createSelector` and `createDeepEqualSelector`. - reduxjs/reselect#351 - reduxjs/reselect#274 - reduxjs/reselect#315 - Upgrade to `^5.1.1` (latest) is necessary to fix type issues in `4.0.0` - https://app.circleci.com/jobs/github/MetaMask/metamask-extension/4320173 [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29094?quickstart=1) ## **Related issues** - Blocks TypeScript conversion of selectors for MetaMask/MetaMask-planning#2894. - #29014 ## **Manual testing steps** ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [x] 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). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] 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. --------- Co-authored-by: MetaMask Bot <[email protected]>
1 parent 57f5a6b commit c671bf3

File tree

13 files changed

+185
-179
lines changed

13 files changed

+185
-179
lines changed

‎lavamoat/browserify/beta/policy.json‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1694,10 +1694,10 @@
16941694
"@metamask/network-controller>@metamask/eth-block-tracker": true,
16951695
"@metamask/network-controller>@metamask/eth-json-rpc-infura": true,
16961696
"@metamask/network-controller>@metamask/swappable-obj-proxy": true,
1697-
"@metamask/network-controller>reselect": true,
16981697
"@metamask/rpc-errors": true,
16991698
"@metamask/utils": true,
17001699
"eslint>fast-deep-equal": true,
1700+
"reselect": true,
17011701
"uri-js": true,
17021702
"uuid": true
17031703
}
@@ -1756,13 +1756,6 @@
17561756
"semver": true
17571757
}
17581758
},
1759-
"@metamask/network-controller>reselect": {
1760-
"globals": {
1761-
"WeakRef": true,
1762-
"console.warn": true,
1763-
"unstable_autotrackMemoize": true
1764-
}
1765-
},
17661759
"@metamask/notification-controller>nanoid": {
17671760
"globals": {
17681761
"crypto.getRandomValues": true
@@ -5138,6 +5131,13 @@
51385131
"@babel/runtime": true
51395132
}
51405133
},
5134+
"reselect": {
5135+
"globals": {
5136+
"WeakRef": true,
5137+
"console.warn": true,
5138+
"unstable_autotrackMemoize": true
5139+
}
5140+
},
51415141
"semver": {
51425142
"globals": {
51435143
"console.error": true

‎lavamoat/browserify/flask/policy.json‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1694,10 +1694,10 @@
16941694
"@metamask/network-controller>@metamask/eth-block-tracker": true,
16951695
"@metamask/network-controller>@metamask/eth-json-rpc-infura": true,
16961696
"@metamask/network-controller>@metamask/swappable-obj-proxy": true,
1697-
"@metamask/network-controller>reselect": true,
16981697
"@metamask/rpc-errors": true,
16991698
"@metamask/utils": true,
17001699
"eslint>fast-deep-equal": true,
1700+
"reselect": true,
17011701
"uri-js": true,
17021702
"uuid": true
17031703
}
@@ -1756,13 +1756,6 @@
17561756
"semver": true
17571757
}
17581758
},
1759-
"@metamask/network-controller>reselect": {
1760-
"globals": {
1761-
"WeakRef": true,
1762-
"console.warn": true,
1763-
"unstable_autotrackMemoize": true
1764-
}
1765-
},
17661759
"@metamask/notification-controller>nanoid": {
17671760
"globals": {
17681761
"crypto.getRandomValues": true
@@ -5138,6 +5131,13 @@
51385131
"@babel/runtime": true
51395132
}
51405133
},
5134+
"reselect": {
5135+
"globals": {
5136+
"WeakRef": true,
5137+
"console.warn": true,
5138+
"unstable_autotrackMemoize": true
5139+
}
5140+
},
51415141
"semver": {
51425142
"globals": {
51435143
"console.error": true

‎lavamoat/browserify/main/policy.json‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1694,10 +1694,10 @@
16941694
"@metamask/network-controller>@metamask/eth-block-tracker": true,
16951695
"@metamask/network-controller>@metamask/eth-json-rpc-infura": true,
16961696
"@metamask/network-controller>@metamask/swappable-obj-proxy": true,
1697-
"@metamask/network-controller>reselect": true,
16981697
"@metamask/rpc-errors": true,
16991698
"@metamask/utils": true,
17001699
"eslint>fast-deep-equal": true,
1700+
"reselect": true,
17011701
"uri-js": true,
17021702
"uuid": true
17031703
}
@@ -1756,13 +1756,6 @@
17561756
"semver": true
17571757
}
17581758
},
1759-
"@metamask/network-controller>reselect": {
1760-
"globals": {
1761-
"WeakRef": true,
1762-
"console.warn": true,
1763-
"unstable_autotrackMemoize": true
1764-
}
1765-
},
17661759
"@metamask/notification-controller>nanoid": {
17671760
"globals": {
17681761
"crypto.getRandomValues": true
@@ -5138,6 +5131,13 @@
51385131
"@babel/runtime": true
51395132
}
51405133
},
5134+
"reselect": {
5135+
"globals": {
5136+
"WeakRef": true,
5137+
"console.warn": true,
5138+
"unstable_autotrackMemoize": true
5139+
}
5140+
},
51415141
"semver": {
51425142
"globals": {
51435143
"console.error": true

‎lavamoat/browserify/mmi/policy.json‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1786,10 +1786,10 @@
17861786
"@metamask/network-controller>@metamask/eth-block-tracker": true,
17871787
"@metamask/network-controller>@metamask/eth-json-rpc-infura": true,
17881788
"@metamask/network-controller>@metamask/swappable-obj-proxy": true,
1789-
"@metamask/network-controller>reselect": true,
17901789
"@metamask/rpc-errors": true,
17911790
"@metamask/utils": true,
17921791
"eslint>fast-deep-equal": true,
1792+
"reselect": true,
17931793
"uri-js": true,
17941794
"uuid": true
17951795
}
@@ -1848,13 +1848,6 @@
18481848
"semver": true
18491849
}
18501850
},
1851-
"@metamask/network-controller>reselect": {
1852-
"globals": {
1853-
"WeakRef": true,
1854-
"console.warn": true,
1855-
"unstable_autotrackMemoize": true
1856-
}
1857-
},
18581851
"@metamask/notification-controller>nanoid": {
18591852
"globals": {
18601853
"crypto.getRandomValues": true
@@ -5230,6 +5223,13 @@
52305223
"@babel/runtime": true
52315224
}
52325225
},
5226+
"reselect": {
5227+
"globals": {
5228+
"WeakRef": true,
5229+
"console.warn": true,
5230+
"unstable_autotrackMemoize": true
5231+
}
5232+
},
52335233
"semver": {
52345234
"globals": {
52355235
"console.error": true

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -426,7 +426,7 @@
426426
"redux": "^4.0.5",
427427
"redux-thunk": "^2.3.0",
428428
"remove-trailing-slash": "^0.1.1",
429-
"reselect": "^3.0.1",
429+
"reselect": "^5.1.1",
430430
"ses": "^1.1.0",
431431
"simple-git": "^3.20.0",
432432
"single-call-balance-checker-abi": "^1.0.0",

‎shared/modules/selectors/smart-transactions.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ export const getSmartTransactionsOptInStatusInternal = createSelector(
8484
* @returns true if the user has explicitly opted in, false if they have opted out,
8585
* or null if they have not explicitly opted in or out.
8686
*/
87+
// @ts-expect-error TODO: Fix types for `getSmartTransactionsOptInStatusInternal` once `getPreferences is converted to TypeScript
8788
export const getSmartTransactionsOptInStatusForMetrics = createSelector(
8889
getSmartTransactionsOptInStatusInternal,
8990
(optInStatus: boolean): boolean => optInStatus,
@@ -96,6 +97,7 @@ export const getSmartTransactionsOptInStatusForMetrics = createSelector(
9697
* @param state
9798
* @returns
9899
*/
100+
// @ts-expect-error TODO: Fix types for `getSmartTransactionsOptInStatusInternal` once `getPreferences is converted to TypeScript
99101
export const getSmartTransactionsPreferenceEnabled = createSelector(
100102
getSmartTransactionsOptInStatusInternal,
101103
(optInStatus: boolean): boolean => {

‎shared/modules/selectors/util.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
import { TransactionStatus } from '@metamask/transaction-controller';
22
import { isEqual } from 'lodash';
3-
import { createSelectorCreator, defaultMemoize } from 'reselect';
3+
import { createSelectorCreator, lruMemoize } from 'reselect';
44

55
export const createDeepEqualSelector = createSelectorCreator(
6-
defaultMemoize,
6+
lruMemoize,
77
isEqual,
88
);
99

‎ui/components/app/snaps/snap-authorship-pill/snap-authorship-pill.tsx‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ const SnapAuthorshipPill: React.FC<SnapAuthorshipPillProps> = ({
2222
onClick,
2323
}) => {
2424
const { name: snapName } = useSelector((state) =>
25-
// @ts-expect-error ts is picking up the wrong type for the selector
2625
getSnapMetadata(state, snapId),
2726
);
2827

‎ui/components/app/snaps/snap-icon/snap-icon.tsx‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@ export const SnapIcon: FunctionComponent<SnapIconProps> = ({
4040
);
4141

4242
const { name: snapName } = useSelector((state) =>
43-
/* @ts-expect-error wrong type on selector. */
4443
getSnapMetadata(state, snapId),
4544
);
4645

‎ui/ducks/bridge/selectors.test.ts‎

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,11 @@ import {
1313
import { mockNetworkState } from '../../../test/stub/networks';
1414
import mockErc20Erc20Quotes from '../../../test/data/bridge/mock-quotes-erc20-erc20.json';
1515
import mockBridgeQuotesNativeErc20 from '../../../test/data/bridge/mock-quotes-native-erc20.json';
16-
import { SortOrder } from '../../pages/bridge/types';
16+
import {
17+
QuoteMetadata,
18+
QuoteResponse,
19+
SortOrder,
20+
} from '../../pages/bridge/types';
1721
import {
1822
getAllBridgeableNetworks,
1923
getBridgeQuotes,
@@ -722,9 +726,11 @@ describe('Bridge selectors', () => {
722726
{ valueInCurrency: new BigNumber('0.156562871410260918428') },
723727
{ valueInCurrency: new BigNumber('0.33900008283534602') },
724728
];
725-
result.sortedQuotes.forEach((quote, idx) => {
726-
expect(quote.cost).toStrictEqual(EXPECTED_SORTED_COSTS[idx]);
727-
});
729+
result.sortedQuotes.forEach(
730+
(quote: QuoteMetadata & QuoteResponse, idx: number) => {
731+
expect(quote.cost).toStrictEqual(EXPECTED_SORTED_COSTS[idx]);
732+
},
733+
);
728734
expect(result).toStrictEqual({
729735
sortedQuotes: expect.any(Array),
730736
recommendedQuote: {
@@ -827,9 +833,11 @@ describe('Bridge selectors', () => {
827833
{ valueInCurrency: new BigNumber('0.15656287141025952') },
828834
{ valueInCurrency: new BigNumber('0.33900008283534464') },
829835
];
830-
result.sortedQuotes.forEach((quote, idx) => {
831-
expect(quote.cost).toStrictEqual(EXPECTED_SORTED_COSTS[idx]);
832-
});
836+
result.sortedQuotes.forEach(
837+
(quote: QuoteMetadata & QuoteResponse, idx: number) => {
838+
expect(quote.cost).toStrictEqual(EXPECTED_SORTED_COSTS[idx]);
839+
},
840+
);
833841

834842
expect(result).toStrictEqual({
835843
sortedQuotes: expect.any(Array),
@@ -898,14 +906,18 @@ describe('Bridge selectors', () => {
898906
'381c23bc-e3e4-48fe-bc53-257471e388ad',
899907
);
900908
expect(sortedQuotes).toHaveLength(2);
901-
sortedQuotes.forEach((quote, idx) => {
902-
expect(
903-
quoteMetadataKeys.every((k) => Object.keys(quote ?? {}).includes(k)),
904-
).toBe(true);
905-
expect(quote?.quote.requestId).toStrictEqual(
906-
mockBridgeQuotesNativeErc20[idx]?.quote.requestId,
907-
);
908-
});
909+
sortedQuotes.forEach(
910+
(quote: QuoteMetadata & QuoteResponse, idx: number) => {
911+
expect(
912+
quoteMetadataKeys.every((k) =>
913+
Object.keys(quote ?? {}).includes(k),
914+
),
915+
).toBe(true);
916+
expect(quote?.quote.requestId).toStrictEqual(
917+
mockBridgeQuotesNativeErc20[idx]?.quote.requestId,
918+
);
919+
},
920+
);
909921
});
910922

911923
it('should sort quotes by ETA', () => {

0 commit comments

Comments
 (0)