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

Feature request: Typescript definitions for 3.0 API #250

Closed
babakness opened this issue May 18, 2017 · 29 comments
Closed

Feature request: Typescript definitions for 3.0 API #250

babakness opened this issue May 18, 2017 · 29 comments
Labels
code-generation issue deals with generated code

Comments

@babakness
Copy link

Please provide Typescript definitions, it makes a world of difference. Currently the 2.0 API bindings are provided for Twilio but not for 3.0.

@natesilva
Copy link

We use TypeScript but as far as I know, Visual Studio Code uses the TypeScript definitions to provide IntelliSense (documentation, auto-completion) even if you are writing plain JavaScript. This would be great to have.

@codejudas
Copy link
Contributor

@natesilva @babakness Thanks for the feature request, we will definitely consider adding this functionality and i've created a ticket to track this work on our end. I'll keep you up to date with any developments on this front.

@ochanje210
Copy link

ochanje210 commented Oct 17, 2017

Still waiting for this..

@codejudas codejudas added feature and removed docs labels Oct 17, 2017
@codejudas
Copy link
Contributor

@ochanje210 Unfortunately we have not scheduled this work yet and our team currently does not have the bandwidth to work on this feature in the near future. This is enough of an edge feature that we cannot prioritize it over the other work we have to do.

That being said, if one of you would like to take a stab at writing typescript definitions in a PR we would be happy to review and merge and re-evaluate our position.

@codejudas codejudas added the code-generation issue deals with generated code label Oct 17, 2017
@dkundel
Copy link
Member

dkundel commented Oct 17, 2017

I'll try to take a stab at this :)

@damianesteban
Copy link

@dkundel how’s it going?

@dkundel
Copy link
Member

dkundel commented Jan 9, 2018

It's making good progress :) I have to add this into the code generation tool we use but hope to have a first version released very soon. Most typings are being generated now but I want to improve a bit more the DX :)

@omidkrad
Copy link

omidkrad commented Jan 25, 2018

Whenever a feature request to support TypeScript is closed I think of this cartoon:

No thanks! We are too busy

There is a good reason that Angular2 team decided to write the library all in TypeScript. With TypeScript they get wonderful tooling on top of their JS development, a lot of possible bugs are detected by the compiler, and it self-documents the API for them so they don't need to maintain type definitions for the library separately. TypeScript generates both .js files and the definition (.d.ts) files from the typescript (.ts) source files.

Even for a large library such as Twilio that is already written in JS, it is still possible to convert the project to TS by renaming all .js files to .ts and some regex string replacement. The benefits that the development team and the users get are going to be significant.

@omidkrad
Copy link

omidkrad commented Jan 26, 2018

I took a look at twilio-node source code, and I like how organized and clean it is written in ES5. Your codebase can be converted to TypeScript relatively easily, and given the benefits that you will get from it, I think it is worth the effort. To get started:

  • First use a tool like lebab to convert ES5 to ES6 classes.
    lebab --replace ./lib/ --transform class
  • Rename all .js files to .ts (you can use this tool)
  • Add tsconfig.json
  • gitignore *.js and *.js.map
  • Install typescript
    npm install typescript
  • Install type modules such as
    npm install @types/node @types/lodash --save-dev
  • Use regex and some manual editing to fix remaining issues

This has to be done by the twilio team because you don't want to have an external typescript fork.

Here's some regex that can help:

Using VSCode replace tool
settings:	Regex, Case Sensitive

———————————————————————————
Change prototype functions to classes
regex:		function ([A-Z]\w+)\((.*)\) \{
replace:	class $1 {\n  constructor($2) {

———————————————————————————
regex:		[A-Z\w]+\.prototype\.constructor = [A-Z]\w+;$
replace:	

———————————————————————————
Use ts export syntax for CommonJS
regex:		^module\.exports =
replace:	export =

———————————————————————————
Export const
regex:		module\.exports\.([\w]+)
replace:	export const $1

———————————————————————————
Add class closing braces
regex:		^export = ([A-Z]\w+)
replace:	}\n\nexport = $1

———————————————————————————
Use import require syntax
regex:		var (\w+) = require\(
replace:	import $1 = require(

———————————————————————————
Class inheritance
regex:		_\.extend\(([A-Z]\w+)\.prototype, ([A-Z]\w+)\.prototype\);
replace:	class $1 extends $2 { }  // TODO: merge with $1

regex:		util\.inherits\(([A-Z]\w+), ([A-Z]\w+)\);
replace:	class $1 extends $2 { } // TODO: merge with $1

———————————————————————————
Instance methods
regex:		[A-Z]\w+\.prototype\.(\w+) = function\s*(\w+)?\s*\(
replace:	$1(

———————————————————————————
Instance methods in two lines
regex:		[A-Z]\w+\.prototype\.(\w+) = function$
replace:	

———————————————————————————
Instance properties
regex:		[A-Z]\w+\.prototype\.(.*) = (.*);
replace:	$1 = $2;

———————————————————————————
Static methods
regex:		^(\s*)[A-Z]\w+\.(\w+) = function\s*(\w+)?\s*\(
replace:	$1static $2(

———————————————————————————
Static properties
regex:		[A-Z]\w+\.(.*) = (.*);
replace:	static $1 = $2;

———————————————————————————
Set constructor params as public properties
regex:		constructor\((\w+)
replace:	constructor(public $1

regex:		constructor\((public \w+), (\w+)
replace:	constructor($1, public $2

regex:		constructor\((public \w+), (public \w+), (\w+)
replace:	constructor($1, $2, public $3

regex:		constructor\((public \w+), (public \w+), (public \w+), (\w+)
replace:	constructor($1, $2, $3, public $4

———————————————————————————
Call super in the constructor
regex:		\w+\.super_\.call\(this, ([\w, ]+)\);
replace:	super($1);

regex:		\w+\.prototype\.constructor\.call\(this, (.*)\);
replace:	super($1);

———————————————————————————
Note: If in your ES5 code you are using `_name` style to indicate private properties, in TS you can use the `private` access modifier for them.

@dkundel
Copy link
Member

dkundel commented Jan 29, 2018

Hey @omidkrad!

Unfortunately it's not that easy in this case. The library is in fact not written in JavaScript but rather largely auto-generated using an internal tool that generates all of our helper libraries to keep up with the current speed of shipping features.

I agree that shipping the library in TypeScript would be better and would bring several benefits and I raised this with the respective team. For now this will be replaced with definition files though. The pull request for the code generation is open and as soon as it has been merged, you'll see a separate PR here with the TypeScript definition files coming :)

@omidkrad
Copy link

Hi @dkundel, that's great to know you're working on it. Thanks for the update.

@omidkrad
Copy link

Thinking a little more about this, I still think getting your code generator to output TS should be easier than modifying it to add a secondary output for type definitions.

For example instead of generating these two pieces:

Api.js

Object.defineProperty(Api.prototype,
  'messages', {
  get: function() {
    return this.account.messages;
  }
});

Api.d.ts

get messages(): string[];

You'll only need to output one piece of code:

Api.ts

get messages() {
  return this.account.messages;
}

cleaner and easier to maintain.

@dkundel
Copy link
Member

dkundel commented Jan 30, 2018

We debated around this and generating separate output made more sense for now considering different internal variables (incl. staffing). But moving this to TS is certainly on the radar of the actual team working on this.

Cheers,
Dominik

@yankeeinlondon
Copy link

I will be using 2.x until type definitions are available. It is a HUGE difference having this.

@julien-c
Copy link

julien-c commented May 2, 2018

Any news on this front @dkundel? We are also still using the v2.x library so that we have typings (and even those are imperfect)

Thanks!

@dkundel
Copy link
Member

dkundel commented May 3, 2018

I'm very sorry. I had to pause working on it for a bit but it's a top priority for the week of May 14th to get in the pieces. The code is largely done. But I'm not a Python developer and the code had to be written in Python 🙈

Thank you all for your patience!

@yankeeinlondon
Copy link

Thanks for the update @dkundel

@myarete
Copy link

myarete commented May 7, 2018

Super stoked for this, thanks @dkundel!!

@bradleat
Copy link

@dkundel fancy finding you here

looking forward to those typescript bindings

does it make sense to use version 2.0 for the verify product or should I wait the bindings oder?

@dkundel
Copy link
Member

dkundel commented May 11, 2018

Oh hey @bradleat!

Are you referring to using 2.0 of the helper library or do you want to use the 2.0 bindings with the latest version of the helper library. If so, I haven't tried that yet.

Cheers,
Dominik

@bradleat
Copy link

bradleat commented May 14, 2018

@dkundel

From this conversation it seems that the 2.0 help library has typescript bindings. So I'm wondering if the verify product will work fine with 2.0.

Also wondering if you still look on track to have the 3.0 bindings finished by this week?

Best,
Brad

@dkundel
Copy link
Member

dkundel commented May 14, 2018

Hey @bradleat, I just submitted the third (and hopefully last) PR to our code generation tool. Once that is merged we should be able to merge #330 which would enable it. If it looks like we won't be able to get everything in by EOW, I might be able to point you against a branch though if you want to give it a test run.

I'll keep you posted though.

Cheers,
Dominik

@nafisto
Copy link

nafisto commented Jun 7, 2018

@dkundel any updates? looks like the PR still needs to be reviewed and de-conflicted? (thanks!)

@ekarson
Copy link
Contributor

ekarson commented Jun 13, 2018

Hey @nafisto (and everyone else), we uncovered some additional problems in our code generation tool during the review process which we need to fix before we can roll out the type declarations. @dkundel is at a developer conference at the moment, so he's handing this off to my engineering team at Twilio HQ. We're hoping to get this wrapped up by the end of the month -- apologies for all the delays.

@myarete
Copy link

myarete commented Jul 5, 2018

@ekarson Any updates on this? :)

@ekarson
Copy link
Contributor

ekarson commented Jul 5, 2018

Hey @MrChristofferson, the PR I merged today is the result of rewriting the tool that generates the node library to be more type-aware -- currently just using that type awareness to generate more accurate docstrings, but it is data that I'll leverage to generate the TS declarations. Continuing to actively work on it, but I do not have a final ETA yet.

@myarete
Copy link

myarete commented Jul 5, 2018

Awesome! Thanks!!

@ekarson
Copy link
Contributor

ekarson commented Aug 23, 2018

The typescript declarations are released now, in version 3.19.2 of twilio-node. Apologies for it taking so much longer than I anticipated!

If you have any feedback when you try them out, please tag @cjcodes and @dkundel in your comments, since I'm moving on to other projects.

@yankeeinlondon
Copy link

Yay! Thanks for all the hard work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code-generation issue deals with generated code
Projects
None yet
Development

No branches or pull requests