Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integrating types to make this work with TypeScript #4

Closed
Mikerah opened this issue Jan 11, 2019 · 8 comments
Closed

Integrating types to make this work with TypeScript #4

Mikerah opened this issue Jan 11, 2019 · 8 comments

Comments

@Mikerah
Copy link
Contributor

Mikerah commented Jan 11, 2019

I brought up TypeScript support in the Libp2p Roadmap document. Raul supports having TypeScript declarations for js-libp2p. I created a repo for this purpose and also found this other repo as well. The end goal would be to merge it into js-libp2p, just like we want to do with this library.

In either case, these will need to be integrated into this repo.

Any thoughts?

cc @GregTheGreek, @ansermino

@GregTheGreek
Copy link
Member

Whats the status on this? Should we wait until we chat with the libp2p2 team?

@Mikerah
Copy link
Contributor Author

Mikerah commented Jan 19, 2019

I think we should wait until the chat with the libp2p team. I will contact Raul again to see what their availability is on their end.

@Mikerah
Copy link
Contributor Author

Mikerah commented Jan 22, 2019

During the call, there were several ideas about how to go about this. For now, we will be making bindings for gossipsub and potentially in the future, make them for libp2p.

@raulk
Copy link

raulk commented Jan 22, 2019

The issue was that as soon as you begin typing a module, you realise that that module likely uses classes or objects from another module (e.g. switch, connection manager, etc.), which ideally would be typed as well.

To avoid galavanting around all of libp2p typing everything at once, you can either use any, object, or type the element locally with only the methods and properties you are using from within gossipsub.

EDIT: As you eventually make your way around the codebase, the types you define at the origin sites will be supersets of the local types at call site, so they can easily subsume those without causing breakage.

@raulk
Copy link

raulk commented Jan 22, 2019

^^ let me know if you need an example of the above. Feels kinda abstract.

@Mikerah
Copy link
Contributor Author

Mikerah commented Jan 22, 2019

If we type the element using any or object, I think that defeats the purpose of using types and we don't get the "built-in" documentation that using TypeScript provides. By locally typing elements, do you mean something like this?

@raulk
Copy link

raulk commented Jan 22, 2019

That's an interesting approach that's enabled by typeRoots. That could definitely work -- and in fact we can go over those types at the Polkadot repo and revise them in order to, eventually, add them to the corresponding repos at libp2p.

What I had in mind was something like this. Imagine an index.d.ts file in this repo's root (names are all made up examples):

declare module 'js-libp2p-switch' {
   interface libp2pswitch {
     // lazily declare only the methods and fields you use.
     methodA(): typeA;
     methodB(): typeB;
     readonly fieldC: typeC;
   }
}

class GossipSub {
  static public readonly switch: libp2pswitch;
  subscribe(topic: string): Result;
}

But yeah, I agree that placing the ambient declarations for each imported module in a file of its own, and linking it via typeRoots is probably cleaner and better scoped.

On a related note, let's try and converge our types with Polkadot's!

@GregTheGreek
Copy link
Member

Closing since its stale

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants