Enable type-checker support in js-ipfs and js-libp2p

[Gozala]

Hi Folks,

Lately I have being trying to use js-ipfs in a project and got somewhat overwhelmed by amount of libraries at play, don’t get me wrong, I like the seperation of concerns this layered setup provides, it’s just incredibly hard to pin down what are the things being passed around and I’ve being following a rabbit hole for multiple days now. I also noticed the pattern across the stack where there are defined interfaces like transports and ipdl-resolvers, etc... and multiple implementations of them.

This got me thinking that it would be exteremly helpful if those interfaces were not just written out in markdown but were typed as flow (or typescript) interfaces than implementation could in fact declare that class / object implements it and type checker will make sure that is the case.

As I have being struggling to understand shapes of things being passed around (argument types, return types) I really started wishing those were tyoe annotated as that would have saved me so much time jumping across libraries to look things up. In fact I noticed that I had to look same things multiple times because as I kept going amount of info overflowed my mental capacity. That is yet another time where I wished type signatures were available as that would allowed my editor to just tell me what the interfaces is without following dependency chain and looking things up. In fact it got so difficult that I end up writing type signatures myself to save time redoing same lookups over and over.

Another controversial but still argument worth making is that there have type checker help make API’s clearer and lmby consequence easier for users. For instance I encountered a function that either takes protocol and buffer and returns string or takes protocol and string and returns buffer, it’s cool but consequently margin for error is higher + has runtime costs. Once you start using type system it navigates you towards less ambiguous API as concrete return types save you from type refinements down the line.

I also noticed multiple cases of polymorphic APIs where funtion needs to match over the type of argument and those checks are somewhat optimistic (as in no guarantee that argument is actually implements expected interface) that is another area where type checker like flow would greatly help. It provides a specific way to refine types and provides guarantees that with in the refined block value implements interface of that type it also ensures that no variant is being overlooked (meaning that each variant of argument type us handled).

[RangerMauve]

Typing would be useful. If this gains traction, I'd +1 for typescript, since flow is mostly just pushed by FB.
Maybe even having both type files where one is generated from the other.

[Gozala]

There is multiple ways one could go about adding type-checker support and each has different value proposition. I would personally advocate to go full on and just add flow to a toolchain as it would give you all the benefits at the cost of contributir workflow. I have experience with starting to use comments initially before going full on and from my experience no developer involved wanted to go back.

I understand that may not be ready to do that. So alternative could be to just add type annotations so that users of libraries will reap all the benefits and hopefully implementation would actually work as typed. I already submitted some pull requests to that end. Main issue with this approach is that type definitions and implementation start to get out off sync mostly because core contributors off the library don’t actually use type annotatins and type check can’t report any mismatches either.

[Gozala]

I’m also happy to lead this effort if there is a will to accept this.

[Gozala]

Typing would be useful. If this gains traction, I'd +1 for typescript, since flow is mostly just pushed by FB.

@RangerMauve It is true, but isn’t ts also just pushed by Microsoft ? I would prefer flow as type inference is far more superior and toolchain does not require full buy-inn.

Maybe even having both type files where one is generated from the other.

Sadly there is no such tool as far as I’m aware but if one of the typecheckers is used writing just tyoe signatures for the other should be viable and less prone to be out of sync

[Gozala]

I am also happy to provide specific explanation of why flow type checker type inference is superior and what does that actually provides as consequence if that is a conversation worth having.

[RangerMauve]

TS is used by more than just Microsoft, for example, Google's Angular team found it appealing enough to focus Angular 2+ on it, and I, personally, have seen more projects using TS than using flow.

[Gozala]

@RangerMauve Well if you're talking about users there are ton of users of flow as well at Mozilla we use it quite a bit and most of the projects around React ecosystem tend to use flow for the reasons you can guess. That being said you're probably right and more projects use TS but I wonder how much of that is due to TS being just an older project.

In terms of contributors to ts and flow projects they seemed to be mostly ms and facebook.

[RangerMauve]

This is why I would suggest supporting both, because otherwise there will be a constant stream of people arguing over which type system should be integrated with or not.

[Gozala]

Anyway here is a small list of objective reasons why I think flow is a better choice:

1. Flow type checker is nothing but a type checker, meaning it just type checks. Transpiling flow typed code into valid JS is done by other means often using babel and there is far more flexibility in terms of what you want output be than what you get with TS where tool does type checking transpiling and more, don't get me wrong it has configuration options but I've run into limitations that are a lot easier to overcome with babel.
2. I would not recommend doing this, but flow has an option to be typed via comments meaning that you have an option not to use any compilation at all.
3. Flow provides nominal typing along with structural typing via opaque type aliases which make a huge difference for performance critical code paths. Whenever you want to represent certain via string or Buffer or number and don't pay boxing costs but reap all the benefits of typing opaque type aliases give you type safety at 0 runtime cost.
4. Flow has type variance and ts does not, that allows flow type checker catch whole class of type errors that ts just cant.
5. Flow type inference is far more sophisticated. Easiest way to spot that is to say have a generic function / class that does not refer to it's type parameter. In flow that type will be inferred from the further usage while TS just treats it as {} that often a very bad assumption often forcing you to manually entype that parameter is any which essentially disables any kind of type checks for it. This is kind of boils down to flow having existential type and TS not having it, also notice that is fundamental difference between approach each type checker takes, or to put it differently it would require fundamental rewrite TS to fix that.
6. Flow provides $Call utility type that provides you essentially a way to extend type system in arbitrary ways as long as you can express that in form of function type signature.

[Gozala]

To be absolutely clear I'm happy to do the work necessary for flow integration. As of TS support, I think it would be great and someone should do it, it's just I can not contribute to that effort as it does not help me with my project. I hope you understand.

[Gozala]

Here is a list of PRs I'have submitted yesterday, before @victorbjelkholm asked me to open this issue:

• Add flow type signatures. libp2p/js-peer-info#62
• Add flow type signatures. libp2p/js-peer-id#75
• Add flow type annotations. multiformats/js-multiaddr#51

[Gozala]

Copying conversation from that happend over at libp2p/js-peer-info#62 (comment) with @dryajov as discussing it here makes more sense:

Is there anyway of distributing the typings as a separate package (similar to typescript @type/), so that we don't clutter this package with language specific types?

@dryajov there is flow-typed but sadly it's not nearly as good as in typescript and most flow users don't seem to use it for that reason. It is also that there is advantage of having type definitions with implementations as that gives some initiative to library contributors to keep them in sync otherwise they just don't match up (at least that is a big issue with flow-typed) type defs.

[Gozala]

So here is the list of options in which flow types could be added to the ipfs libraries:

1. Go full on with flow type checker.

Pros

• Type signatures will be guaranteed to be in sync with implementation.
• Implementations will be able to take advantages of type checker.
• Integration tools could type check pulls to make sure type safety.
• Libraries that implementing interface will be verified to do so by type checker.
• Users will just get types and guarantees without any extra work.

Cons

• Contributors would need to use flow type checker.
• Contributors will have to transpile flow typed code into JS.

2. Comment Types

Same list of pros / cons as above, except you'd trade need for transpilation with a lengthy comments blocks inline (usually makes it column width 80 a huge challenge).

3. Write file.js.flow along the side of implementation.

Pros

• Some libraries could take advantage of type checker. Kind of allow you to gradually explore options 1 and 2.
• Users will just get types and guarantees without any extra work.
• Provides some initiative to contributors to keep interface and implementations in in sync.

Cons

• No guarantee that implementation actually complies with type definition.
• Interfaces and implementations can get out of sync unless contributors are disciplined, in other words no tools won't be able to help you.

4. Add ipfs to flow-typed

I personally found that wrapping untyped library in another that just adds types is far more manageable than going through flow-typed as it reduces tool overhead and coordination with flow-typed maintainers.

Pros

• Users of ipfs could use flow-typed tool to get some type information.

Cons

• No guarantee that implementation actually complies with type definition.
• Users need to use additional tool in form of flow-typed
• Implementation and types are likely to get out of sync as contributors aren't even exposed to type defs.
• Keeping implementation and types in sync add's more steps to the workflow. Updating separate package, submitting a pull request etc..
• Sharing across libraries is somewhat broken, which is going to be quite painful due to number of libraries at play.

[Gozala]

Update

After doing some more exploration I realized that without type information for pull-streams utility of having type annotations in ipfs libs will be fairly limited. So I started doing flow-typed annotations for pull-streams which as it turns out has a very very very generic API since it takes 1 or 0 Source stream, arbitrary number of Through streams and 1 or 0 Sink stream and depending on combination returns either void, Sink, Through or Source and in order for type checker to be useful types need to flow through. Short version you can entype this to some degree (pull that takes up to 8 args) in flow and get a meaningful error messsages (sadly in the sea of useless output):

But in order this to work type checker needs support for existential types and type variance as described in the prior comment typescript has neither and there for it would not be able to handle type mismatches like this:

Note that transform on line 10 ends up being generic specifically it's type is <x, a> (Source<x, a>) => Source<x, a> regardless of what x and a types are. There for on line 11 error is reported as a is number due to pull.count(5) which missmatches expected string by pull.concat(). On the other hand there is no error on line 12 because because types align.

Typescript here would treated both x and a types in transform as {} and reporting errors in both instances line 11 an 12. Alternatively you could choose to use any type there, but it would not report errors neither on line 11 or 12.

I just wanted to put this out here to make it clear why I'm pushing for flow over ts. Please also note that pull-stream has quite exotic API but this issue occurs in not so exotic cases, essentially any time you return generic function who's type parameters are refined by it's usage later.

Edit: Changed second example to make my explanation somewhat more clear.

[daviddias]

Woa, tons of work and exploration here, thank you so much @Gozala 🙌🏽

This got me thinking that it would be exteremly helpful if those interfaces were not just written out in markdown but were typed as flow (or typescript) interfaces than implementation could in fact declare that class / object implements it and type checker will make sure that is the case.

💯 and also compliance tests to ensure that behavior is correct.

We have had in the past experimented with transpilation steps to support old ES5 browsers and after a while no one could bare to debug transpiled code all the way. The situation was especially bad since we have the code super modular and sometimes making a change requires you to to npm link 5 or 6 modules and so with transpilation, you would have to npm link everything and then transpile for every change. It was super painful to develop.

I saw your suggestion to add 3. Write file.js.flow along the side of implementation. and followed up on one of your PRs libp2p/js-peer-id#75 (comment).

Isn't there a way to check the type annotations on CI so that only well annotated code gets merged? This way the contributors would still be able to tinker with the code in plain JS and learn flow or get support to update the annotations just during the PR process. This would also avoid all the transpilation steps.

Adding the flow type annotations would also make doc generation way simpler. 10x💯 for that 👌🏽