mockdeep / typewiz

Automatically discover and add missing types in your TypeScript code
https://medium.com/@urish/manual-typing-is-no-fun-introducing-typewiz-58e3e8813f4c
1.1k stars 46 forks source link

Offering help to open-source projects migrating to TypeScript #35

Closed urish closed 6 years ago

urish commented 6 years ago

If you own (or contribute to) an open source project which is currently in the process of migrating to TypeScript (or planning to do so), and want some help - I'd love to help your project by adding types with TypeWiz. Just leave a comment below and ask :-)

fbartho commented 6 years ago

I'm not an owner of this, but I'm doing TypeScript and using react-navigation, and it feels very untyped / the DefinitelyTyped community supported typedefs get out of date fast. Additionally, the types that are defined have very convoluted specializations / shared structure, and discovering the correct parameters for things has been painful (for me as an individual) trying to learn the library.

I'm a big fan of the library, and have found TypeScript to be a great tool in learning how to use libraries in general.

urish commented 6 years ago

Thank you @fbartho! Can you please open an issue for them and ask them if they are interested in migrating their source code to TypeScript? If they are up to it, we can start working on a PR

tcbyrd commented 6 years ago

@urish Saw your post on Twitter. You can see in https://github.com/probot/probot/pull/372 we've already made a ton of progress to add types, but I'd definitely love to see if TypeWiz can help in some way.

urish commented 6 years ago

@tcbyrd sure thing!

Yomguithereal commented 6 years ago

@urish following up from Twitter. I recently added type definition to my mnemonist library which offers a selection of data structures. But: 1) I am not sure my use case is within what you want since I have type definitions but do not want to type the JS code itself 2) I have really tricky cases, mostly regarding generics, that would be probably hard to cover automatically - but a good use case for a tool such as typewiz, probably?. But any help would be appreciated :)

urish commented 6 years ago

@Yomguithereal nice to meet you!

Tricky cases sound fun :-)

I'm really curious - what is the reason you don't want to type the JS code itself?

Yomguithereal commented 6 years ago

I'm really curious - what is the reason you don't want to type the JS code itself?

It's actually really simple :) I personally prefer writing raw JS rather than TypeScript. I am not overly fond of static typed languages. Though they can really be useful at times. And, since TypeScript is becoming more and more popular, I started to add type definitions to my library code so it can be easily consumed by TypeScript users. Plus, in this particular case, I still have some classes I am not sure how to type correctly. What I mean is here, I still have the feeling that I am battling the type system to make really trivial JS things work finely in TypeScript.

urish commented 6 years ago

@Yomguithereal one of the things TypeWiz does is to spare you from having to type the annotations in many of the cases, so you can still write raw JS for the most part. Can you give an example for a class you are now sure how to type correctly? What is your test coverage percentage?

Yomguithereal commented 6 years ago

Test coverage percentage should be close to 100% though I did not use something to assess a precise number.

One example - and I hope my understanding of typed systems is flawed, because this would mean I am the problem :).

I have a MultiMap class which takes as argument the constructor of a class that will serve as container for the inserted items.

// Basic MultiMap
var map = new MultiMap();
// ... same as writing:
map = new MultiMap(Array);
// But if I want Sets as containers
map = new MultiMap(Set);

Now, this multimap will have types for keys and for values, expressed classically through generics:

const map: MultiMap<string, number> = new MultiMap();

But, having only those generics, type system for the map becomes a bit blurry because the get methods - and others will return something which will probably amount to any.

// Too bad, I would prefer having the correct type, Array<T> or Set<T>
const container: any = map.get();

So, would you add a third generic and do something like this?:

const map: MultiMap<Set, string, number> = new MultiMap(Set);
// or
const map: MultiMap<SetConstructor, string, number> = new MultiMap(Set);
// or even
const map: MultiMap<string, number, Set> = new MultiMap(Set);

which starts being a bit tedious and Java-y if we consider the repetition?

So I guess here, the types are the expression of the API design and I am unsure which way to select.

urish commented 6 years ago

@Yomguithereal thanks, that's a really good example!

I spent the last hour thinking about what would be the best way to implement it without having to repeat the types when instantiating the class, and here is the solution I came up with:

class MultiMap<K, V, C = Array<K> | Set<K>> {
    instance: C;

    constructor(Container: { new(): C }) {
        this.instance = new Container();
        // ...
    }
}

Basically, this would let you instantiate the class as follows:

const map = new MultiMap<string, number>(Set);

So basically zero repetition, and the type of instance will be Array<string> | Set<string>. Thoughts?

Yomguithereal commented 6 years ago

This looks like a good solution. Is this like a default generic like you would have a default function argument? I don't know about this syntax. I should check the docs once more.

It should probably be C = Array<V> | Set<V> though since the containers hold values, not keys.

I have another case like this one where I don't know the constructor beforehand, just to spice things up. But let me try things on my end with this new syntax tomorrow.

On a side note, I am curious to know what your tool would produce on such classes.

Yomguithereal commented 6 years ago

And, out of curiosity, how do you infer generics?

MadaraUchiha commented 6 years ago

I think we'll wait until TypeWiz has proper object shapes and support for multiple types, but I think we might give it an honest attempt, cc @linkgoron

urish commented 6 years ago

Cool @Yomguithereal. Basically, this is something I'm still pondering about. Right now I'm prototyping with a combination of runtime + static analysis (using the built-in type inference of TS) - you can see some initial work in #27 (there's an example of input + output with inferred generic type after running TypeWiz).

One reason I am seeking to help projects is to try to learn from this experience and hopefully generalize some of the process in order to improve TypeWiz.

mweststrate commented 6 years ago

@urish yeah Immer can be ported no problem. Note that there are already typings in the package, so you could verify the outcome. But I think TypeWiz won't get that far on produce, it is overloaded a lot, and variadic, but still would be intersting to see how far it gets. And all internal functions are completely untyped now :). Have fun, TypeWiz is really cool :)

andrewjaykeller commented 6 years ago

Hi Uri my friend!

The main module that most of the OpenBCI projects are dependent on is OpenBCI_Javascript_Utilities which uses webpack to work for both NodeJS and the Web Browser. Should the first goal be to migrate this repo?

The repo I really want to make type script is the Cyton NodeJS.

Thoughts? Thanks!

rarkins commented 6 years ago

I would be interested to explore Typescript for Renovate - https://github.com/renovateapp/renovate

100% test coverage, but zero work done so far to understand or start the process

styfle commented 6 years ago

I haven't convinced the rest of the markedjs team, but I was curious so I ran typewiz on markedjs/marked.

It's only a single file (/lib/marked.js) so I thought typewiz could infer most of the types.

But I was wrong. Only 3 lines were changed 😮

urish commented 6 years ago

@styfle Can you share the diff? We will be hacking on TypeWiz today and it will be interesting to look how to improve this

styfle commented 6 years ago

@urish I'm not sure if I'm doing it right because the instructions aren't very clear to me.

git clone https://github.com/markedjs/marked
cd marked/lib
mv marked.js marked.ts
typewize-node marked.ts
mv marked.ts marked.js
git diff

Here's the diff output

diff --git a/lib/marked.js b/lib/marked.js
index 6a71e92..219962c 100644
--- a/lib/marked.js
+++ b/lib/marked.js
@@ -898,7 +898,7 @@ InlineLexer.prototype.mangle = function(text) {
  * Renderer
  */

-function Renderer(options) {
+function Renderer(options: undefined) {
   this.options = options || marked.defaults;
 }

@@ -1296,7 +1296,7 @@ function unescape(html) {
   });
 }

-function edit(regex, opt) {
+function edit(regex: RegExp|string, opt: string|undefined) {
   regex = regex.source || regex;
   opt = opt || '';
   return {
@@ -1339,7 +1339,7 @@ var originIndependentUrl = /^$|^[a-z][a-z0-9+.-]*:|^[?#]/i;
 function noop() {}
 noop.exec = noop;

-function merge(obj) {
+function merge(obj: {}) {
   var i = 1,
       target,
       key;
urish commented 6 years ago

Thanks @styfle ! which instructions have you followed? Is there anything particular that can be improved there?

styfle commented 6 years ago

@urish Thanks for asking!

It's not clear what the typewiz-core is supposed to do.

I ended up using the instructions from typewiz-node.

The other thing that threw me off was that running typewiz-node file.js does absolutely nothing which is not very intuative. What did I do wrong as a user? Oh I need to rename the file to .ts. But there is no warning or error message.

Even running typewiz-node --help did not help me figure this out because -e script | script.ts seems like it would work with any file extension.

urish commented 6 years ago

@styfle thanks! good points. Can you please open an issue about typewiz-node file.js and another one about typewiz-node --help?