Janus-gateway: janus.js should not depend on JQuery

Created on 1 May 2017  路  11Comments  路  Source: meetecho/janus-gateway

The core janus.js library need not depend on JQuery.

Currently JQuery is used for two purposes: $.ajax() requests to Janus gateway and to check an element for a Chrome extension id/marker value. In modern and even quite a few dated browsers, $.ajax() can be replaced with fetch() and the check for the marker could be rewritten quite straightforwardly with document.querySelector()

Benefits are:

  • One dependency less for the core janus.js library (and there are polyfills for folks who need backwards compatibility with browsers that lack a native fetch() implementation).
  • With fetch() the code is typically quite a bit more compact, and it would yield easier to test code compared to nested JQuery callbacks resulting from nested $.ajax() calls.

Would you be interested in merging & carrying a PR that addresses this?

Most helpful comment

Sorry for the ultraslow response on this, but I've been busy on other stuff and completely forgot about this (going through some other old issues right now for the very same reason!).

If you guys can come up with a simple pull request that makes janus.nojquery.js less prone to problems (e.g., "better" AJAX shim, in case it needs love), and replaces janus.js, I'll definitely consider it for merging (you're probably much better at JS than me!). It would also give people a sandbox to test it in multiple WebRTC compliant environments (Chrome, Firefox, Edge, Safari, mobile browsers), something I most definitely can't do all by myself...

All 11 comments

I'm guessing you have already seen janus.nojquery.js?

Yeah that's almost but not quite what I'm after. ;) I'm proposing to use the native API of modern browsers, which also happens to give you a nicer async programming model (Promises instead of callbacks in callbacks). In fact, it's quite likely that in the browser you are using right now, XHR is actually a backwards compatibility, make-believe shim layered on top op fetch().

Also, what I somehow forgot to mention: neither janus.js nor janus.nojquery.js makes it easy to augment outgoing requests or intercept responses (for example to work with custom HTTP headers). If the gatewayCallbacks parameter to the Janus constructor was extended with a couple of optional arguments/options, code like this could be the basic pattern

var outgoingRequestFilter = ... // passed in or just identity function to continue
var responseInterceptFilter = ... // passed in or just identity function to continue

function doFetch(url, options) {
  return new Promise(function(resolve) {
    resolve({url: url, options: options});
  })
  .then(outGoingRequestFilter || function(x) { return x; })
  .then(function(request) {
    if(request) {
      return fetch(request.url, request.options);
    }
  })
  .then(function(response) {
    if(response.ok) {
      return response;
    }
    else { // HTTP request succeeded, but HTTP status code indicates an error response
      return Promise.reject({
        status: response.status,
        statusText: response.statusText,
        response: response
      });
    }
  })
  .then(responseInterceptFilter || function(x) { return x; })
  .then(function(response) {
    if(response) { // parse JSON response payload
      return response.json(); 
  });
}

Calling code would do something like this:

// long poll
var longPollQueryParams = '?rid=' + new Date().getTime() + ... ;
doFetch('/' + sessionId + longPollQueryParams, {method: 'get', credentials: 'omit', cache: 'no-cache' })
  .then(successHandler)
  .catch(errorHandler);

That depends on how widespread these features you mention are. The purpose of janus.js has always been to provide a very simple (maybe not always the best) way to talk to Janus, and in a way that is sure to work on all compliant browsers (hence why I chose jQuery at the time, because I'm a lazy and poor JavaScript developer). The janus.nojquery.js version was built for those who didn't want jQuery in the library, but still using stuff that may be boring but should work everywhere.

Why not exposing the changes you'd like as a separate project, e.g., a separate client stack that people can use instead of the plain janus.js? That would allow you to have complete freedom on how you'd like it to look like, whether you keep it close to the original janus.js API or not. That's the road most of the available resources took, and I frankly think it's really cool to have different option and not be forced to be stuck with what we provide in that sense.

Why not just have janus.nojquery.js become janus.js, and the existing janus.js could be removed? I do agree with the sentiment that janus.js should not rely on jQuery.

The only differences in the janus.nojquery.js flavor are:

  • getElementById (Supported since Chrome 1, Edge, Firefox 1.7, IE 5.5, Opera 7, Safari 1)
  • Array.isArray (Since Chrome 5+, Edge, Firefox 4, IE 9, Opera 10.5, Safari 5)
  • XMLHttpRequest (Variable introduction dates depending on methods, but generally IE 9+)
  • An easily replaced use of jQuery.noop (a function that, by definition, does nothing)

What exactly is gained from using/supporting the version with jQuery at this point? It doesn't seem as though there are any real compatibility gains.

On the other hand, it seems like adding support for fetch would be better accomplished through a Promise-focused rewrite of janus.js considering everything else is already callback-based, which, I agree with Lorenzo, shouldn't be his responsibility to deal with.

The reason why I'm keeping the jQuery version around is simply that I trust their code much more than mine, when it comes to JavaScript and browsers compatibility... :wink:

XMLHttpRequest is a good example. I added some sort of shim that should hopefully universally work, but I'm pretty sure in all their years of experience the jQuery guys came up with tons of fine tunes and fixes for obscure problems they encountered with browsers. So it's easier to just let the smarter guys take care of what they do best!

Anyway, as I said other times, I'm not married to that: those are simply the reasons why I came up with that in the first place at the time. I personally see no harm in keeping both the jQuery and non-jQuery versions around, as it suits people in both camps, and since all of my demos make use of jQuery anyway, there's no point for me to get rid of the jQuery one.

Well, I think there's still a good reason to work toward only having one flavor of janus.js in that it makes ongoing maintenance easier and there's less likelihood of accidentally having things missing in one file (right now I think there is an extra try/catch in the jQuery version not tracked in the non-jQuery version). We could at least make the non-XMLHttpRequest changes which are very low-risk and re-evaluate your Janus.ajax shim to see if there's any reason it would give us problems that jQuery would have covered.

Sorry for the ultraslow response on this, but I've been busy on other stuff and completely forgot about this (going through some other old issues right now for the very same reason!).

If you guys can come up with a simple pull request that makes janus.nojquery.js less prone to problems (e.g., "better" AJAX shim, in case it needs love), and replaces janus.js, I'll definitely consider it for merging (you're probably much better at JS than me!). It would also give people a sandbox to test it in multiple WebRTC compliant environments (Chrome, Firefox, Edge, Safari, mobile browsers), something I most definitely can't do all by myself...

Thinking about it, the easiest thing to do is to extend the Janus constructor and/or library init functions with an additional dependencies parameter, that is just a property list which fills in the missing 'pieces' for Janus.js to work. Then there could be a function called Janus.simpleDependencies() that returns a suitable default implementation of that property list. JQuery can & probably should be excised from janus.js code entirely:

  1. For $.ajax() there is fetch(). The default dependency implementation for this should wrap fetch() and basically do: fetch().then(onsuccess, onerror). Existing flow control with janus.js can then remain intact.
  2. For $() there is document.querySelector(), the default dependency implementation for this should just use that.
  3. IIRC that just leaves jQuery.noop() which is just not worth it: we can write a no-op ourselves just fine.

That leaves the WebRTC adapter. Default dependencies should simply include the adapter object, and people who do customise the deps are expected to simply pass it in.

Finally, a compatDependencies() function may be useful, and perhaps should even be used to select the 'default' deps. Its implementation would be just a bit of indirection to get back at the current control flow. The only reason for including it is for existing applications which rely on the assumption that janus.js uses the various global objects of JQuery, $ & adapter.


This does not get you a clean-room implementation of how control flow would be done with modern browsers, but it does make it significantly easier:

  • For client code to integrate with Janus without pulling in unwanted dependencies
  • Integrating it with modern build-tooling like rollup and webpack (already doable, but the specific tooling hacks/configs do not necessarily translate well to application scaffolding frameworks like create-react-app which disallow such hacks and cause the build to fail if you try and use them.)
  • The inversion-of-control code flow pattern means you can inject custom behaviour because since client code is in control of the deps, it is also in control of when callbacks are dispatched. This means that a custom implementation of the deps can simply be a trivial facade to your favourite event dispatching $framework (e.g. redux, cycle.js).

    • Using janus.js in the context of mobile apps seem like it should definitely become possible, the only question is WebRTC support; maybe the janus.js code can even run on node.

We are not using browser side of janus, but i like what youve done here. Its more cleaner this way.

Closing as we merged #991.

Was this page helpful?
0 / 5 - 0 ratings