refactor: Remove method Chaining from videojs (#3860)

Remove method chaining from videojs. This helps keep the methods consistent especially since play() now returns a Promise in some cases. Also, it can add some performance benefits. BREAKING CHANGE: player methods no longer return a player instance when called. Fixes #3704.
videojs · Jan 18, 2017 · 8f07f5d · 8f07f5d
1 parent 0bba319
commit 8f07f5d
Show file tree

Hide file tree

Showing 9 changed files with 174 additions and 330 deletions.
diff --git a/src/js/clickable-component.js b/src/js/clickable-component.js
@@ -108,9 +108,8 @@ class ClickableComponent extends Component {
    * @param {Element} [el=this.el()]
    *        Element to set the title on.
    *
-   * @return {string|ClickableComponent}
+   * @return {string}
    *         - The control text when getting
-   *         - Returns itself when setting; method can be chained.
    */
   controlText(text, el = this.el()) {
     if (!text) {
@@ -122,8 +121,6 @@ class ClickableComponent extends Component {
     this.controlText_ = text;
     this.controlTextEl_.innerHTML = localizedText;
     el.setAttribute('title', localizedText);
-
-    return this;
   }
 
   /**
@@ -138,9 +135,6 @@ class ClickableComponent extends Component {
 
   /**
    * Enable this `Component`s element.
-   *
-   * @return {ClickableComponent}
-   *         Returns itself; method can be chained.
    */
   enable() {
     this.removeClass('vjs-disabled');
@@ -152,14 +146,10 @@ class ClickableComponent extends Component {
     this.on('click', this.handleClick);
     this.on('focus', this.handleFocus);
     this.on('blur', this.handleBlur);
-    return this;
   }
 
   /**
    * Disable this `Component`s element.
-   *
-   * @return {ClickableComponent}
-   *         Returns itself; method can be chained.
    */
   disable() {
     this.addClass('vjs-disabled');
@@ -171,7 +161,6 @@ class ClickableComponent extends Component {
     this.off('click', this.handleClick);
     this.off('focus', this.handleFocus);
     this.off('blur', this.handleBlur);
-    return this;
   }
 
   /**

diff --git a/src/js/component.js b/src/js/component.js
@@ -573,9 +573,6 @@ class Component {
    *        The event handler if `first` is a `Component` and `second` is an event name
    *        or an Array of event names.
    *
-   * @return {Component}
-   *         Returns itself; method can be chained.
-   *
    * @listens Component#dispose
    */
   on(first, second, third) {
@@ -618,8 +615,6 @@ class Component {
         target.on('dispose', cleanRemover);
       }
     }
-
-    return this;
   }
 
   /**
@@ -635,9 +630,6 @@ class Component {
    * @param {EventTarget~EventListener} [third]
    *        The event handler if `first` is a `Component` and `second` is an event name
    *        or an Array of event names.
-   *
-   * @return {Component}
-   *         Returns itself; method can be chained.
    */
   off(first, second, third) {
     if (!first || typeof first === 'string' || Array.isArray(first)) {
@@ -662,8 +654,6 @@ class Component {
         target.off('dispose', fn);
       }
     }
-
-    return this;
   }
 
   /**
@@ -678,9 +668,6 @@ class Component {
    * @param {EventTarget~EventListener} [third]
    *        The event handler if `first` is a `Component` and `second` is an event name
    *        or an Array of event names.
-   *
-   * @return {Component}
-   *         Returns itself; method can be chained.
    */
   one(first, second, third) {
     if (typeof first === 'string' || Array.isArray(first)) {
@@ -700,8 +687,6 @@ class Component {
 
       this.on(target, type, newFunc);
     }
-
-    return this;
   }
 
   /**
@@ -713,13 +698,9 @@ class Component {
    *
    * @param {Object} [hash]
    *        Data hash to pass along with the event
-   *
-   * @return {Component}
-   *         Returns itself; method can be chained.
    */
   trigger(event, hash) {
     Events.trigger(this.el_, event, hash);
-    return this;
   }
 
   /**
@@ -731,9 +712,6 @@ class Component {
    *
    * @param  {boolean} [sync=false]
    *         Execute the listener synchronously if `Component` is ready.
-   *
-   * @return {Component}
-   *         Returns itself; method can be chained.
    */
   ready(fn, sync = false) {
     if (fn) {
@@ -749,7 +727,6 @@ class Component {
         this.readyQueue_.push(fn);
       }
     }
-    return this;
   }
 
   /**
@@ -847,27 +824,19 @@ class Component {
    *
    * @param {string} classToAdd
    *        CSS class name to add
-   *
-   * @return {Component}
-   *         Returns itself; method can be chained.
    */
   addClass(classToAdd) {
     Dom.addElClass(this.el_, classToAdd);
-    return this;
   }
 
   /**
    * Remove a CSS class name from the `Component`s element.
    *
    * @param {string} classToRemove
    *        CSS class name to remove
-   *
-   * @return {Component}
-   *         Returns itself; method can be chained.
    */
   removeClass(classToRemove) {
     Dom.removeElClass(this.el_, classToRemove);
-    return this;
   }
 
   /**
@@ -880,65 +849,45 @@ class Component {
    *
    * @param  {boolean|Dom~predicate} [predicate]
    *         An {@link Dom~predicate} function or a boolean
-   *
-   * @return {Component}
-   *         Returns itself; method can be chained.
    */
   toggleClass(classToToggle, predicate) {
     Dom.toggleElClass(this.el_, classToToggle, predicate);
-    return this;
   }
 
   /**
    * Show the `Component`s element if it is hidden by removing the
    * 'vjs-hidden' class name from it.
-   *
-   * @return {Component}
-   *         Returns itself; method can be chained.
    */
   show() {
     this.removeClass('vjs-hidden');
-    return this;
   }
 
   /**
    * Hide the `Component`s element if it is currently showing by adding the
    * 'vjs-hidden` class name to it.
-   *
-   * @return {Component}
-   *         Returns itself; method can be chained.
    */
   hide() {
     this.addClass('vjs-hidden');
-    return this;
   }
 
   /**
    * Lock a `Component`s element in its visible state by adding the 'vjs-lock-showing'
    * class name to it. Used during fadeIn/fadeOut.
    *
-   * @return {Component}
-   *         Returns itself; method can be chained.
-   *
    * @private
    */
   lockShowing() {
     this.addClass('vjs-lock-showing');
-    return this;
   }
 
   /**
    * Unlock a `Component`s element from its visible state by removing the 'vjs-lock-showing'
    * class name from it. Used during fadeIn/fadeOut.
    *
-   * @return {Component}
-   *         Returns itself; method can be chained.
-   *
    * @private
    */
   unlockShowing() {
     this.removeClass('vjs-lock-showing');
-    return this;
   }
 
   /**
@@ -969,14 +918,10 @@ class Component {
    * @param {string} value
    *        Value to set the attribute to.
    *
-   * @return {Component}
-   *         Returns itself; method can be chained.
-   *
    * @see [DOM API]{@link https://developer.mozilla.org/en-US/docs/Web/API/Element/setAttribute}
    */
   setAttribute(attribute, value) {
     Dom.setAttribute(this.el_, attribute, value);
-    return this;
   }
 
   /**
@@ -985,14 +930,10 @@ class Component {
    * @param {string} attribute
    *        Name of the attribute to remove.
    *
-   * @return {Component}
-   *         Returns itself; method can be chained.
-   *
    * @see [DOM API]{@link https://developer.mozilla.org/en-US/docs/Web/API/Element/removeAttribute}
    */
   removeAttribute(attribute) {
     Dom.removeAttribute(this.el_, attribute);
-    return this;
   }
 
   /**
@@ -1005,10 +946,9 @@ class Component {
    * @param {boolean} [skipListeners]
    *        Skip the resize event trigger
    *
-   * @return {Component|number|string}
-   *         - The width when getting, zero if there is no width. Can be a string
+   * @return {number|string}
+   *         The width when getting, zero if there is no width. Can be a string
    *           postpixed with '%' or 'px'.
-   *         - Returns itself when setting; method can be chained.
    */
   width(num, skipListeners) {
     return this.dimension('width', num, skipListeners);
@@ -1024,10 +964,9 @@ class Component {
    * @param {boolean} [skipListeners]
    *        Skip the resize event trigger
    *
-   * @return {Component|number|string}
-   *         - The width when getting, zero if there is no width. Can be a string
-   *           postpixed with '%' or 'px'.
-   *         - Returns itself when setting; method can be chained.
+   * @return {number|string}
+   *         The width when getting, zero if there is no width. Can be a string
+   *         postpixed with '%' or 'px'.
    */
   height(num, skipListeners) {
     return this.dimension('height', num, skipListeners);
@@ -1041,13 +980,11 @@ class Component {
    *
    * @param  {number|string} height
    *         Height to set the `Component`s element to.
-   *
-   * @return {Component}
-   *         Returns itself; method can be chained.
    */
   dimensions(width, height) {
     // Skip resize listeners on width for optimization
-    return this.width(width, true).height(height);
+    this.width(width, true);
+    this.height(height);
   }
 
   /**
@@ -1075,9 +1012,8 @@ class Component {
    * @param  {boolean} [skipListeners]
    *         Skip resize event trigger
    *
-   * @return {Component}
-   *         - the dimension when getting or 0 if unset
-   *         - Returns itself when setting; method can be chained.
+   * @return {number}
+   *         The dimension when getting or 0 if unset
    */
   dimension(widthOrHeight, num, skipListeners) {
     if (num !== undefined) {
@@ -1106,8 +1042,7 @@ class Component {
         this.trigger('resize');
       }
 
-      // Return component
-      return this;
+      return;
     }
 
     // Not setting a value, so getting it

diff --git a/src/js/menu/menu-button.js b/src/js/menu/menu-button.js
@@ -245,9 +245,6 @@ class MenuButton extends ClickableComponent {
 
   /**
    * Disable the `MenuButton`. Don't allow it to be clicked.
-   *
-   * @return {MenuButton}
-   *         Returns itself; method can be chained.
    */
   disable() {
     // Unpress, but don't force focus on this button
@@ -257,19 +254,15 @@ class MenuButton extends ClickableComponent {
 
     this.enabled_ = false;
 
-    return super.disable();
+    super.disable();
   }
 
   /**
    * Enable the `MenuButton`. Allow it to be clicked.
-   *
-   * @return {MenuButton}
-   *         Returns itself; method can be chained.
    */
   enable() {
     this.enabled_ = true;
-
-    return super.enable();
+    super.enable();
   }
 }