Add TypeScript type definitions

[enko]

This allows a better integration into new TypeScript projects

[dominictarr]

I'll merge these, but is it some way to have test coverage for wether these are correct? this code may change, and I don't usually use typescript.

[enko]

@dominictarr I will add some tests

[enko]

@dominictarr I added tests by duplicating index.js, converting it to typescript and integrating it in the existing test suite, I hope that is ok for you?

[evbogue]

@enko For those of us who don't code in TypeScript, would you be able to explain what this code does, why we're adding it, and what problem does it solve? Or link to a relevent article on the subject.

This module handles public/private key imports, and thus is very important to keep simple for the sake of security. It's important to know why we're adding to it.

[enko]

@evbogue I added a TypeScript definition file. All JavaScript code has some sort of implicit types, sometimes more, sometimes less and the definition file puts the theses types in a form of contract. So if program against the library and use TypeScript, the compiler can check if I follow the type definition that I have layed out here and used the correct types.

Having a definition files also helps at development time, as it helps your ide to better understand the code, even if you use javascript. You can use typescript as a backend for the language server protocol and get better completion for javascript. There are implementations for the lsp for emacs, vim, sublime and Visual Studio Code has it's own implementation integrated.

I started with ssb-client, because I wanted to do experiments with angular and ssb, but the function that creates the client takes as first parameter the key object, so I went for ssb-keys as the definition for the key interface should be here. I have also added a definition for ssb-config but did not create a pull request yet. I will add typings as I see fit from library to library as I go on in the ssb universe.

In my opinion the whole project would benefit from typings, as it promotes thinking what implicit types are used and make them more explicit and also helps with development.

[AljoschaMeyer]

@enko: Fyi, @staltz has some typescript definitions here

[evbogue]

I did a little reading about TypeScript, because I've never read about it before. I think the point is that it asks you to be explicit about what types of data you are using.

Do we need to modify every module in scuttlebot to use TypeScript with it, or is it possible to use an external module similar to what @staltz is doing with https://github.com/ssbc/ssb-typescript ?

[enko]

@evbogue You are defining how the module is typed, so you need to put the typings inside the module. Having the typings in one module like ssb-typescript would require work everytime you bootstrap a project, copy files from the ssb-typescript repo into your local typings folder and keep it up to date. This also does not work with the aforementioned ide support. So the best way is to include typings in every module.

[staltz]

Hi! Chiming in: this PR is useful for people who write code in TypeScript, and makes no difference for people who use plain JavaScript. People writing code in TypeScript and importing ssb-keys can now be sure they are using the API correctly, and not giving the wrong arguments that will cause crashes, e.g.

ssb-typescript on the other hand is for a different concern, it validates common SSB messages, so it covers the message object shapes, but not an API like ssb-keys does.

[enko]

And to clarify things, you do not need typings to use a library in typescript, it just makes things better.

[enko]

@staltz wrote:

and makes no difference for people who use plain JavaScript

If you write plain javascript and use an ide which supports the language server protocol, you can still benefit from the typings, the server can guess from the require statements what your types are and give you better completion options and warn you about wrongly used types (See here: https://code.visualstudio.com/docs/languages/javascript)

[evbogue]

Ok, I get why this is useful for TypeScript programmers. Mergeable! Thanks for taking the time to answer my questions @enko + @staltz

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[enko]

If I would fix the merge conflicts and look over it again, would you still be interested in patches?

[dominictarr]

@enko sure. btw, how do the tests test? by running the tests they just ensure that the internally used types are correct? like, can't you run a check that the types are work without running the tests...

I'd also advise doing this some other way than just duplicating the tests. That adds a maintenance burden. I like to have reusable tests... but I don't see anything that is different about the tests expect es6 style import and .ts file extention? is it possible to refactor the tests so that you can pass in the thing to be tested (this is the usual approach that I use) and ... I guess run it with typescript instead of node?

[staltz]

Quick comment on running tests that are written in .ts files: ts-node is an npm package that wraps node.js with TypeScript's type checker, and sometimes I have used that in combination with mocha like this: mocha test/*.ts --require ts-node/register

[enko]

Hey people!

I rewrote the typings and made them a bit more isolated and also wrote tests that don't use tape but dtslint.

Have a look and tell me what you are thinking.

love

Tim

[stale[bot]]

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

[staltz]

This PR should be merged, I'm inclined to merge it as is. If the tests structure is debatable, we could just add the .d.ts file, and then later consider a separate PR just to add tests for the types.

[dominictarr]

@staltz I don't really know type script, if you want to merge, please do so.

[staltz]

@enko If you are still willing to update this PR, I would recommend we remove all the changes that are not absolutely required for adding TypeScript types, such as dtslint, types/test.ts, types/tsconfig.json, etc. Tests are good, but in this case it is mostly just checking whether the output type of a function is what we expect it to be, which is just restating what the .d.ts file said, so it turns out to be not so valuable.

Also editorconfig is nice but it's not strictly needed for this PR, and we can gladly open another PR for editorconfig (and maybe discuss adding it uniformly on all ssbc repos, uniformity is important). Essentially, for this PR we just need types/index.d.ts and it could be moved to ./index.d.ts.

Finally, I believe one commit is enough, so you could rebase, or I can do it as a squash commit upon merging.

[dominictarr]

@staltz clarify: I asked for tests, so that I didn't break types in future changes. (happy to defer to your judgement here though)

[staltz]

@dominictarr Ah, right, sorry I missed that context. The challenge here is that because the implementation is in JavaScript, it's possible to change the JS implementation so that it mismatches (hence breaks compatibility) with TypeScript. The tests that were added don't protect us from that risk. The way of ensuring JS-TS will stay in sync is to write the implementation in TS, which I don't know if you want to do. So adding just a .d.ts file and hoping for the best is actually the standard way of adding TypeScript types when the library's implementation is not in TypeScript.

[dominictarr]

@staltz actually I've been thinking of maybe porting some parts of ssb.js to type script, to make porting to other typed languages (rust, go) more straightforward. Obviously there are a lot of node.js-isms, I'm not considering typing the glue code but just the parts that implement algorithms and have well defined behaviour. Especially ssb-ebt and ssb-friends I guess ssb-keys could be included, and probably ssb-validate too. All that stuff is written so that IO is separated, it's just data structures/algorithms that should be easy to port.

[staltz]

@dominictarr This project may interest you https://github.com/AssemblyScript/assemblyscript

[MaxGraey]

@staltz See wasm-crypto from Frank Denis

[dominictarr]

@staltz wow, that look cool!

[stale[bot]]

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

[staltz]

@enko Would you still be interested in updating this PR?

[stale[bot]]

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

[christianbundy]

Could we just use the .d.ts file and then add a test command like:

tsc --allowJs --resolveJsonModule --lib dom --checkJs --noEmit --skipLibCheck *.js

This is what I'm doing for sanity-checking Oasis and it's working pretty well so far.

[christianbundy]

This is the oldest open PR in the SSBC and I'd like to either merge or close. Anyone have opinions? I'd like to merge the code and improve it later if we care, but I'd be comfortable closing if that's what others prefer.

[staltz]

We could close.

[christianbundy]

Doing the thing, please anyone feel free to reopen.