feat: reshape finally #148
Conversation
imranbarbhuiya
requested review from
favna,
kyranet and
vladfrangu
as code owners
|
after reading readme again, i think I've understood reshape incorrectly. Will change it |
Codecov Report
@@ Coverage Diff @@
## main #148 +/- ##
=======================================
Coverage 99.28% 99.28%
=======================================
Files 55 55
Lines 3198 3205 +7
Branches 795 796 +1
=======================================
+ Hits 3175 3182 +7
Misses 13 13
Partials 10 10
Continue to review full report at Codecov.
|
tests/validators/base.test.ts
Outdated
| : // FIXME: if I don't add this number generic, then `reshape` isn't able to infer the type | ||
| // Lmk should I keep it like this or r there any fix possible |
There was a problem hiding this comment.
I wonder if this a Results 2.0 issue 👀
There was a problem hiding this comment.
shapeshift don't uses @sapphire/result. The issue is Result.ok(number) becomes Result<number, Error> and Result.err(Error) becomes Result<unknown, Error> so the return type becomes Result<number, Error> | Result<unknown, Error> but reshape expects only one type that's why it errors.
There was a problem hiding this comment.
added a fix. But now Result.err(new Error('message')) is showing Result<string> idk why. Can u check again pls?
There was a problem hiding this comment.
What if we used @sapphire/result, tho? Shapeshift uses a class-based stripped-down version of it.
There was a problem hiding this comment.
Is it worth adding the full package when shapeshift only uses Ok and err?
There was a problem hiding this comment.
The idea was for shapeshift to be dependency-less, we've already had to accept some dependencies for the sake of browser compatibility / other validation library parity in fast-deep-equal and lodash.uniqwith but I would still prefer to keep it at a very low minimum.
No description provided.