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

Move TypeScript to DefinitelyTyped #16

Closed
vweevers opened this issue Feb 21, 2018 · 36 comments
Closed

Move TypeScript to DefinitelyTyped #16

vweevers opened this issue Feb 21, 2018 · 36 comments

Comments

@vweevers
Copy link
Member

Because this is about all Level projects, I'm opening it here.

See Level/abstract-leveldown#204 for background or the TLDR: Level/abstract-leveldown#204 (comment).

@Tapppi, @ralphtheninja and I are in favor. Before we discuss the how, let's give @zixia and @MeirionHughes some time to respond.

@MeirionHughes
Copy link
Member

MeirionHughes commented Feb 21, 2018

That's fair enough; we could potentially just rollback the defiantly typed commits that removed the abstract-down and level-down typings; and remove the typings entirely from all level repos. the levelup typings on definitely typed are still there, but incorrect.

I've been playing with the new inferring mechanism in typescript 2.8, but unfortunately it falls short due to not supporting function overloads. Ultimately, the way levelup is structured makes it very difficult to type and infer from any arbitrary abstract-down.

In experiencing trying to implement and distribute the typings - I agree it probably makes sense to get it done on definitely typed.

@huan
Copy link
Contributor

huan commented Feb 22, 2018

I agree we use DefinitelyTyped to manage level typings because it will give the community the maximum flexibility to maintain the types for our Level projects.

We have too much pain with writing a .d.ts separately with the JavaScript code. I always believe that if we could write all in TypeScript, the world would be better from now, and the JavaScript developer could still write ES6 JavaScript code without any problem.

Anyway, any solution would be a wonderful solution as long as it could make typing work. Let's move on. :)

BTW: We can use the discussion board of Github Org Teams for common TypeScript discussion at here: https://github.com/orgs/Level/teams/typescript

@vweevers
Copy link
Member Author

Alright, that's settled then!

It's your call when to remove the typings from Level packages. Seeing as they were experimental, I think we can do so in minor releases across the board (as if they were pre-1.0.0).

@MeirionHughes
Copy link
Member

MeirionHughes commented Feb 22, 2018

I'll strip out the generics this weekend (reduce the coupling between abstract encoder and levelup) and get some prs going, unless someone beats me to it.

We simply need function overload inference in typescript, until then people will just have to make do with a general typing or make their own interface on-top of levelup.

level, however, can be typed explicitly

@Tapppi
Copy link

Tapppi commented Feb 22, 2018

@zixia

BTW: We can use the discussion board of Github Org Teams for common TypeScript discussion at here: https://github.com/orgs/Level/teams/typescript

As a warning, that chat is private, not public. I think discussions involving changes to users should be public.

We simply need function overload inference in typescript, until then people will just have to make do with a general typing or make their own interface on-top of levelup.

level, however, can be typed explicitly

I think this sounds like a good plan. I guess level-rocksdb etc. also need explicit typings in DT or they won't work?

@huan
Copy link
Contributor

huan commented Feb 23, 2018

@Tapppi Thanks for the warning, you are right. I thought the discussion is public but it isn't.

@sqwk
Copy link

sqwk commented Mar 16, 2018

Is there a pull request for DefinitelyTyped already? Which typings should get moved/merged?

@MeirionHughes
Copy link
Member

MeirionHughes commented May 11, 2018

I wasn't really comfortable doing patch work on Defiantly Typed; potentially for me to further break things for people. :/ I think its better to clear up the typings separately, then if you still want to move them it can be all done in one go.

I've made a start anyway, see https://github.com/Level/typings

I've also made a quick branch in down, encoding, up and abstract to add a notypes branch that strips each of their own typings. then level-typings directly depends on them (github dep)

On a side note, it looks like no one else has decided to fix the DT types either; So they're still out-of-date.

@asross
Copy link

asross commented Jun 19, 2018

Any plan to address this in the near future? I'm encountering pretty major problems using this library with @types/levelup and @types/leveldown. It might actually be better to remove those libraries from DefinitelyTyped if they no longer work!

@asross
Copy link

asross commented Jun 21, 2018

I was actually able to get things working with https://github.com/Level/typings, thanks @MeirionHughes !

@MeirionHughes
Copy link
Member

For my work I'll be needing to use leveldb / rockdb, so hopefully I can pencil in some time to move this to DT. Unless someone wants to take over that aspect.

As far as I can tell the typing s are done, only need to deal with the DT package and sorting out the inter-dependencies. An alternative would be to make each package entirely self-sufficient, at which point they can just be thrown up to DT.

@huan
Copy link
Contributor

huan commented Jun 22, 2018

Good work @MeirionHughes !

Move TypeScript to DefinitelyTyped +1

And I'm always thinking about to rewrite the JS to TS... because that will make the typing very easy to generate and sync with the latest code.

@huan
Copy link
Contributor

huan commented Aug 3, 2018

Currently, we had moved all the typings to https://github.com/Level/typings, what will we expect the user to use it? I had just upgrade my dep to the latest and found that it seems no good way to integrate with our typings.

If we maintain the typings by ourselves, how about move the Level/typings to DefinitelyTyped, so that we can use @types/encoding-down for convenience?

huan added a commit to huan/flash-store that referenced this issue Aug 3, 2018
@sqwk
Copy link

sqwk commented Aug 17, 2018

Is there any guide on how to get this working with the current version of levelup/leveldown?

@danwbyrne
Copy link

Hello, I'm just getting to this discussion. I'm currently in the process of typing some external libraries we use at neo-one-suite/neo-one#419, and am curious what the status is here for moving to DT.

I see there is already a Level/typings and I'm not trying to step on any toes so if you just need someone to do the work of making the DT repo let me know and I am willing. 😃

@huan
Copy link
Contributor

huan commented Aug 25, 2018

@danwbyrne Thanks for the volunteer! I believe it is very necessary to publish all the Level/typings to the DT repo so that we can use @types/levelXXX for convenience.

Appreciate for that. @Level/typescript

@danwbyrne
Copy link

danwbyrne commented Aug 26, 2018

I've submitted a PR for abstract-leveldown, need to wait for that one to go through before I submit for the others as they depend on its typings (also I'm slow and can't submit them all at once 😂).

In the typings for encoding-down there is this import:

import { CodecOptions, CodecEncoder } from 'level-codec';

but level-codec is not on DT and has its own typings included with it. What would you guys at Level like to do about that? We could put level-codec types on DT as well, or find a way to import the types into the DT types that need them. https://github.com/Level/codec/blob/master/index.d.ts

@zixia @vweevers

@vweevers
Copy link
Member Author

level-codec [..] has its own typings included with it.

That's a leftover, should be moved out of Level/codec.

@ralphtheninja
Copy link
Member

Level/codec@1cfd23f

@wookieb
Copy link

wookieb commented Sep 11, 2018

I was redirected here from the issue: Level/encoding-down#67 (comment)

I'm very sad that you've decided to move typings out of repository. It very error-prone in terms of synchronization of the api. Even now its out of sync since it doesn't say that methods return a promise
https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/levelup/index.d.ts
P.s. I'm not going to update it since it's counter productive and effects of my work might be outdated any time.

I understand the reason that many of maintainers don't know typescript but that's the real problem. Developing nowadays almost anything in node without typescript is very error-prone, not reliable, much slower and much harder.

I'm very sad that that you're throwing away so many valuable traits instead learning typescript basics within an hour.

I was happy to rewrite few libs to typescript as it should be fairly easy, but if you don't want it and not willing to change the opinion about typescript then OK.

@danwbyrne
Copy link

danwbyrne commented Sep 11, 2018

@wookieb I think you are misinformed. From my understanding both of the declaration files for @DefinitelyTyped/levelup and @DefinitetlyTyped/leveldown are incorrect and extremely deprecated.

The ONLY Level packages that have actually been transferred from Level/Typings as of now are abstract-leveldown and level-codec with the rest to follow in the coming weeks.

Hopefully that clears up the situation, I also noticed the levelup/leveldown typings in DT but as far as I can tell by their contents they are NOT correct nor should you attempt to use them with a recent version of the packages.

THAT BEING SAID, I also can't guarantee that the typings ive moved over are completely correct, so I would really love help or at least another set of eyes on them. If @MeirionHughes is around I am curious about why you chose an interface structure for your declaration file (specifically abstract-leveldown) instead of creating Class declarations? I see that it allows you to create new objects with new and with (...) but is that necessary? I'm not entirely familiar with the library.

@wookieb
Copy link

wookieb commented Sep 11, 2018

@danwbyrne You're right about DefinitelyTypes but that doesn't matter where the typings are. What matters is that you're creating them manually which has all downsides I've mentioned above.

Hopefully that clears up the situation, I also noticed the levelup/leveldown typings in DT but as far as I can tell by their contents they are NOT correct nor should you attempt to use them with a recent version of the packages.

How I was suppose to know that? :)

I also can't guarantee that the typings ive moved over are completely correct, so I would really love help or at least another set of eyes on them

Same as with fixing typings in DefinitelyTyped. That doesnt make sense for you to update typing manually as they might be outdated pretty quickly.

@vweevers
Copy link
Member Author

@wookieb For keeping typings in sync, it doesn't matter whether they reside in Level or DefinitelyTyped. Either way someone (preferably a team of people) will have to keep them updated. You can call that "manual work" but there is no alternative. I believe we (the JavaScript maintainers of Level) have been very open to supporting TypeScript users however we can, realizing that this is a big audience. I hope that you can return the favor and realize that there is a big JavaScript audience.

@wookieb
Copy link

wookieb commented Sep 12, 2018

You can call that "manual work" but there is no alternative.
The alternative is: Rewrite to typescript. The alternative is javascript with a lot of typings synchronization problems that every popular js library has.

It's not a joke. Please don't get me wrong. I'm not a Typescript fanboy that is coming here to spread "typescript" disease. I'm talking about real solution for real problems. Developing anything in node.js without typings is a real disease. Developing with wrong typings is even more annoying and that happens really really often.

I hope that you can return the favor and realize that there is a big JavaScript audience.

Not sure what are you trying to say.

@vweevers
Copy link
Member Author

I'll rephrase: rewriting to TypeScript is not an option because many people are perfectly happy with JavaScript. That's all.

@wookieb
Copy link

wookieb commented Sep 12, 2018

I know. That's why I'm not insisting anymore.

@huan
Copy link
Contributor

huan commented Sep 13, 2018

Hi all,

In the past 12 months, we had faced lots of issues with the TypeScript type definition, but it had not fixed well and the users still cannot work with the right typings after we spent lots of time on it (thanks @MeirionHughes , et al)

Rewriting to TypeScript will fix this problem. However, @vweevers is very happy with the JavaScript and always against to do that.

After some thoughts, I think we have a new direction to make everyone happy.

In short: let's create a new npm module that supports TypeScript natively.

The following will be my purposal:

  1. Create a new repository, like: level/levelmap
  2. levelmap will use levelup as its backend, expost all API as a ES6-Map-like
  3. Develop in TypeScript
  4. publish to npm as levelmap, and suggest TypeScript users to use this module.

The above is my 2-cents, please feel free to contribute any good ideas. If we can get a agreement about it, then I'd like to create the repository, develop it(with portional collabrators like @wookieb ), and publish it to npm as a level alternative for TypeScript users.

P.S. Actually, I had already build a module like what I had purposed at here, I used it for more than a year and it can fullfil all my requirements. See: https://github.com/zixia/flash-store

@vweevers
Copy link
Member Author

the users still cannot work with the right typings after we spent lots of time on it

I believe it simply isn't done yet? Someone has to see it through, instead of changing direction (again).

As @danwbyrne said (emphasis and applause mine):

The ONLY Level packages that have actually been transferred from Level/Typings as of now are abstract-leveldown and level-codec with the rest to follow in the coming weeks. 👏

@danwbyrne
Copy link

danwbyrne commented Sep 17, 2018

Okay! All of the types in https://github.com/level/typings have been at least PR'd to DefinitelyTyped. If you want to track them all in one place here they are:

I still don't fully support the way these are typed, but I unfortunately don't have the spare time right now to go through them all. You can see I change the pattern of the declarations on encoding-down and rocksdb which is how I think they should all look honestly.

I'll continue to monitor the repos if anyone else decides to contribute to the types, but that does it for me here :)

@vweevers
Copy link
Member Author

Many thanks @danwbyrne!

huan added a commit to huan/flash-store that referenced this issue Dec 29, 2018
@MeirionHughes
Copy link
Member

MeirionHughes commented Jun 20, 2019

Just had another look at typing this beast... typescript still can't deal with inferring derived generic parameters. i.e. :

type InferKey<DB> = DB extends AbstractDB<infer K, any> ?
  K :
  any;

interface AbstractDB<K = any, V = any> {
}

interface SpecialDB<K, V> extends AbstractDB<K, V>{
}

let db: SpecialDB<string, Buffer>;
let A: InferKey<AbstractDB<string, Buffer>>; // string type
let B: InferKey<SpecialDB<string, Buffer>>; // unknown type

@danwbyrne belated thanks for getting it up to dt... I've never liked the typings either. :P

@reuzel
Copy link

reuzel commented Dec 16, 2020

The error in the callback of the level(up) get operation should be extended. Typescript complains that the 'notFound' or 'type' properties do not exist on plain Error objects. You would need that to check if there is actual failure or if the record is simply not found in the database...

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