search

SWI-Prolog / packages-pengines

Pengines: Prolog engines
11 stars 13 forks source link

pengines.js dependency on jQuery in browsers #44

Open ridgeworks opened 5 years ago

ridgeworks commented 5 years ago

pengines.js has a somewhat poorly documented dependency on jQuery's network support, i.e., any HTML document using pengines must also use jQuery. Furthermore pengines.js will fail to initialize unless jQuery has already been initialized (code from pengins.js initialization:

if (typeof window === 'undefined') {
  // Node.js
  module.exports = Pengine;
  var najax = require('najax');
  Pengine.network = najax;
  Pengine.network.ajax = najax;
} 
else {
  Pengine.network = $;
  // Browser
  $(window).on("beforeunload", function() {
    Pengine.destroy_all();
  });
}

While this can be work by loading scripts in the correct order in the <head> element, that's discouraged since it delays loading and rendering of the <body> resulting in a "poor user experience" (not my words). So web content designers are encouraged to load scripts asynchronously; in fact that's generally the only way when scripts are in the body element. And when that's done the current pengines.js may fail if $ is not yet defined.

The bottom line is that pengines.js is quite sensitive to the the host HTML document structure. I can see a couple of ways of improving the situation.

  1. Remove the dependency so JQuery doesn't have to be loaded at all. It's fairly straightforward just to use the native XHR already in browsers; as the najax developer says "jQuery ajax is stupid simple." And, unless jQuery is required for some other reason, it's one less script to load, which is generally a good thing. But this may entail a fair amount of work, so as an interim solution:

  2. Move the initialization of Pengine.network into the constructor. The chances are good that by the time the application itself is running and wants to use a Pengine, jQuery will be ready to go. And, if not, at least the application has an opportunity to recover, e.g., delay and try again.

Perhaps there's a better solution, but 2. is usable, simple, and safe.

JanWielemaker commented 5 years ago

Makes sense to me. Please file a pull request (I guess for (2)).

ridgeworks commented 5 years ago

I think I've done what's required but I'm not used to generating pull requests so let me now if I've screwed things up.