Skip to content
This repository has been archived by the owner on Oct 5, 2021. It is now read-only.

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

Closed
urish opened this issue Mar 20, 2018 · 23 comments
Closed

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

urish opened this issue Mar 20, 2018 · 23 comments
Assignees
Labels
discussion For direction and implementation discussions

Comments

@urish
Copy link
Collaborator

urish commented Mar 20, 2018

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 :-)

@urish urish self-assigned this Mar 20, 2018
@urish urish added the discussion For direction and implementation discussions label Mar 20, 2018
@fbartho
Copy link

fbartho commented Mar 20, 2018

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
Copy link
Collaborator Author

urish commented Mar 20, 2018

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
Copy link

tcbyrd commented Mar 20, 2018

@urish Saw your post on Twitter. You can see in probot/probot#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
Copy link
Collaborator Author

urish commented Mar 20, 2018

@tcbyrd sure thing!

@Yomguithereal
Copy link

@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
Copy link
Collaborator Author

urish commented Mar 20, 2018

@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
Copy link

Yomguithereal commented Mar 20, 2018

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
Copy link
Collaborator Author

urish commented Mar 20, 2018

@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
Copy link

Yomguithereal commented Mar 20, 2018

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
Copy link
Collaborator Author

urish commented Mar 20, 2018

@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
Copy link

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
Copy link

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

@MadaraUchiha
Copy link
Contributor

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
Copy link
Collaborator Author

urish commented Mar 21, 2018

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
Copy link

@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
Copy link

andrewjaykeller commented Mar 21, 2018

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
Copy link

rarkins commented Mar 21, 2018

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

@urish urish closed this as completed Aug 20, 2018
@styfle
Copy link

styfle commented Aug 21, 2018

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
Copy link
Collaborator Author

urish commented Aug 21, 2018

@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
Copy link

styfle commented Aug 21, 2018

@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
Copy link
Collaborator Author

urish commented Aug 21, 2018

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

@styfle
Copy link

styfle commented Aug 21, 2018

@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
Copy link
Collaborator Author

urish commented Aug 21, 2018

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
discussion For direction and implementation discussions
Projects
None yet
Development

No branches or pull requests

9 participants