Skip to content

Commit 970faa8

Browse files
authored
fix(types): fix and improve component ready callback definition (#8766)
- Removes erroneous `@return {Component}` from `ReadyCallback` in Component - Remove `@this {Component}` from `ReadyCallback` in Component, because this is not accurate for classes that extend Component - Adds `@returns {void}` to `ReadyCallback` in Component - Produces a more accurate typedef - Isn't strictly accurate in jsdoc/javascript but does this matter since it's well understood. Absence of `@return` is interpreted by tsc as returning `{any}`, `@returns {undefined}` would require an explicit `return undefined` in ts. - Adds a `{PlayerReadyCallback}` in Player with `@this {Player}` used in the `new Player()` and `videos()` constructors. - Are we ok adding this new typedef - Is inconsistent with `player.ready()` which uses `ReadyCallback` without `@this` - but this can't be changed without adding an otherwise unnecessary override just to pander to tsc. ## Requirements Checklist - [x] Feature implemented / Bug fixed - [ ] If necessary, more likely in a feature request than a bug fix - [x] 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](https://codepen.io/gkatsev/pen/GwZegv?editors=1000#0)) - [x] Has no DOM changes which impact accessiblilty or trigger warnings (e.g. Chrome issues tab) - [x] Has no changes to JSDoc which cause `npm run docs:api` to error - [ ] Reviewed by Two Core Contributors
1 parent 6221a8f commit 970faa8

File tree

3 files changed

+18
-20
lines changed

3 files changed

+18
-20
lines changed

src/js/component.js

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,14 @@ import {merge} from './utils/obj.js';
1515

1616
/** @import Player from './player' */
1717

18+
/**
19+
* A callback to be called if and when the component is ready.
20+
* `this` will be the Component instance.
21+
*
22+
* @callback ReadyCallback
23+
* @returns {void}
24+
*/
25+
1826
/**
1927
* Base class for all UI Components.
2028
* Components are UI objects which represent both a javascript object and an element
@@ -25,14 +33,6 @@ import {merge} from './utils/obj.js';
2533
*/
2634
class Component {
2735

28-
/**
29-
* A callback that is called when a component is ready. Does not have any
30-
* parameters and any callback value will be ignored.
31-
*
32-
* @callback ReadyCallback
33-
* @this Component
34-
*/
35-
3636
/**
3737
* Creates an instance of this class.
3838
*
@@ -837,9 +837,6 @@ class Component {
837837
*
838838
* @param {ReadyCallback} fn
839839
* Function that gets called when the `Component` is ready.
840-
*
841-
* @return {Component}
842-
* Returns itself; method can be chained.
843840
*/
844841
ready(fn, sync = false) {
845842
if (!fn) {

src/js/player.js

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,12 @@ import './tech/html5.js';
6060
/** @import { TimeRange } from './utils/time' */
6161
/** @import HtmlTrackElement from './tracks/html-track-element' */
6262

63+
/**
64+
* @callback PlayerReadyCallback
65+
* @this {Player}
66+
* @returns {void}
67+
*/
68+
6369
// The following tech events are simply re-triggered
6470
// on the player when they happen
6571
const TECH_EVENTS_RETRIGGER = [
@@ -307,7 +313,7 @@ class Player extends Component {
307313
* @param {Object} [options]
308314
* Object of option names and values.
309315
*
310-
* @param {Function} [ready]
316+
* @param {PlayerReadyCallback} [ready]
311317
* Ready callback function.
312318
*/
313319
constructor(tag, options, ready) {

src/js/video.js

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@ import xhr from '@videojs/xhr';
4141
import Tech from './tech/tech.js';
4242
import { use as middlewareUse, TERMINATOR } from './tech/middleware.js';
4343

44+
/** @import { PlayerReadyCallback } from './player' */
45+
4446
/**
4547
* Normalize an `id` value by trimming off a leading `#`
4648
*
@@ -53,13 +55,6 @@ import { use as middlewareUse, TERMINATOR } from './tech/middleware.js';
5355
*/
5456
const normalizeId = (id) => id.indexOf('#') === 0 ? id.slice(1) : id;
5557

56-
/**
57-
* A callback that is called when a component is ready. Does not have any
58-
* parameters and any callback value will be ignored. See: {@link Component~ReadyCallback}
59-
*
60-
* @callback ReadyCallback
61-
*/
62-
6358
/**
6459
* The `videojs()` function doubles as the main function for users to create a
6560
* {@link Player} instance as well as the main library namespace.
@@ -121,7 +116,7 @@ const normalizeId = (id) => id.indexOf('#') === 0 ? id.slice(1) : id;
121116
* Options object for providing settings.
122117
* See: [Options Guide](https://docs.videojs.com/tutorial-options.html).
123118
*
124-
* @param {ReadyCallback} [ready]
119+
* @param {PlayerReadyCallback} [ready]
125120
* A function to be called when the {@link Player} and {@link Tech} are
126121
* ready.
127122
*

0 commit comments

Comments
 (0)