Skip to content

Commit 2ad12d1

Browse files
authored
chore: Refactor stateful component check in constructState (#3251)
This PR refactors `constructState` to use an util function called `isStatefulComponent` that uses an array of component types that includes all the stateful components. Fixes: #3235
1 parent 8ce2e71 commit 2ad12d1

File tree

3 files changed

+82
-25
lines changed

3 files changed

+82
-25
lines changed
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
2-
"branches": 93.51,
2+
"branches": 93.34,
33
"functions": 97.36,
44
"lines": 98.33,
5-
"statements": 98.06
5+
"statements": 98.07
66
}

packages/snaps-controllers/src/interface/utils.test.tsx

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import {
2525
getAssetSelectorStateValue,
2626
getDefaultAsset,
2727
getJsxInterface,
28+
isStatefulComponent,
2829
} from './utils';
2930
import { MOCK_ACCOUNT_ID } from '../test-utils';
3031

@@ -1222,3 +1223,45 @@ describe('getDefaultAsset', () => {
12221223
);
12231224
});
12241225
});
1226+
1227+
describe('isStatefulComponent', () => {
1228+
it.each([
1229+
<Input name="foo" />,
1230+
<Dropdown name="foo">
1231+
<Option value="option1">Option 1</Option>
1232+
</Dropdown>,
1233+
<RadioGroup name="foo">
1234+
<Radio value="option1">Option 1</Radio>
1235+
</RadioGroup>,
1236+
<Checkbox name="foo" />,
1237+
<Selector name="foo" title="Choose an option">
1238+
<SelectorOption value="option1">
1239+
<Text>Option 1</Text>
1240+
</SelectorOption>
1241+
</Selector>,
1242+
<AssetSelector
1243+
name="foo"
1244+
addresses={[
1245+
'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp:7S3P4HxJpyyigGzodYwHtCxZyUQe9JiBMHyRWXArAaKv',
1246+
]}
1247+
/>,
1248+
<FileInput name="foo" />,
1249+
<AddressInput name="foo" chainId="eip155:1" />,
1250+
])('returns true for "%p"', () => {
1251+
expect(isStatefulComponent(<Input name="foo" />)).toBe(true);
1252+
});
1253+
1254+
it('returns false for stateless components', () => {
1255+
expect(isStatefulComponent(<Text>foo</Text>)).toBe(false);
1256+
});
1257+
1258+
it('returns false for nested stateful components', () => {
1259+
expect(
1260+
isStatefulComponent(
1261+
<Box>
1262+
<Input name="foo" />
1263+
</Box>,
1264+
),
1265+
).toBe(false);
1266+
});
1267+
});

packages/snaps-controllers/src/interface/utils.ts

Lines changed: 37 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,41 @@ import {
4040
parseCaipChainId,
4141
} from '@metamask/utils';
4242

43+
/**
44+
* A list of stateful component types.
45+
*/
46+
const STATEFUL_COMPONENT_TYPES = [
47+
'Input',
48+
'Dropdown',
49+
'RadioGroup',
50+
'FileInput',
51+
'Checkbox',
52+
'Selector',
53+
'AssetSelector',
54+
'AddressInput',
55+
] as const;
56+
57+
/**
58+
* Type for stateful component types.
59+
*/
60+
type StatefulComponentType = (typeof STATEFUL_COMPONENT_TYPES)[number];
61+
62+
/**
63+
* Check if a component is a stateful component.
64+
*
65+
* @param component - The component to check.
66+
* @param component.type - The type of the component.
67+
*
68+
* @returns Whether the component is a stateful component.
69+
*/
70+
export function isStatefulComponent(component: { type: string }): component is {
71+
type: StatefulComponentType;
72+
} {
73+
return STATEFUL_COMPONENT_TYPES.includes(
74+
component.type as StatefulComponentType,
75+
);
76+
}
77+
4378
/**
4479
* A function to get the MultichainAssetController state.
4580
*
@@ -366,18 +401,7 @@ export function constructState(
366401
}
367402

368403
// Stateful components inside a form
369-
// TODO: This is becoming a bit of a mess, we should consider refactoring this.
370-
if (
371-
currentForm &&
372-
(component.type === 'Input' ||
373-
component.type === 'Dropdown' ||
374-
component.type === 'RadioGroup' ||
375-
component.type === 'FileInput' ||
376-
component.type === 'Checkbox' ||
377-
component.type === 'Selector' ||
378-
component.type === 'AssetSelector' ||
379-
component.type === 'AddressInput')
380-
) {
404+
if (currentForm && isStatefulComponent(component)) {
381405
const formState = newState[currentForm.name] as FormState;
382406
assertNameIsUnique(formState, component.props.name);
383407
formState[component.props.name] = constructInputState(
@@ -390,17 +414,7 @@ export function constructState(
390414
}
391415

392416
// Stateful components outside a form
393-
// TODO: This is becoming a bit of a mess, we should consider refactoring this.
394-
if (
395-
component.type === 'Input' ||
396-
component.type === 'Dropdown' ||
397-
component.type === 'RadioGroup' ||
398-
component.type === 'FileInput' ||
399-
component.type === 'Checkbox' ||
400-
component.type === 'Selector' ||
401-
component.type === 'AssetSelector' ||
402-
component.type === 'AddressInput'
403-
) {
417+
if (isStatefulComponent(component)) {
404418
assertNameIsUnique(newState, component.props.name);
405419
newState[component.props.name] = constructInputState(
406420
oldState,

0 commit comments

Comments
 (0)