-
Notifications
You must be signed in to change notification settings - Fork 639
chore: Refactor stateful component check in constructState
#3251
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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 | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -40,6 +40,41 @@ 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; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Awesome! 👍 |
||
| } { | ||
| return STATEFUL_COMPONENT_TYPES.includes( | ||
| component.type as StatefulComponentType, | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * A function to get the MultichainAssetController state. | ||
| * | ||
|
|
@@ -366,18 +401,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( | ||
|
|
@@ -390,17 +414,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, | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.