Typescript plans? Need help?

[jskrzypek]

@markandrus I noticed in #136 that you mentioned you may be planning to move to typescript at some point. I am thrilled to hear it!

I've been maintaining a twilio-video declaration file for use in our angular apps as we go along, building off of your jsdoc comments (I was actually in the process of updating it when I stumbled across your comment in #136).

I am curious, is the typescript thing still on the table and do you have a timeline for when you might switch? (I'm certainly eager for it since it would mean that we get our declarations out of the box rather than ad-hoc 😄).

If I can be of any help with the rewrite, please let me know!

[markandrus]

Hi @jskrzypek,

@markandrus I noticed in #136 that you mentioned you may be planning to move to typescript at some point. I am thrilled to hear it!

This is something @manjeshbhargav and I have talked about often. Yes, we'd like to!

I've been maintaining a twilio-video declaration file for use in our angular apps as we go along, building off of your jsdoc comments (I was actually in the process of updating it when I stumbled across your comment in #136).

Nice 👍 Mind linking it? Others may find it helpful.

I am curious, is the typescript thing still on the table and do you have a timeline for when you might switch? (I'm certainly eager for it since it would mean that we get our declarations out of the box rather than ad-hoc 😄).

Yes, it's on the table. We've planned the migration in pieces, roughly

1. Land tsconfig.json and tslint.json in the project. We use TypeScript internally for some projects, so we have some of these that fit the style of our codebase already. We should copy these over.
2. Start by converting internal modules in lib/. Likely we'll move .ts files to the src/ directory and place the compiled .js in lib/. We tried this work out during a hack week a while back and found that starting with internal modules with the fewest (if any) dependencies and working out was the best way to go.
3. Convert public API modules in lib/. Same process as above, except now we'll be converting public APIs to TypeScript. See challenges below.
4. Convert unit and integration tests under test/. We'll have a lot of these to convert. 😬
5. Create/ship the TypeScript declarations. I'm not sure if this needs to be a separate step or not, since our team hasn't shipped a library with TypeScript declarations. Maybe it comes in Step 3? At any rate, it introduces some challenges (see below).

Challenges

• AFAIK, once we move to TypeScript, you can't just pin to a twilio-video.js git ref. For example, if you wanted to build against master (not recommended!) you could do this

  {
    "twilio-video": "twilio/twilio-video.js#master"
  }

  Unless we get clever with our install steps, this won't work with TypeScript components since they have to be compiled.

• Once we ship TypeScript declarations, these become part of our API contract (at least for TypeScript users). There are two challenges with this:

  • Some reorganizations of our class hierarchy in the 1.x releases were only possible because JavaScript is dynamically typed. TypeScript would have forced us to make a breaking change where we would have preferred not to. Counterpoint: maybe this isn't so bad, and is just good hygiene? (after all, it is what our Mobile SDKs had to do, and the reason they are on 2.x now.)

  • The TypeScript compiler doesn't follow SemVer, so our TypeScript declarations may only work with a particular TypeScript release, e.g. 2.6.x. We may need to advertise the particular TypeScript release our typings are compatible with, and new TypeScript releases may break builds.

Short-term Alternatives

Short of fully converting to TypeScript, we could also consider maintaining some declarations ourselves. We might have some of the same challenges as above WRT API contracts.

---

@jskrzypek any input from your or the community would be great 👍

[jskrzypek]

Hey @markandrus I'm glad to hear that you guys are interested in the switch. I have some comments/feedback on your reply, I'll lay them out inline.

Nice 👍 Mind linking [the declaration files]? Others may find it helpful.

Sure, I'll try to throw up a gist this week sometime.

1. Land tsconfig.json and tslint.json in the project. We use TypeScript internally for some projects, so we have some of these that fit the style of our codebase already. We should copy these over.

This definitely comes in handy and it can be good to help maintain a common style across the company.

2. Start by converting internal modules in lib/. Likely we'll move .ts files to the src/ directory and place the compiled .js in lib/. We tried this work out during a hack week a while back and found that starting with internal modules with the fewest (if any) dependencies and working out was the best way to go.

That's an interesting idea. So the there would be to have the un-converted files just use the compiled output of the converted ones?

3. Convert public API modules in lib/. Same process as above, except now we'll be converting public APIs to TypeScript. See challenges below.
4. Convert unit and integration tests under test/. We'll have a lot of these to convert. 😬

You can also leave these as-is. Tests don't strictly need to be in .ts even when sources are, and the tests ought to be run against the compiled code anyhow. You could easily get away with only upgrading tests as you need to patch/replace them, as they don't actually need to consume the source .ts files as long as your compilation emits a compiled file .js file per source file

5. Create/ship the TypeScript declarations. I'm not sure if this needs to be a separate step or not, since our team hasn't shipped a library with TypeScript declarations. Maybe it comes in Step 3? At any rate, it introduces some challenges (see below).

This is actually the easy part, when you run the typescript compiler, it produces the declarations for you, and then you simply include a "types": "./index.d.ts" entry in your package.json that tells the compiler where to find the types for your package. I'll respond to the challenges below.

Challenges
AFAIK, once we move to TypeScript, you can't just pin to a twilio-video.js git ref.

Actually I believe this is not so difficult to do, as long as you commit the compiled code along with the git. It does perhaps get tricky to ensure that the .js code from a given commit was actually compiled against the .ts source from that commit, but there are ways to manage.

One of easier ways I can imagine to do this that would be to implement a sort of nightly/mini-release process where you make a chore commit with the compiled code at that state. You can use a specific format for the commit message and also tag it so it is easy to reference directly. For the rest of the commits you don't worry about obeying this contract.

Some reorganizations of our class hierarchy in the 1.x releases were only possible because JavaScript is dynamically typed. TypeScript would have forced us to make a breaking change where we would have preferred not to. Counterpoint: maybe this isn't so bad, and is just good hygiene? (after all, it is what our Mobile SDKs had to do, and the reason they are on 2.x now.)

Good point & counterpoint. I think there is something to be said for a version bump/breaking change, lots of projects have turned the switch to TS into an opportunity for a major version change with a breaking of previous contracts. That question specifically seems like one you need to answer internally as also involves thing like customer eduction, internal APIs, and branding. I do think that since the native SDKs have jumped to 2, it could make sense to follow suit for a TS refactor, even if you do keep the same APIs, simply to signal that the library is taking a somewhat different direction.

The other consideration of being stuck with a specific contract for the whole lib is not actually the case as long as you type your library correctly. You can have your whole project be strongly typed without tight coupling between internal and external classes. The way to maintain this separation is to create interfaces containing the public methods and properties you want consumers to use. If you consume/re-export the interfaces from the public-facing classes & modules, then you don't expose the internal/private classes that implement those interfaces. Given that, as long as the public contract of the interfaces are kept, the internal classes and APIs can change as much as you want.

As another thought on that topic, it could even make sense to do this for the public-facing classes/APIs first, as this you might help ensure a separation of concerns for the public and private APIs of the library, since the interfaces would simply describe the contract fulfilled by the internal javascript components.

The TypeScript compiler doesn't follow SemVer, so our TypeScript declarations may only work with a particular TypeScript release, e.g. 2.6.x. We may need to advertise the particular TypeScript release our typings are compatible with, and new TypeScript releases may break builds.

This is potentially a challenge, agreed. But I think it's one that the whole community is having to deal with. One alternative could be to provide additional declaration files compiled against older TypeScript versions, but that's probably just additional work for little-to-no reward. If this is a concern for consumers of twilio-video.js, it would make more sense to simply compile the whole lib against an older typescript version and publish that, as the compiler is supposed to be compatible with older declarations, even if the declarations don't work with older compilers.

Short-term Alternatives

Short of fully converting to TypeScript, we could also consider maintaining some declarations ourselves. We might have some of the same challenges as above WRT API contracts.

This should be relatively easy to do, given your extensive use of jsdoc, and I'll be happy to donate my declarations to the effort if desired.

[omidkrad]

related: twilio/twilio-node#250 (comment)

[markandrus]

Thanks, @omidkrad—I've started attempting the mostly mechanical translations on another branch. I realized that at least one of the lebab transforms, --transform destruct-param, is really unsafe. At least on this code base. Luckily a unit test caught it, but I will need to be more conservative in converting.

[omidkrad]

That's great @markandrus. If you are using vscode, the ">Format Document" command may come handy. 😉 Also I've noticed lebab will recognize class inheritance from util.inherits(A, B) and convert it to class A extends B, so if you are using _.extend(A.prototype, B.prototype) replace it with util.inherits(A, B) and then run lebab with --transform class to convert those.

[alalonde]

@jskrzypek Care to share your typings? I'd love to slap a few in my project.

Until then, here's a starter. This was just the minimum to cover the API surface area I was using; I'm sure yours are more complete 😀

[darioblanco]

Hi guys, we also use TypeScript and I was a bit annoyed that this library had none.

I have just created @types/twilio-video and I am constantly keeping an eye and updating the types. For now it works quite well and it was not so difficult to create typings, as the documentation in https://media.twiliocdn.com/sdk/js/video/releases/2.0.0-beta1 is quite good.

I just focused on twilio video 2.0, sorry! I also encourage the twilio team to create their own typings in this library, as it really was not a big deal after looking the docs.

Hope it helps!

[jskrzypek]

Hey @alalonde here's what I have for an older version, I think 1.10.0. Sorry for the delay 🙏

https://gist.github.com/jskrzypek/b6478881fc3bda887c60c9b37a1245c7

@darioblanco if you want to take these and use them in a pr/maintain them definitely feel free

[absin1]

@darioblanco I did the following:

npm install twilio-video --save
npm install @types/twilio-video --save
Then inside my component, I do
import * as Video from 'twilio-video';
and
Video.connect(data.token, connectOptions)
I get this error:
index.js:20 Uncaught ReferenceError: global is not defined at Object../node_modules/twilio-video/es5/util/insightspublisher/index.js (index.js:20) at __webpack_require__ (bootstrap:78) at Object../node_modules/twilio-video/es5/signaling/v2/transport.js (transport.js:13) at __webpack_require__ (bootstrap:78) at Object../node_modules/twilio-video/es5/signaling/v2/cancelableroomsignalingpromise.js (cancelableroomsignalingpromise.js:6) at __webpack_require__ (bootstrap:78) at Object../node_modules/twilio-video/es5/signaling/v2/index.js (index.js:12) at __webpack_require__ (bootstrap:78) at Object../node_modules/twilio-video/es5/connect.js (connect.js:21) at __webpack_require__ (bootstrap:78)
What am I missing?

[darioblanco]

@absin1 Are you installing the twilio2.0.0-beta1? I haven’t created the types for 1.x but I would gladly give support to that version if needed. Maybe with the VideoV1 namespace.

Can you try if it works for you with 2.x?

Edit: oh! i see that @jskrzypek created some types, I will take a look and see how they can be integrated

[jonckvanderkogel]

@darioblanco I did the following:

npm install twilio-video --save
npm install @types/twilio-video --save
Then inside my component, I do
import * as Video from 'twilio-video';
and
Video.connect(data.token, connectOptions)
I get this error:
index.js:20 Uncaught ReferenceError: global is not defined at Object../node_modules/twilio-video/es5/util/insightspublisher/index.js (index.js:20) at __webpack_require__ (bootstrap:78) at Object../node_modules/twilio-video/es5/signaling/v2/transport.js (transport.js:13) at __webpack_require__ (bootstrap:78) at Object../node_modules/twilio-video/es5/signaling/v2/cancelableroomsignalingpromise.js (cancelableroomsignalingpromise.js:6) at __webpack_require__ (bootstrap:78) at Object../node_modules/twilio-video/es5/signaling/v2/index.js (index.js:12) at __webpack_require__ (bootstrap:78) at Object../node_modules/twilio-video/es5/connect.js (connect.js:21) at __webpack_require__ (bootstrap:78)
What am I missing?

any update on this? I am also getting the webpack_require (bootstrap:78) error (actually for me it's at line 83)

To add some info to this, I currently do have it working with Angular 5 (so thus TypeScript) and ng-bootstrap 1.1.2. When I tried upgrading to Angular 7 and ng-bootstrap 4 it broke.

Any help would be greatly appreciated!

[darioblanco]

Which version of twilio-video are you using @jonckvanderkogel ?

You should do npm install [email protected] for instance if you want to use the types

[darioblanco]

Regarding the types for V1, if you want to add them (for instance in a namespace called VideoV1 or something similar), we can definitely do it here: https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/twilio-video

Just an MR and I would be happy to review or contribute. I sadly don't have much time to test the V1 types :(

[jonckvanderkogel]

Which version of twilio-video are you using @jonckvanderkogel ?

You should do npm install [email protected] for instance if you want to use the types

@darioblanco I have actually resolved the issue by adding the following to my polyfills.ts:

(window as any).global = window;

After adding that line Twilio Video once again works as expected.

Thanks for your help!

[zbagley]

@darioblanco you and @minddocdev @ktsn are the true MVPs on this one. Thanks for making the dev process that much faster for me/my team.