fix(types): Minor fix for types

[andreifilip123]

Description

Hello! We're using videojs but since the upgrade to v8, we've ran into some typing issues that we had to patch with @ts-ignore and such.
This PR fixes the issues we had with the .off() function always requiring a second param for function and the issues we have with the videojs.getComponent('Component') not returning the right type.
I'm not sure how to build from the jsdoc into the .d.ts files to check that the generated types are correct but I think these changes might work.
I'd also like to help fixing the missing ".remoteTextTracks()" function on the Player type and the missing export for video player options but I don't know how. If you give me some pointers, I'd be glad to help with this

Specific Changes proposed

• remove the fn argument as required on the off function
• change the returned type of the getComponent function

Requirements Checklist

• Feature implemented / Bug fixed
• If necessary, more likely in a feature request than a bug fix
  • Change has been verified in an actual browser (Chrome, Firefox, IE)
  • Unit Tests updated or fixed
  • Docs/guides updated
  • Example created (starter template on JSBin)
• Reviewed by Two Core Contributors

[welcome]

💖 Thanks for opening this pull request! 💖

Things that will help get your PR across the finish line:

• Run npm run lint -- --errors locally to catch formatting errors earlier.
• Include tests when adding/changing behavior.
• Include screenshots and animated GIFs whenever possible.

We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can.

[gkatsev]

While providing a function to off is technically optional, I would highly suggest not using it that way unless you're doing so for an object you've created. Calling off without a function param, will remove all handlers for that type, whether you added the handler or Video.js did it internally.

[codecov]

Codecov Report

Merging #8466 (ca71705) into main (cd2dac4) will not change coverage.
Report is 2 commits behind head on main.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #8466   +/-   ##
=======================================
  Coverage   82.71%   82.71%           
=======================================
  Files         113      113           
  Lines        7589     7589           
  Branches     1826     1826           
=======================================
  Hits         6277     6277           
  Misses       1312     1312           

Files	Coverage Δ
src/js/component.js	93.54% <ø> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[andreifilip123]

While providing a function to off is technically optional, I would highly suggest not using it that way unless you're doing so for an object you've created. Calling off without a function param, will remove all handlers for that type, whether you added the handler or Video.js did it internally.

The place we're using this is in the react unmount hook. Therefore there can't be any other events after that. If I recall correctly, the fn argument was optional in the past. That's why I made it optional now too. If you want to keep it required I can remove it and now I understand the reason.
What about the other parts of the PR? I want to help with the other parts too if you give me some hints 😄

[gkatsev]

It probably should be optional, as that is the proper method signature. Just wanting to make sure you're using it properly because otherwise, it could cause problems :)

[mister-ben]

Re remoteTextTracks(), this is documented in jsdoc, but tsc ignores it 😡. The methods are added here. One way around it without a bigger refactor (but needs discussion and a separate PR) is to stub these methods in player.js with a placeholder that gets replaced instead of the @method jsdoc.

[andreifilip123]

Thank you for looking into this.

[jolson88]

Re remoteTextTracks(), this is documented in jsdoc, but tsc ignores it 😡. The methods are added here. One way around it without a bigger refactor (but needs discussion and a separate PR) is to stub these methods in player.js with a placeholder that gets replaced instead of the @method jsdoc.

Outside observer here having run into the typing issues here as well. I think there may be a simpler solution for this that would avoid a refactor. I'm prototyping an idea I have in a fork and then can create a PR for description soon.

[jolson88]

Re remoteTextTracks(), this is documented in jsdoc, but tsc ignores it 😡. The methods are added here. One way around it without a bigger refactor (but needs discussion and a separate PR) is to stub these methods in player.js with a placeholder that gets replaced instead of the @method jsdoc.

Yup, you were right. That's what was needed. I have a PR I'm polishing up now that I hope to get created today. My company just needed audioTracks and videoTracks, but I'll add the textTracks and remote track stuff too so those on this thread needing it can get it in the same PR. I'll tag this PR.

[jolson88]

Just wanted to follow-up. Looks like I'll be getting this up tomorrow. I found some changes that were caused to the generated JS docs for the website that aren't good. So I need to do a little more polishing before submitting the PR.

[jolson88]

Just wanted to follow-up. Looks like I'll be getting this up tomorrow. I found some changes that were caused to the generated JS docs for the website that aren't good. So I need to do a little more polishing before submitting the PR.

Apologies for the delay. An initial PR has been submitted. This doesn't address all the issues (just a minor subset for now). I wanted to get the low-hanging fruit done first and drive some conversation before more invasive changes can be explored. I hope the PR that I submitted and linked to this helps other TypeScript devs at least unblock themselves for the next steps though.

#8486

[welcome]

Congrats on merging your first pull request! 🎉🎉🎉