More type friendly API
Gozala opened this issue · 0 comments
Give the en-typing effort (see #49) I think it might be a good moment to re-consider some API choices that could further increase benefits of using a type-checker. It also would not make sense to bundle any API changes with en-typing efforts so this issue is probably good place to outline current limitations along with proposed improvements.
I'll provide a list in no particular order (just things that catch my as I was taking a look at #49)
-
Exceptions aren't very type-friendly. Flow might eventually get a support for this, but as things stand now type-checker will not require you to
try / catch
functions that might throw. And even if you do usecatch (e) {...}
type checker won't know what the type ofe
is. It is worth pointing out that common languages with inferred static types primarily model errors viaResult<x, a>
type which is more or less following:type Result<x, a> | { value: a, isOk: true } | { error: x, isOk: false }
I would recommend encode / decode functions to return such a
Result<x, a>
type. On the other hand argument could be made that such API would feel awkward for plain JS consumer. To address that concern I would suggest one or both of the following:-
Rust uses
.unwrap()
for these kind of stuff that would cause panic if result is an error. Our results could also have.unwrap()
(or more javascripty.valueOf()
) method that returnsresult.value
or throwresult.error
. So API would be something likedecode(buffer).unwrap()
for untyped consumer anddecode(buffer)//:Result<Error, Multihash>
for typed. Please note that typed consumer would likely want to doif (result.isOk) { console.log(result.value) } else { console.error(result.error) }
& type checker will be able to infer everything. -
Expose functions that return
.unwrap()
-ed results. In fact current API does not need to change, rather new functions could be exposed that return results and existing API could just call those and return unwrapped results. I personally think that making untyped js to.unwrap()
is better option as it makes consumer acknowledge that function might throw, but then again I'm biased and migration to new AP may not be practical.
-
- Predicate functions like
isValidCode
aren't helpful with flow (ts actually has a some support for that). What that means in practice is thatif (isValidCode(x)) { encode(buf, x, len) }
will not type check ifx
isnumber
as at theencode
call-sitex
will still be inferred as number whileencode
expectsCode
.
To address this I would recommend replacing isValidCode
with (or adding another function) parseCode(number):?Code
which would allow type checker to infer everything by rewriting above example as:
const code = parseCode(x)
if (code) {
encode(buf, code, len)
}
For untyped consumer things would thing won't need to change as if (parseCode(x)) { encode(buf, x, len) }
would work just the same.
- Function
validate
is in fact a hybrid of above two, presumably intention is to describe for what reason validation failed. I would proposeparseMultihash(Buffer):Result<Error, Multihash>
instead which would just wrapbuffer
in result if it's valid multihash (but will have refinedMultihash
type) or error result carrying info what it's not valid just like with exception.