Skip to content
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

[base-controller] Require metadata for optional state properties #4252

Closed
Gudahtt opened this issue May 3, 2024 · 2 comments · Fixed by #4612
Closed

[base-controller] Require metadata for optional state properties #4252

Gudahtt opened this issue May 3, 2024 · 2 comments · Fixed by #4612
Assignees
Labels
bug Something isn't working team-wallet-framework

Comments

@Gudahtt
Copy link
Member

Gudahtt commented May 3, 2024

The StateMetadata type in BaseControllerV2 was intended to require each state property to have associated metadata. However, currently it doesn't require metadata for optional fields. This is problematic because all top-level state properties are expected to have metadata, and will throw an error at runtime if it's missing.

We should update the StateMetadata type to require metadata for all properties, including optional properties.

@Gudahtt Gudahtt added bug Something isn't working team-wallet-framework labels May 3, 2024
@desi
Copy link

desi commented Jul 18, 2024

Wrap in the utility type called "Required".

@MajorLift MajorLift assigned MajorLift and unassigned MajorLift Jul 22, 2024
@Gudahtt
Copy link
Member Author

Gudahtt commented Aug 15, 2024

Here is an example of a bug fix for a bug that would have been caught by this improvement to the type: #4245

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working team-wallet-framework
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants