Skip to content

window.Promise dependency #94

Closed
Closed
@wakamoleguy

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions