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

Add TypeScript definitions #611

Closed
masaeedu opened this issue Jul 31, 2017 · 15 comments
Closed

Add TypeScript definitions #611

masaeedu opened this issue Jul 31, 2017 · 15 comments

Comments

@masaeedu
Copy link

Following on from #567, this issue tracks adding TypeScript definitions to describe the implemented JS API. This would involve creating a .d.ts file describing the public facing API and submitting it to the DefinitelyTyped repository or adding it to the minio-js module.

@brendanashworth
Copy link
Contributor

@masaeedu thanks for opening an issue. After talking with @krishnasrinivas we think we'd want to include it in this repository. Would you be interested in taking this on, or would you defer to someone else?

@masaeedu
Copy link
Author

masaeedu commented Aug 1, 2017

@brendanashworth Sure. I've been tackling this by converting all your *.js files to *.ts files, then going through it to fix all the errors due to missing type annotations; once all type errors are gone I can compile the whole project to generate an index.d.ts describing your exported API, and PR this file.

You can follow progress in master...masaeedu:master (which builds correctly despite type errors), although perhaps it is a good idea to have a PR just to allow you to verify the correctness of the annotations I am adding.

@deekoder
Copy link
Contributor

deekoder commented Aug 2, 2017

Looking fwd to your pr @masaeedu

@masaeedu
Copy link
Author

masaeedu commented Aug 3, 2017

I'm actually finding some bugs as I go through this process: e.g.

throw new errors.isValidBucketNameError('Invalid bucket name: ' + bucketName)
references new errors.isValidBucketNameError, which should probably be new errors.InvalidBucketNameError. You folks sure you don't just want to keep the converted TypeScript files?

@brendanashworth
Copy link
Contributor

Ah @masaeedu, good catch. There are perks to switching to a typed language 😄 I don't think we have any plans to switch the client to TypeScript right now though, I think we want to keep it in JS for now. Unfortunately our tests aren't very thorough so we miss some things like that. We'd love a PR that fixes it though ❤️

@deekoder deekoder modified the milestones: Future, Current Aug 17, 2017
@deekoder deekoder removed the triage label Aug 17, 2017
@harshavardhana
Copy link
Member

@masaeedu would you mind sending a PR and we can take your work from there?

@dcharbonnier
Copy link
Contributor

dcharbonnier commented Dec 13, 2017

Anytime someone check your code with typescript he discover issues in your code without even writing a test (#572 to #578), look like a good argument no ? I think you didn't check typescript enough... it's javascript it's not a weird language like coffeescript. It's just type annotations.

@krishnasrinivas
Copy link
Contributor

I think you didn't check typescript enough... it's javascript it's not a weird language like coffeescript. It's just type annotations.

Guilty of not giving a good enough thought for TS. We'll check it.

@masaeedu
Copy link
Author

@harshavardhana Sorry for not responding earlier, wasn't using the project. I've opened a PR with whatever I had as you requested, hope that is of some minimal use.

@harshavardhana
Copy link
Member

@harshavardhana Sorry for not responding earlier, wasn't using the project. I've opened a PR with whatever I had as you requested, hope that is of some minimal use.

No problem @masaeedu - thanks for all the hard work.

@rightisleft
Copy link

+1 Would love to see this completed

@barinbritva
Copy link
Contributor

PR about typings - #669. May be it will be useful.

@dcharbonnier
Copy link
Contributor

I think this can be closed, the types are available with @types.
Next step is https://github.com/canalplus/rx-player/blob/master/doc/why_typescript.md

@krishnasrinivas
Copy link
Contributor

Thanks @masaeedu @barinbritva @dcharbonnier will close this

@dcharbonnier no plans of rewriting typescript for now. https://reasonml.github.io/ looks interesting.

@dcharbonnier
Copy link
Contributor

yes, nice like transpiling golang to javascript or using coffeescript, can't wait for it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants