Use standard `fetch` API instead of jQuery for Ajax

[jamesnvc]

Hello! If people are interested, I ported the pengines.js to not depend on jQuery, so it can be fully stand-alone.

It uses the fetch API instead of jQuery.ajax; while this is supported by newer browsers, there is a polyfill available to support older browsers.

[jamesnvc]

Oh, the other ES6 feature that this uses is template strings. I can remove those to preserve compatibility with Internet Explorer, if desired.

[JanWielemaker]

I'm traveling with only a phone.  Basically I'm in favor of dropping jQuery dependency, but compatibility is high on the list as well.Op 26 mrt. 2018 15:16 schreef James Cash notifications@github.com:Oh, the other ES6 feature that this uses is template strings. I can remove those to preserve compatibility with Internet Explorer.

—You are receiving this because you are subscribed to this thread.Reply to this email directly, view it on GitHub, or mute the thread.

[jamesnvc]

I've removed the template strings, so I believe that now the only possible-unsupported feature this introduces is fetch, which in turn needs promises. For compatibility, people can include the following polyfill (which will also pull in the dependency on Promises.

<script src="https://cdn.polyfill.io/v2/polyfill.min.js?features=fetch"></script>

[JanWielemaker]

Hmmm. The phantomjs tests fail because fetch doesn't exist. Annotated a clear bug while trying to run SWISH using your version. Although I very much like to get rid of jQuery as a dependency, I still wonder whether the time is ready for fetch. Using polyfill might be nice, but doesn't cover offline usage which isn't uncommon for Pengines. Can we include the work-around?

[jamesnvc]

For offline use, I suppose we could bundle https://github.com/github/fetch + https://github.com/taylorhakes/promise-polyfill

[jamesnvc]

That is, if I understand correctly, we could add script tags for https://raw.githubusercontent.com/github/fetch/master/fetch.js and https://github.com/taylorhakes/promise-polyfill/blob/master/dist/promise.min.js to make fetch work with phantomjs?

[JanWielemaker]

Pushed the collected stuff to a branch fetch. Testing didn't work properly anyway, so I moved that to slimerjs, which does pass with your code as it runs firefox in headless mode. Still making up my mind about the dependency. I'm thinking about https://philipwalton.com/articles/loading-polyfills-only-when-needed/, accepting that offline work won't work out-of-the-box for IE browsers. Then the issue is where to place this code? Inside pengines.js? As a file next to it? In SWISH, which is the main application anyway? Opinions are welcome ...

[jamesnvc]

Then the issue is where to place this code? Inside pengines.js? As a file next to it? In SWISH, which is the main application anyway?

My impulse would be to suggest that it should be in the application that needs it, so SWISH in this case....

[JanWielemaker]

I already added a compatibility helper to pengines.js. To some extend you can argue this is not the right place, but it is short and should fix things out of the box for most cases while allowing for more dedicated solutions.

[JanWielemaker]

A bigger worry is that .abort() was not working and in general error handling was pretty much broken. I think it is now at least to some extend working again. Tested abort on FF and Chrome. Didn't test other error handling. Could you have a look at the changes?

[jamesnvc]

Ah, okay, I will have a look.

Sorry about things not working -- I've just started learning to use pengines, so I don't have a full grasp on how error reporting & such is supposed to look.

[JanWielemaker]

It is only very partly your fault. The unit tests are far from complete and were even dysfunctional due to a bug in phantomjs. I restored that functionality by moving to slimerjs. There should be tests for error handling and aborting in there.

[JanWielemaker]

I'm afraid I had to revert this. I have been spending most of the afternoon finding why swish threads do not exit properly after a user abort. It turns out there are two main reasons: (1) issues with error handling, e.g., the code throws new Error(responseobject) and than catches this, but Pengine.error doesn't handle Error objects and Error objects seem to require a string. More severe is (2): the lack of a portable and properly functioning way to abort fetch() that screws up the flow of events on an abort. This can possibly be fixed using additional server side logic, but given that we had a pretty much reliable implementation, switching back seems a safer and quicker option.

 Sorry --- Jan

[jamesnvc]

Oh dear, sorry to hear that. I really appreciate your patience & explanation.

[JanWielemaker]

I appreciate your contribution and hope it will eventually materialize, Using jQuery for pengines.js seems silly.