From 970faa8fb19f31d31a81e1387cdd026494fcc9b5 Mon Sep 17 00:00:00 2001
From: mister-ben <1676039+mister-ben@users.noreply.github.com>
Date: Sat, 6 Jul 2024 08:31:15 +0100
Subject: [PATCH] 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
---
 src/js/component.js | 19 ++++++++-----------
 src/js/player.js    |  8 +++++++-
 src/js/video.js     | 11 +++--------
 3 files changed, 18 insertions(+), 20 deletions(-)

diff --git a/src/js/component.js b/src/js/component.js
index 69cb75275b..ad8e67f627 100644
--- a/src/js/component.js
+++ b/src/js/component.js
@@ -15,6 +15,14 @@ import {merge} from './utils/obj.js';
 
 /** @import Player from './player' */
 
+/**
+ * A callback to be called if and when the component is ready.
+ * `this` will be the Component instance.
+ *
+ * @callback ReadyCallback
+ * @returns  {void}
+ */
+
 /**
  * Base class for all UI Components.
  * Components are UI objects which represent both a javascript object and an element
@@ -25,14 +33,6 @@ import {merge} from './utils/obj.js';
  */
 class Component {
 
-  /**
-   * A callback that is called when a component is ready. Does not have any
-   * parameters and any callback value will be ignored.
-   *
-   * @callback ReadyCallback
-   * @this Component
-   */
-
   /**
    * Creates an instance of this class.
    *
@@ -837,9 +837,6 @@ class Component {
    *
    * @param {ReadyCallback} fn
    *        Function that gets called when the `Component` is ready.
-   *
-   * @return {Component}
-   *         Returns itself; method can be chained.
    */
   ready(fn, sync = false) {
     if (!fn) {
diff --git a/src/js/player.js b/src/js/player.js
index beea9031fb..e9c7300a56 100644
--- a/src/js/player.js
+++ b/src/js/player.js
@@ -60,6 +60,12 @@ import './tech/html5.js';
 /** @import { TimeRange } from './utils/time' */
 /** @import HtmlTrackElement from './tracks/html-track-element' */
 
+/**
+ * @callback PlayerReadyCallback
+ * @this     {Player}
+ * @returns  {void}
+ */
+
 // The following tech events are simply re-triggered
 // on the player when they happen
 const TECH_EVENTS_RETRIGGER = [
@@ -307,7 +313,7 @@ class Player extends Component {
    * @param {Object} [options]
    *        Object of option names and values.
    *
-   * @param {Function} [ready]
+   * @param {PlayerReadyCallback} [ready]
    *        Ready callback function.
    */
   constructor(tag, options, ready) {
diff --git a/src/js/video.js b/src/js/video.js
index 474c02103e..918b071802 100644
--- a/src/js/video.js
+++ b/src/js/video.js
@@ -41,6 +41,8 @@ import xhr from '@videojs/xhr';
 import Tech from './tech/tech.js';
 import { use as middlewareUse, TERMINATOR } from './tech/middleware.js';
 
+/** @import { PlayerReadyCallback } from './player' */
+
 /**
  * Normalize an `id` value by trimming off a leading `#`
  *
@@ -53,13 +55,6 @@ import { use as middlewareUse, TERMINATOR } from './tech/middleware.js';
  */
 const normalizeId = (id) => id.indexOf('#') === 0 ? id.slice(1) : id;
 
-/**
- * A callback that is called when a component is ready. Does not have any
- * parameters and any callback value will be ignored. See: {@link Component~ReadyCallback}
- *
- * @callback ReadyCallback
- */
-
 /**
  * The `videojs()` function doubles as the main function for users to create a
  * {@link Player} instance as well as the main library namespace.
@@ -121,7 +116,7 @@ const normalizeId = (id) => id.indexOf('#') === 0 ? id.slice(1) : id;
  *         Options object for providing settings.
  *         See: [Options Guide](https://docs.videojs.com/tutorial-options.html).
  *
- * @param  {ReadyCallback} [ready]
+ * @param  {PlayerReadyCallback} [ready]
  *         A function to be called when the {@link Player} and {@link Tech} are
  *         ready.
  *