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.

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

fix: improve types and allow augmentation #8545

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 10 additions & 12 deletions src/js/component.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@ import {toTitleCase, toLowerCase} from './utils/str.js';
import {merge} from './utils/obj.js';
import keycode from 'keycode';

/**
* @typedef {Object} ComponentOptions
* @property {Object[]} children
* @property {string} classname
*/

/**
* Base class for all UI Components.
* Components are UI objects which represent both a javascript object and an element
Expand All @@ -38,17 +44,9 @@ class Component {
* @param { import('./player').default } player
* The `Player` that this class should be attached to.
*
* @param {Object} [options]
* @param {ComponentOptions} [options]
* The key/value store of component options.
*
* @param {Object[]} [options.children]
* An array of children objects to initialize this component with. Children objects have
* a name property that will be used if more than one component of the same type needs to be
* added.
*
* @param {string} [options.className]
* A class or space separated list of classes to add the component
*
* @param {ReadyCallback} [ready]
* Function that gets called when the `Component` is ready.
*/
Expand Down Expand Up @@ -1777,11 +1775,11 @@ class Component {
*
* @param {string} name
* The name of the `Component` to register.
*
* @param {Component} ComponentToRegister
* @template {Component} C
* @param {C} ComponentToRegister
* The `Component` class to register.
*
* @return {Component}
* @return {C}
Copy link
Contributor

Choose a reason for hiding this comment

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

The jsdoc output is bad here, has a {C} type that's not defined
image

whereas the getComponent you've modified below in video.js is displayed correctly, with a {Class.<Component>}
image

Copy link
Author

Choose a reason for hiding this comment

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

🙇 Thanks, fixed

* The `Component` that was registered.
*/
static registerComponent(name, ComponentToRegister) {
Expand Down
104 changes: 53 additions & 51 deletions src/js/player.js
Original file line number Diff line number Diff line change
Expand Up @@ -4606,6 +4606,57 @@ class Player extends Component {
});
}

// The following empty getters are defined below the class. They are declared here
// for `tsc` to pick them up in type definitions.

/**
* Get the {@link VideoTrackList}
*
* @link https://html.spec.whatwg.org/multipage/embedded-content.html#videotracklist
*
* @return {VideoTrackList}
* the current video track list
*
* @method Player.prototype.videoTracks
*/
videoTracks() {}

/**
* Get the {@link AudioTrackList}
*
* @link https://html.spec.whatwg.org/multipage/embedded-content.html#audiotracklist
*
* @return {AudioTrackList}
* the current audio track list
*/
audioTracks() {}

/**
* Get the {@link TextTrackList}
*
* @link http://www.w3.org/html/wg/drafts/html/master/embedded-content-0.html#dom-media-texttracks
*
* @return {TextTrackList}
* the current text track list
*/
textTracks() {}

/**
* Get the remote {@link TextTrackList}
*
* @return {TextTrackList}
* The current remote text track list
*/
remoteTextTracks() {}

/**
* Get the remote {@link HtmlTrackElementList} tracks.
*
* @return {HtmlTrackElementList}
* The current remote text track element list
*/
remoteTextTrackEls() {}

/**
* A helper method for adding a {@link TextTrack} to our
* {@link TextTrackList}.
Expand Down Expand Up @@ -5258,57 +5309,6 @@ class Player extends Component {
}
}

/**
* Get the {@link VideoTrackList}
*
* @link https://html.spec.whatwg.org/multipage/embedded-content.html#videotracklist
*
* @return {VideoTrackList}
* the current video track list
*
* @method Player.prototype.videoTracks
*/

/**
* Get the {@link AudioTrackList}
*
* @link https://html.spec.whatwg.org/multipage/embedded-content.html#audiotracklist
*
* @return {AudioTrackList}
* the current audio track list
*
* @method Player.prototype.audioTracks
*/

/**
* Get the {@link TextTrackList}
*
* @link http://www.w3.org/html/wg/drafts/html/master/embedded-content-0.html#dom-media-texttracks
*
* @return {TextTrackList}
* the current text track list
*
* @method Player.prototype.textTracks
*/

/**
* Get the remote {@link TextTrackList}
*
* @return {TextTrackList}
* The current remote text track list
*
* @method Player.prototype.remoteTextTracks
*/

/**
* Get the remote {@link HtmlTrackElementList} tracks.
*
* @return {HtmlTrackElementList}
* The current remote text track element list
*
* @method Player.prototype.remoteTextTrackEls
*/

TRACK_TYPES.names.forEach(function(name) {
const props = TRACK_TYPES[name];

Expand Down Expand Up @@ -5484,3 +5484,5 @@ TECH_EVENTS_RETRIGGER.forEach(function(event) {

Component.registerComponent('Player', Player);
export default Player;
// Including a named export so Typescript can use module augmentation with plugins
export { Player };
64 changes: 64 additions & 0 deletions src/js/plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,12 @@ class Plugin {
this.log = this.player.log.createLogger(this.name);
}

// Remove the placeholder event methods. If the component is evented, the
// real methods are added next
['on', 'off', 'one', 'any', 'trigger'].forEach(fn => {
this[fn] = undefined;
});

// Make this object evented, but remove the added `trigger` method so we
// use the prototype version instead.
evented(this);
Expand Down Expand Up @@ -248,6 +254,61 @@ class Plugin {
return hash;
}

// `on`, `off`, `one`, and `any` are here so tsc includes them in definitions.
// They are replaced or removed in the constructor

/**
* Adds an `event listener` to an instance of an `EventTarget`. An `event listener` is a
* function that will get called when an event with a certain name gets triggered.
*
* @param {string|string[]} type
* An event name or an array of event names.
*
* @param {Function} fn
* The function to call with `EventTarget`s
*/
on(type, fn) {}

/**
* Removes an `event listener` for a specific event from an instance of `EventTarget`.
* This makes it so that the `event listener` will no longer get called when the
* named event happens.
*
* @param {string|string[]} type
* An event name or an array of event names.
*
* @param {Function} [fn]
* The function to remove. If not specified, all listeners managed by Video.js will be removed.
*/
off(type, fn) {}

/**
* This function will add an `event listener` that gets triggered only once. After the
* first trigger it will get removed. This is like adding an `event listener`
* with {@link EventTarget#on} that calls {@link EventTarget#off} on itself.
*
* @param {string|string[]} type
* An event name or an array of event names.
*
* @param {Function} fn
* The function to be called once for each event name.
*/
one(type, fn) {}

/**
* This function will add an `event listener` that gets triggered only once and is
* removed from all events. This is like adding an array of `event listener`s
* with {@link EventTarget#on} that calls {@link EventTarget#off} on all events the
* first time it is triggered.
*
* @param {string|string[]} type
* An event name or an array of event names.
*
* @param {Function} fn
* The function to be called once for each event name.
*/
any(type, fn) {}

/**
* Triggers an event on the plugin object and overrides
* {@link module:evented~EventedMixin.trigger|EventedMixin.trigger}.
Expand Down Expand Up @@ -478,6 +539,9 @@ Player.prototype.hasPlugin = function(name) {

export default Plugin;

// Including a named export so Typescript can use module augmentation with plugins
export { Plugin };

/**
* Signals that a plugin is about to be set up on a player.
*
Expand Down
7 changes: 4 additions & 3 deletions src/js/video.js
Original file line number Diff line number Diff line change
Expand Up @@ -320,10 +320,11 @@ videojs.getComponent = Component.getComponent;
* @param {string} name
* The class name of the component
*
* @param {typeof Component} comp
* The component class
* @template {typeof Component} C
* @param {C} comp
* The `Component` class to register.
*
* @return {typeof Component}
* @return {C}
* The newly registered component
*/
videojs.registerComponent = (name, comp) => {
Expand Down