You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
This repository has been archived by the owner on Aug 24, 2021. It is now read-only.
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 use catch (e) {...} type checker won't know what the type of e is. It is worth pointing out that common languages with inferred static types primarily model errors via Result<x, a> type which is more or less following:
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 returns result.value or throw result.error. So API would be something like decode(buffer).unwrap() for untyped consumer and decode(buffer)//:Result<Error, Multihash> for typed. Please note that typed consumer would likely want to do if (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 that if (isValidCode(x)) { encode(buf, x, len) } will not type check if x is number as at the encode call-site x will still be inferred as number while encode expects Code.
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:
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 propose parseMultihash(Buffer):Result<Error, Multihash> instead which would just wrap buffer in result if it's valid multihash (but will have refined Multihash type) or error result carrying info what it's not valid just like with exception.
The text was updated successfully, but these errors were encountered:
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: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.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:For untyped consumer things would thing won't need to change as
if (parseCode(x)) { encode(buf, x, len) }
would work just the same.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.The text was updated successfully, but these errors were encountered: