Skip to content

Commit

Permalink
feat: Return the native Promise from play() (#3907)
Browse files Browse the repository at this point in the history
Return the native Promise from `play()` if it exists. `undefined` is returned otherwise.
This comes in as part of the greater effort to remove method chaining.

BREAKING CHANGE: `play()` no longer returns the player object but instead the native Promise or nothing.
  • Loading branch information
brandonocasey authored and gkatsev committed Jan 18, 2017
1 parent 29ffbfb commit 091bdf9
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 22 deletions.
22 changes: 14 additions & 8 deletions src/js/player.js
Original file line number Diff line number Diff line change
Expand Up @@ -1564,20 +1564,26 @@ class Player extends Component {
/**
* start media playback
*
* @return {Player}
* A reference to the player object this function was called on
* @return {Promise|undefined}
* Returns a `Promise` if the browser returns one, for most browsers this will
* return undefined.
*/
play() {
// Only calls the tech's play if we already have a src loaded
if (this.src() || this.currentSrc()) {
this.techCall_('play');
} else {
this.tech_.one('loadstart', function() {
this.play();
});
return this.techGet_('play');
}

return this;
this.ready(function() {
this.tech_.one('loadstart', function() {
const retval = this.play();

// silence errors (unhandled promise from play)
if (retval !== undefined && typeof retval.then === 'function') {
retval.then(null, (e) => {});
}
});
});
}

/**
Expand Down
24 changes: 10 additions & 14 deletions src/js/tech/html5.js
Original file line number Diff line number Diff line change
Expand Up @@ -455,19 +455,6 @@ class Html5 extends Tech {
this.removeOldTracks_(techTracks, elTracks);
}

/**
* Called by {@link Player#play} to play using the `Html5` `Tech`.
*/
play() {
const playPromise = this.el_.play();

// Catch/silence error when a pause interrupts a play request
// on browsers which return a promise
if (playPromise !== undefined && typeof playPromise.then === 'function') {
playPromise.then(null, (e) => {});
}
}

/**
* Set current time for the `HTML5` tech.
*
Expand Down Expand Up @@ -1570,7 +1557,16 @@ Html5.resetMediaElement = function(el) {
* @method Html5#load
* @see [Spec]{@link https://www.w3.org/TR/html5/embedded-content-0.html#dom-media-load}
*/
'load'
'load',

/**
* A wrapper around the media elements `play` function. This will call the `HTML5`s
* media element `play` function.
*
* @method Html5#play
* @see [Spec]{@link https://www.w3.org/TR/html5/embedded-content-0.html#dom-media-play}
*/
'play'
].forEach(function(prop) {
Html5.prototype[prop] = function() {
return this.el_[prop]();
Expand Down
28 changes: 28 additions & 0 deletions test/unit/player.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1066,6 +1066,34 @@ QUnit.test('should be scrubbing while seeking', function(assert) {
player.dispose();
});

if (window.Promise) {
QUnit.test('play promise should resolve to native promise if returned', function(assert) {
const player = TestHelpers.makePlayer({});
const done = assert.async();

player.tech_.play = () => window.Promise.resolve('foo');
const p = player.play();

assert.ok(p, 'play returns something');
assert.equal(typeof p.then, 'function', 'play returns a promise');
p.then(function(val) {
assert.equal(val, 'foo', 'should resolve to native promise value');

player.dispose();
done();
});
});
}

QUnit.test('play promise should resolve to native value if returned', function(assert) {
const player = TestHelpers.makePlayer({});

player.tech_.play = () => 'foo';
const p = player.play();

assert.equal(p, 'foo', 'play returns foo');
});

QUnit.test('should throw on startup no techs are specified', function(assert) {
const techOrder = videojs.options.techOrder;

Expand Down

0 comments on commit 091bdf9

Please sign in to comment.