Definition State

[blakeembrey]

So I spent the day typing this by hand and am currently around 1000 lines and a third of the definition through. However, much of it is likely incorrect in some way and will need refactoring anyway. Then I run across this and it looks fairly complete and would like to learn more.

Is/was it all automatically generated? What do you think the main issues are? Any reason for noImplicitAny failures? Would you want to collaborate in @types on the definition? Can the original script/scraper be shared?

[niieani]

Hello Blake!

This was initially automatically generated from the file that the RethinkDB docs. The ugly, rudimentary generator is in the repo.
However the automation proved to be a partial dead end - there are many typos and inconsistencies in the way RethinkDB's documentation is done, so I had to add many regex hacks to automatically fix it. However automation might be useful for seeing the diff between versions. I've put the autogenerated file in the autogenerated branch, so technically we could re-run it with the latest reql_docs.js file to add the missing features that were added since the end of last year.

In the end I've analyzed all the internal types of RethinkDB, matched up the common and differing implementations of various methods in the various types and rewrote the typings by hand, partially copy & pasting the autogenerated ones. Unfortunately even that has proven to be really hard, as there are many features in the API of RethinkDB that TypeScript just does not yet support. Those features would make this endeavor much simpler, and in the case of some - at all possible:

• Allow skipping some generics when calling a function with multiple generics microsoft/TypeScript#10571
• Type's generic type constraint on member level microsoft/TypeScript#1290
• Polymorphic this and Generics microsoft/TypeScript#6223
• Suggestion: Type Property type microsoft/TypeScript#1295

Then there are also some things like RethinkDB's varar arity that will never be supported (microsoft/TypeScript#6755) and require hacks.

See this issue for more details see this and the following comment: DefinitelyTyped/DefinitelyTyped#4551 (comment).

I've put this project on hold pending at least microsoft/TypeScript#1295 implementation that will make this much more useful.

The typings in this repo are useful as they are, but not 100% complete.
The main issue is the lack of support for group and ungroup, which was too complicated to support, and some other errors which came out of improper/imperfect documentation.

Coming with TypeScript 2.0 there's now a nice workaround for Type's generic type constraint on member level microsoft/TypeScript#1290 (comment) which would maybe make it possible to implement group and ungroup. But I didn't have the time to try and tackle this yet.

We also need more tests, since the ones available are only using the simplest examples. Using the typings in practice, I've stumbled upon many cases which required me to cast parts of the RethinkDB call to any, because of the incomplete implementation.

The typings are relatively well organized, and if you use a code editor with folding support you can easily see which interface implements which methods in what way. If you want to help out with this effort I'm happy to add you as a collaborator.

[niieani]

The current input file for the old generator can be found here.
Although the most up-to-date file can be generated by running:

_scripts/dataexplorer.py from the rethinkdb documentation in http://github.com/rethinkdb/docs

Regarding your question:

Would you want to collaborate in @types on the definition?

The answer is yes - in fact that was the aim of DefinitelyTyped/DefinitelyTyped#4551 all along. We didn't publish the typings to DefinitelyTyped (me and @ronzeidman), because of the unresolved issues that are still present.
As for noImplicitAny - I don't know, as I didn't check.

[blakeembrey]

Thanks for the detailed answer. I'm not too worried about the shortcomings, we can eventually improve those. The biggest issue I run into when trying these were actually incorrect types - the obvious ones were around document manipulation and .do(function (row) { return row('x') }) isn't currently possible and looking into the definition it's actually hard to see where this functionality should be add. If you're able to point in the right direction, I can try the changes. I'm thinking I'll continue with my current effort for a bit and use this repo for reference. In whichever case, I'd like to add you to the @types implementation eventually if you'd like to help out 😄

[niieani]

Do you have a specific piece of code where you cannot run do on? The do method is implemented in RAny interface which is almost all interfaces extend.
Implementation here.

Would you mind sharing your definitions in their current, incomplete state? I'm curious how you solved certain problems like the varar arity, group/ungroup, type-loss in chains, etc.

[blakeembrey]

Mostly I've run into document manipulation issues with the () notation (selecting the field to use). I haven't tested my definitions, was only typing them from scratch - I was previously using any. I definitely have broken some types, but I think I have a better solution now after going through the docs a lot and then looking at this implementation also. For instance, I think the return types could be generics to avoid some code/documentation duplication. I've also found that my initial implementation following the programmatic interface (which loads everything under RDBOp) is terrible and I'll need to split those apart. I've also run into a lot of interface issues (E.g. leftBound, rightBound usages, etc). I'll be sure to share my definition when I am able to replace any in my current project 😄

[blakeembrey]

@niieani This is what I've ended up on. I ended up tossing away my definition and using yours. I refactored it continuously for the last day (or two) until I had gone over all the methods. Toward the end, I realised I needed to delete the abstract Sequence type to make things work, and I also had to introduce some addition types (I went for RDatum since that's the return from most accessors). Although not 100% correct, it makes it easier to work with.

Edit: I also removed a couple of other types, and haven't come back to visit the Selection<Array> vs Selection<Stream> distinction.

[niieani]

OK, that's interesting.
The Array vs Stream Selection distinction is important, because certain methods have different signatures based on the underlying type, especially, they will result in different outputs. Also, Streams have more methods than Arrays.
Would you mind pushing your changes somewhere so we could collaboratively work to get this working well?

[blakeembrey]

@niieani Sorry, I thought I put the link in there 😄 It's http://github.com/types/npm-rethinkdb. Both RStream and RArray are still separate, I was referring to Selection<Array> and Selection<Stream> from the data types docs in RethinkDB (https://rethinkdb.com/docs/data-types/). I found them helpful to understand the structures I needed. The abstract Sequence type was deleted because there's too many methods that use a stream/array/selection and go to a different type.

[blakeembrey]

I also discovered that there's native result, Array results (which the interface is similar to cursors) and Cursor. And ArrayResult actually extends Array internally so Array.isArray(ArrayResult) && ArrayResult.toArray() both work. It confused me for a little while after this refactor to understand how my code worked before and how it could possible work now with this change - it just turned out I was using the array types and as a cursor (but never close()).

[niieani]

Looks good.
I'll try to throw it into some of my code bases using RethinkDB and let you know if or where it crashes ;)

[blakeembrey]

@niieani Awesome, cheers! Definitely log plenty of issues.

We can move any discussions there and happy to log lots of issues for everything encountered. The biggest issue (or non-issue) I've been facing is using the definition on 2.0 with strictNullChecks. I'm not sure if most the types should actually be unions with | null once 2.0 comes out. What do you think? Err on the side of correctness (could be null when using get()) or allow the user to override strictness (the current way I'm doing it is r.table<User | null> for instance).

[blakeembrey]

@niieani By the way, the script I was hoping for was

rethinkdb-typescript-definition/parser-documentation-to-ts-d/reql_docs.ts

Line 1 in ef621d4

// The contents of reql_docs were generated by _scripts/dataexplorer.py from the rethinkdb documentation in http://github.com/rethinkdb/docs

. If you don't have it, I can quickly knock one up in node. As you said, it's probably a great way to automatically check for changes in docs/interfaces while we then apply the changes more strategically.

[niieani]

Since null is a valid return from the DB in case of a non-existent record, I think we should have it baked into the definition. This will ensure the user checks for null before trying to access the object.
Not sure which script you mean that you were hoping for? 🐶 The JSON docs are generated here from the markdown files. The file is used by the autocomplete in the RethinkDB admin panel, you can see they regenerate the file every now and then here. Might be useful to see what's changed since November 2015 and add those missing features to the typings.

[blakeembrey]

Ah, thanks! I somehow didn't realise it was part of the RethinkDB org, cheers! Yeah, you're right about null - I'll fix that 2.0 support lands.

[Extarys]

Any updates on those fixes? :P Or any idea how to use the lib in typescript without that?

[niieani]

This repo has been deprecated by https://github.com/types/rethinkdb. Please see that for up-to-date typings instead.