Increase Typescript Usage - Help Wanted!

johnBartos commented 5 years ago

Since adding support for TS in our build process, we've yet to follow up with significant TS integration. I think we should organize our efforts with anyone who wants to participate so that we can work towards increasing coverage.

This issue will be used to organize/discuss TS integration; if you'd like to help, please leave a comment here with what you'd like to help out with (or a plan if you have one). See the Typescript Integration project board for any active tickets/PRs.

itsjamie commented 5 years ago

I'm happy to help with this. I'm looking at starting with working from core components out. @johnBartos you had linked me the code for manifest parsing, I was planning on going from MSE implementation to manifest code.

itsjamie commented 5 years ago

I'm going to look at converting src/controller/buffer-controller.js to TypeScript.

patrickmichalina commented 5 years ago

I am willing to refactor into proper .ts files if that is the goal of this ticket?

jonnyboris commented 5 years ago

I'm happy to help out too. I've got more experience with typescript than the internals of hls.js, so it may be best to take some direction on where to start. Totally down to begin with adding interfaces and types to more boring parts of the codebase, or to pick up any monotonous jobs, just to get up and running.

dsparacio commented 5 years ago

plus 1

germain-gg commented 5 years ago

👋

I'd be happy to help increasing the TS coverage! Don't really have a plan of attack but definitely willing to give a hand

itsjamie commented 5 years ago

Going to start to convert src/controller/eme-controller.js.

itsjamie commented 5 years ago

Going to start to convert ~src/controller/stream-controller.js~ src/controller/base-stream-controller.

johnBartos commented 5 years ago

Thanks for pitching in everyone. I think @itsjamie has done a great job of converting individual modules to start - I think repeating his process for other modules would be great. Each module converted should have no functional changes, and should be minimal - if you're finding too much needs to be changed you should report back here so we can take a more deliberate approach.

If anyone wants to pick up where I left off, I converted a bunch of modules to TS in another project: https://github.com/video-dev/media-lighthouse/tree/master/src/hls. I believe that they can be copy-pasted back here with minimal fuss.

itsjamie commented 5 years ago

Thanks @johnBartos.

If anyone wants to take a file and add types to it I think it's a great way to learn about the codebase.

The core things to know that I learned as a first-timer:

A function called for example onXXX (onBufferAppended) will likely be an event handler. To confirm, look to see if the class it is in extends EventHandler if it does, anything by this name will be called by other files that do this.hls.trigger(eventName). This is how I tracked down what was being sent into events to type these functions, or infers it based on the usage.
I would focus on just adding basic interface types for the first pass. Then progressively handling property initializers, and then the null checks for undefined variables.

itsjamie commented 5 years ago

Added a PR that converts fragment, and level-key to typescript. I also have a follow up to that which does playlist-loader that I'll open once frag and level key gets in.

itsjamie commented 5 years ago

If anyone is looking for a file to get started, I'd stay away from fragment-loader as John is planning a refactor of that.

stale[bot] commented 5 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

silltho commented 5 years ago

Hey! I'm looking forward to get hls.js in typescript.

Can I do anything to help getting this done? Whats the status of the typescript refactoring? When will it be released?

johnBartos commented 5 years ago

Note: I have typescripted the stream-controllers, and all of the demuxing/remuxing code already.

Things which still need TSing:

[x] abr-controller.js done in JWFork
[x] cap-level-controller.js done in JWFork
[ ] fps-controller.js
[x] fragment-finders.js #2347
[x] fragment-track.js done in JWFork
[ ] gap-controller.js
[x] id3-track-controller.js #2242
[x] timeline-controller.js #2243
[x] key-loader.js #2341
[x] m3u8-parser.js #2343
[x] is-supported.js

itsjamie commented 5 years ago

Just did key loader. ~Going to see if I can get m3u8-parser done.~ ~Not getting to it this weekend.~ I lied. Got to it.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

LucasMaupin commented 4 years ago

Hello, we are a small group of devs who would like to contribute to this initiative. We are wondering where we can help? There seems to be a lot of work already done in the feature/v1.0.0 branch, should we continue from there? Any files prioritised?

itsjamie commented 4 years ago

@LucasMaupin definitely work off the feature/v1.0.0 branch for converting .js -> .ts. Then target your PR's against that branch.

I don't think there are any higher priority ones. There is also an open PR where I'm in the progress of making the event emitter typed that is currently off the feature/v1.0.0 branch, but I think getting the remaining .js files converted as much as possible would be awesome.

If you're going to slice off a file, just a comment that you're doing so here helps to not have duplicate work. I think for the most part it's mostly utilities or helper classes that aren't fully typed so the cross-dependencies should be lower.

LucasMaupin commented 4 years ago

@itsjamie we have made a PR to feature/v1.0.0 where we converted a few files to TS. The PR has been open for a week and we were wondering if we need to add a reviewer in order for someone to review it or just be patient. We cannot seem to add a reviewer.

itsjamie commented 4 years ago

Took a look, they both look good to me, thank you again for the help!

michaelcunningham19 commented 4 years ago

This issue was closed unintentionally by stalebot

robwalch commented 4 years ago

Added tasks for converting the remaining JS files, and a task to note that we need to list modules/issues where typing is weak (I still run into many places where any was used a shortcut to complete conversion) https://github.com/video-dev/hls.js/projects/5

We're pretty close. If you have time, keep it up!

matamatanot commented 3 years ago

Is the reason that "strict": false mentioned anywhere?

"strict": false means

"noImplicitAny" : true, "noImplicitThis" : true, "alwaysStrict" : true, "strictBindCallApply" : true, "strictNullChecks" : true, "strictFunctionTypes" : true, "strictPropertyInitialization" : true

Currently, "strictNullChecks" and "strictPropertyInitialization" has been activated, but what are your plans for the future?

video-dev / hls.js

Increase Typescript Usage - Help Wanted! #2070