fix: Properly merge structs #2512
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2512 +/- ##
=======================================
Coverage 94.40% 94.40%
=======================================
Files 442 442
Lines 9127 9134 +7
Branches 1412 1412
=======================================
+ Hits 8616 8623 +7
Misses 511 511 ☔ View full report in Codecov by Sentry. |
FrederikBolding
force-pushed
the
fb/properly-merge-structs
branch
from
c44eb90
to
95816c3
Compare
| * @returns The merged struct. | ||
| */ | ||
| export function mergeStructs(...structs: Struct<any>[]): Struct<any> { | ||
| const mergedStruct = (assign as (...structs: Struct<any>[]) => Struct)( |
There was a problem hiding this comment.
Why is this type assertion necessary?
There was a problem hiding this comment.
I'm assuming it is caused by the type declarations for Superstruct being overwritten. The type declarations does not contain types for the function with a rest parameter.
Without the cast we get A spread argument must either have a tuple type or be passed to a rest parameter., but everything still works.
There was a problem hiding this comment.
To be clear, the underlying function does support a rest parameter: https://github.com/ianstormtaylor/superstruct/blob/88563ad3abb818ee4d129e2d4af3c75f5ef56462/src/structs/utilities.ts#L60
This PR introduces
mergeStructswhich merges structs, including refinements, whichsuperstructsassigndoes not do by default. This also fixes a bug whereendowment:rpc: {}would not be reported as invalid until attempting to install the Snap.Fixes #2405