-
Notifications
You must be signed in to change notification settings - Fork 557
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
feat: Add selectiveUnion
for improved Superstruct error messaging
#2696
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
FrederikBolding
changed the title
feat: Add selectiveUnion for improved Superstruct error messaging
feat: Add Sep 4, 2024
selectiveUnion
for improved Superstruct error messaging
Mrtenz
reviewed
Sep 4, 2024
FrederikBolding
force-pushed
the
fb/selective-union
branch
from
September 4, 2024 13:53
2e747c5
to
53660b4
Compare
Mrtenz
reviewed
Sep 4, 2024
Mrtenz
reviewed
Sep 4, 2024
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2696 +/- ##
==========================================
+ Coverage 94.31% 94.35% +0.04%
==========================================
Files 481 481
Lines 10057 10129 +72
Branches 1521 1540 +19
==========================================
+ Hits 9485 9557 +72
Misses 572 572 ☔ View full report in Codecov by Sentry. |
Mrtenz
force-pushed
the
fb/selective-union
branch
from
September 10, 2024 09:00
5347466
to
d76036d
Compare
@metamaskbot update-pr |
Mrtenz
force-pushed
the
fb/selective-union
branch
from
September 10, 2024 11:00
d76036d
to
62d8e96
Compare
david0xd
approved these changes
Sep 10, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds
selectiveUnion
and rewrites a bunch of structs to use this pattern. This helps alleviate a problem where Superstruct errors would be impossible to read. The idea ofselectiveUnion
is to be able to guide Superstruct validation more with simple checks that can be used to choose between validation paths. This combined withtypedUnion
makes it possible to have drastically improved error messages for structs that use unions. This PR rewrites some of the UI structs to use these new patterns, but is not necessarily exhaustive. It also adds support for refinements and coercion totypedUnion
.Generally, this PR turns errors like this:
into errors like this:
This PR does not solve this problem for structs that use the
children()
utility function with more than one struct as the input (e.g.Button
). These components will fail with something like:There may also still be other UI components that fail with similar errors, however, this is a huge step forward in readability and therefore dev-ex.
Progresses #2692