Description
There are several issues with the recent addition of ECMAScript 6 Promises into SIP.js.
First of all, and most egregiously, the existence of Promises are never checked before their use. The general idea seems to have been to assume "If a browser supports WebRTC, then it must support Promises." That is a dangerous assumption, as there is nothing preventing a browser from implementing WebRTC before Promises.
On a similar note, we do assume that the JavaScript environment running SIP.js supports WebSockets. This is a much looser requirement than WebRTC. One could argue that if Promises were supported on all platforms that supported WebSockets, we would be okay. That said, they aren't, and the code using WebSockets is limited to the (easily replaceable) Transport.js file.
The other issues are all in the manner in which Promises have been used. We are looking at the Utils.js file here. The addition of Promises in commit a9542a6b added a dependency on window
, without adding it to the module function. This "works," as Utils.js is required from the main SIP.js closure, which has a defined window
object. It mucks with the dependencies, though, making them hard to track and easy to break.
The previous would be an issue, if Promises should even be referred to as window.Promise
. Notice that since 2992938, only non-ECMAScript globals are referenced with window
. That includes things like WebSocket, window.navigator
, etc. ECMAScript globals, however, should not need to depend on having a single 'global' object that we can reference, as they should be present in browsers, Node, etc.
Lastly, the placement of the Promises in the Utils.js file makes it very tempting to be used outside of the WebRTC checks. That would cause this seemingly useful function to remove SIP.js support for half of the major browsers.
Platform support links, for reference:
http://kangax.github.io/compat-table/es6/#Promise
http://caniuse.com/#search=promise
This cannot make it into a tagged release as is.
Activity