TypeScript conversion

[garyhuntddn]

TypeScript conversion and related pull requests

I suggest we have a single issue to collate issues around the conversion work I'm planning on doing (for anyone else - please feel free to help out).

I've got 30+ PRs prepared for a bunch of simple conversion from .js to .ts for the constants and the likes.

I've also written a policy document in a separate PR which needs a good review to see if you agree with me etc.

I'm also tempted to write a whole load of brittle tests (brittle because they test implementation rather than function) around each of the conversions that we'd then throw away after the conversion work is complete (they'll be here a fair while :-) ) This would be to ensure that we've not created any regressions.

Is there a consensus about the best way to approach this? we definitely want small PRs that should be aborted on the slightest sniff of a merge-conflict (it would indicate that the source .js changed prior to the merge).

Before I raise the PRs I feel it's worth having a quick discussion around the approach - I'm happy to lead, but happier still if I'm working with others' inputs to ensure that I'm making things as easy as possible for the maintainers.

[saghul]

I like the approach of small PRs, easier to make sure we don't miss things during review.

Tests would most certainly help too!

I'd suggest we get started with a PR or two and adjust the plan as necessary. It's virtually impossible to have everything fully thought through, so we can adapt.

Please @ me for a review when you post the first one and let's take it from there!

[garyhuntddn]

3x PRs in place

• one with the brittle tests - https://github.com/jitsi/lib-jitsi-meet/pull/1891
• one with the proposed typescript policy - https://github.com/jitsi/lib-jitsi-meet/pull/1892
• the quick one I did for the first conversion - https://github.com/jitsi/lib-jitsi-meet/pull/1875/files

I have 30+ additional PRs that follow this pattern that I'll submit if we're happy with what I've done so far

[garyhuntddn]

Hmm - hold tight - I'm hitting a build error in jenkins - will go and investigate

[garyhuntddn]

Cracked it - an issue with the packages.lock file - now sorted and the build is successful

[garyhuntddn]

@saghul would you consider this change to the package file? It's a bit lazy on my behalf, but will ensure that my future PRs include the updated .d.ts files. The downside is a bit of CPU effort running the typescript compiler.

garyhuntddn:gh/ts/patch-039-tsc-before-karma

I'm also going to wait until PR 1891 is merged before I rebase the other branches and push them over to you. Having 1893 in place as well would make that a tiny bit more reliable.

[garyhuntddn]

Right - I've got a process for updating these branches and PRs - apologies for the number of them - but hey ho :-)

[saghul]

I'd rather not have that one, sorry. And thanks a ton for the PRs!

[garyhuntddn]

No probs - I've just added a script that runs does "tsc && npm run test" that does what I need :-)

[saghul]

Thanks for understanding.

[garyhuntddn]

That's the bulk of the PRs in place - take a quick look at my comment against the one https://github.com/jitsi/lib-jitsi-meet/pull/1915

[saghul]

Dang, we got some work to do reviewing! Thanks a ton for all of this!

[garyhuntddn]

The next steps are going to be much harder, and some will require clean-up of the js first

[garyhuntddn]

A question about public vs internal code...

I feel like we could do with a definition of which parts of the code are part of it's public API (and therefore won't change without a major version jump) vs. code that is internal to the project and therefore subject to change.

Any thoughts on how we should make this clear to those consuming the library? eventually I guess we can re-export at the index.js level but for now would comments in the code do the trick - or is there a better option?

[saghul]

I feel like we could do with a definition of which parts of the code are part of it's public API (and therefore won't change without a major version jump) vs. code that is internal to the project and therefore subject to change.

Makes sense.

Any thoughts on how we should make this clear to those consuming the library? eventually I guess we can re-export at the index.js level but for now would comments in the code do the trick - or is there a better option?

I think so far we have used the _functionName syntax to express that. I guess we could turn that into private in TS terms.

Or do you have anything else in mind?

[garyhuntddn]

The _ works nicely and is equivalent for private - but I'm thinking more about internal (it's a C# term) that refers to parts of the project which are visible between components inside the project but not directly accessible to those consuming the project.

e.g. from what I can gather the xmpp stuff where we change the enum - that was a breaking change if somebody outside of the library was referencing those modules, but as far as your concerned they are internal to the project and therefore you were happy to change them and nobody should be referring to them.

That make sense?

On Mon, 28 Feb 2022 at 09:20, Saúl Ibarra Corretgé @.***> wrote:

I feel like we could do with a definition of which parts of the code are part of it's public API (and therefore won't change without a major version jump) vs. code that is internal to the project and therefore subject to change.

Makes sense.

Any thoughts on how we should make this clear to those consuming the library? eventually I guess we can re-export at the index.js level but for now would comments in the code do the trick - or is there a better option?

I think so far we have used the _functionName syntax to express that. I guess we could turn that into private in TS terms.

Or do you have anything else in mind?

— Reply to this email directly, view it on GitHub https://github.com/jitsi/lib-jitsi-meet/issues/1890#issuecomment-1054050510, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKT54Z4OAUFVNX62SPXKFLU5M45ZANCNFSM5OXXYGXQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you authored the thread.Message ID: @.***>

--

Gary Hunt

Senior Developer, Data Delivery Network Ltd**Mob: 07970 908428

*DISCLAIMER :*This Email contains confidential information some or all of which may be legally privileged. It is intended for the recipient only. If an addressing or transmission error has misdirected this Email, please notify the above author.

[saghul]

Ah, I see what you mean. This is actually our public API: https://github.com/jitsi/lib-jitsi-meet/blob/master/JitsiMeetJS.js

Anything not exposed there or publicly accessible through exported objects is private.

Not sure how we can annotate stuff to reflect this though.

[garyhuntddn]

Yeah - that was what I was getting at - seems to be much harder in javascript due to the use of modules both internally and externally.

On Mon, 28 Feb 2022 at 10:11, Saúl Ibarra Corretgé @.***> wrote:

Ah, I see what you mean. This is actually our public API: https://github.com/jitsi/lib-jitsi-meet/blob/master/JitsiMeetJS.js

Anything not exposed there or publicly accessible through exported objects is private.

Not sure how we can annotate stuff to reflect this though.

— Reply to this email directly, view it on GitHub https://github.com/jitsi/lib-jitsi-meet/issues/1890#issuecomment-1054094992, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKT545K3CJN4P4HXS3LQ4TU5NC55ANCNFSM5OXXYGXQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you authored the thread.Message ID: @.***>

--

Gary Hunt

Senior Developer, Data Delivery Network Ltd**Mob: 07970 908428

*DISCLAIMER :*This Email contains confidential information some or all of which may be legally privileged. It is intended for the recipient only. If an addressing or transmission error has misdirected this Email, please notify the above author.

[saghul]

Hum. We could use stripInternal and annotate stuff with /** @internal */.

[garyhuntddn]

That sounds like a good plan - I'll adopt that and see how we get on

On Mon, 28 Feb 2022 at 10:35, Saúl Ibarra Corretgé @.***> wrote:

Hum. We could use stripInternal and annotate stuff with /** @internal */.

— Reply to this email directly, view it on GitHub https://github.com/jitsi/lib-jitsi-meet/issues/1890#issuecomment-1054116353, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKT54ZOZVJ2BDWVMFH4DH3U5NFXHANCNFSM5OXXYGXQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you authored the thread.Message ID: @.***>

--

Gary Hunt

Senior Developer, Data Delivery Network Ltd**Mob: 07970 908428

*DISCLAIMER :*This Email contains confidential information some or all of which may be legally privileged. It is intended for the recipient only. If an addressing or transmission error has misdirected this Email, please notify the above author.