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 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
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
17 changes: 7 additions & 10 deletions src/js/component.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,12 @@ import {merge} from './utils/obj.js';
* @returns {void}
*/

/**
* @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 @@ -39,17 +45,9 @@ class Component {
* @param {Player} 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 @@ -1978,7 +1976,6 @@ class Component {
*
* @param {string} name
* The name of the `Component` to register.
*
* @param {Component} ComponentToRegister
* The `Component` class to register.
*
Expand Down
104 changes: 53 additions & 51 deletions src/js/player.js
Original file line number Diff line number Diff line change
Expand Up @@ -4674,6 +4674,57 @@
});
}

// 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() {}

Check warning on line 4690 in src/js/player.js

View check run for this annotation

Codecov / codecov/patch

src/js/player.js#L4690

Added line #L4690 was not covered by tests

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

Check warning on line 4700 in src/js/player.js

View check run for this annotation

Codecov / codecov/patch

src/js/player.js#L4700

Added line #L4700 was not covered by tests

/**
* 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() {}

Check warning on line 4710 in src/js/player.js

View check run for this annotation

Codecov / codecov/patch

src/js/player.js#L4710

Added line #L4710 was not covered by tests

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

Check warning on line 4718 in src/js/player.js

View check run for this annotation

Codecov / codecov/patch

src/js/player.js#L4718

Added line #L4718 was not covered by tests

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

Check warning on line 4726 in src/js/player.js

View check run for this annotation

Codecov / codecov/patch

src/js/player.js#L4726

Added line #L4726 was not covered by tests

/**
* A helper method for adding a {@link TextTrack} to our
* {@link TextTrackList}.
Expand Down Expand Up @@ -5363,57 +5414,6 @@
/* end-delete-from-build */
}

/**
* 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 @@ -5565,3 +5565,5 @@

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 @@
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 @@
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) {}

Check warning on line 270 in src/js/plugin.js

View check run for this annotation

Codecov / codecov/patch

src/js/plugin.js#L270

Added line #L270 was not covered by tests

/**
* 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) {}

Check warning on line 283 in src/js/plugin.js

View check run for this annotation

Codecov / codecov/patch

src/js/plugin.js#L283

Added line #L283 was not covered by tests

/**
* 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) {}

Check warning on line 296 in src/js/plugin.js

View check run for this annotation

Codecov / codecov/patch

src/js/plugin.js#L296

Added line #L296 was not covered by tests

/**
* 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) {}

Check warning on line 310 in src/js/plugin.js

View check run for this annotation

Codecov / codecov/patch

src/js/plugin.js#L310

Added line #L310 was not covered by tests

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

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 @@ -316,10 +316,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