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.

Sign up for GitHub

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

Jump to bottom

[WIP] Type enhancements #8968

Draft
wants to merge 17 commits into
base: main
Choose a base branch
from
Draft

[WIP] Type enhancements #8968

wants to merge 17 commits into from
+266 −105

Conversation

mister-ben
Copy link
Contributor

@mister-ben mister-ben commented Jan 23, 2025 •
edited
Loading

Several changes for Typescript.

  • Adds a jsdoc plugin to not break on @template {xyz} T. Replaces uses of T with Class.<xyz>
  • Adds definitions for ComponentOptions, SourceObject, PlayerOptions, MediaObject
  • Removes further uses of ~ namespacing in jsdoc, which tsc fails to understand
  • Changes track kinds to simple union types. Change the existing definitions to enums so we can use them as object lookups without conflicting with the standard definitions.
  • Use native TextTrackCue, TextTrackCue definitions.
  • Exports Player, PlayerOptions and Plugin from video.js.
    • This makes it easier to add to the object than at present e.g. in a plugin to declare module 'video.js' { interface Player {
    • This sets a precedent - what else could/should be exported at this level? Should the names be more verbose?
  • Adds additional jsdoc on for track and readyState in HTMLTrackElement, as tsc doesn't understand documentation for properties added with Object.defineProperties . Fixes HTMLTrackElement class provided missing two properties as stated in MDN #8959

@codecov Codecov
Copy link

codecov bot commented Jan 23, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.72%. Comparing base (f8e9bfc) to head (c884bfa).

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #8968   +/-   ##
=======================================
  Coverage   83.72%   83.72%           
=======================================
  Files         120      120           
  Lines        8110     8110           
  Branches     1948     1948           
=======================================
  Hits         6790     6790           
  Misses       1320     1320

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

mister-ben
mister-ben commented Jan 25, 2025
View reviewed changes
src/js/video.js
@@ -617,3 +620,7 @@ videojs.url = Url;
videojs.Error = VjsErrors;

export default videojs;
export {
Player,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This allows adding to the Player interface in plugins:

declare module 'video.js' {
  interface Player {
    touchOverlay: TouchOverlay;
    touchOverlayDirect: TouchOverlay;
    mobileUi: {
      (options?: MobileUiPluginOptions): void;
      VERSION: string;
    }
  }
}

mister-ben
mister-ben commented Jan 31, 2025
View reviewed changes
src/js/component.js
* The key/value store of options that will get passed to children of
* the child.
*
* @param {number} [index=this.children_.length]
* The index to attempt to add a child into.
*
*
* @return {Component}
* @return {T}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renders like this in the API docs with the changes to jsdoc-typeof-plugin:
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

HTMLTrackElement class provided missing two properties as stated in MDN
1 participant
@mister-ben