Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/snaps-controllers/coverage.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"branches": 93.51,
"branches": 93.34,
"functions": 97.36,
"lines": 98.33,
"statements": 98.06
"statements": 98.07
}
11 changes: 11 additions & 0 deletions packages/snaps-controllers/src/interface/utils.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
getAssetSelectorStateValue,
getDefaultAsset,
getJsxInterface,
isStatefulComponent,
} from './utils';
import { MOCK_ACCOUNT_ID } from '../test-utils';

Expand Down Expand Up @@ -1222,3 +1223,13 @@ describe('getDefaultAsset', () => {
);
});
});

describe('isStatefulComponent', () => {
it('returns true for stateful components', () => {
expect(isStatefulComponent(<Input name="foo" />)).toBe(true);
});

it('returns false for stateless components', () => {
expect(isStatefulComponent(<Text>foo</Text>)).toBe(false);
});
});
61 changes: 38 additions & 23 deletions packages/snaps-controllers/src/interface/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,42 @@ import {
parseCaipChainId,
} from '@metamask/utils';

/**
* A list of stateful component types.
*/
const STATEFUL_COMPONENT_TYPES = [
'Input',
'Dropdown',
'RadioGroup',
'FileInput',
'Checkbox',
'Selector',
'AssetSelector',
'AddressInput',
] as const;

/**
* Type for stateful component types.
*/
type StatefulComponentType = (typeof STATEFUL_COMPONENT_TYPES)[number];

/**
* Check if a component is a stateful component.
*
* @param component - The component to check.
* @param component.type - The type of the component.
*
* @returns Whether the component is a stateful component.
*/
export function isStatefulComponent(component: { type: string }): component is {
type: StatefulComponentType;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, does this then properly infer the props inside the if-clause in constructState? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I have no issues in my IDE and when I look at the infered type its an union of the stateful components, I would like to send you a screenshot but I can't hit ctrl without the IDE context widow closing 😅

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! 👍

props: Record<string, any>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not ideal 🤔 Is this required?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we at least type it to be a union of the stateful component's props?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thought it was required at some point but it isn't anymore apparently, removed

} {
return STATEFUL_COMPONENT_TYPES.includes(
component.type as StatefulComponentType,
);
}

/**
* A function to get the MultichainAssetController state.
*
Expand Down Expand Up @@ -366,18 +402,7 @@ export function constructState(
}

// Stateful components inside a form
// TODO: This is becoming a bit of a mess, we should consider refactoring this.
if (
currentForm &&
(component.type === 'Input' ||
component.type === 'Dropdown' ||
component.type === 'RadioGroup' ||
component.type === 'FileInput' ||
component.type === 'Checkbox' ||
component.type === 'Selector' ||
component.type === 'AssetSelector' ||
component.type === 'AddressInput')
) {
if (currentForm && isStatefulComponent(component)) {
const formState = newState[currentForm.name] as FormState;
assertNameIsUnique(formState, component.props.name);
formState[component.props.name] = constructInputState(
Expand All @@ -390,17 +415,7 @@ export function constructState(
}

// Stateful components outside a form
// TODO: This is becoming a bit of a mess, we should consider refactoring this.
if (
component.type === 'Input' ||
component.type === 'Dropdown' ||
component.type === 'RadioGroup' ||
component.type === 'FileInput' ||
component.type === 'Checkbox' ||
component.type === 'Selector' ||
component.type === 'AssetSelector' ||
component.type === 'AddressInput'
) {
if (isStatefulComponent(component)) {
assertNameIsUnique(newState, component.props.name);
newState[component.props.name] = constructInputState(
oldState,
Expand Down
Loading