change portalDetail.request to return a jQuery promise

[terencehonles]

portalDetail.request returns a jQuery promise which if the request is successful resolves to the value which would be returned by portalDetail.get otherwise the promise is rejected.

[nexushoratio]

Why not a standard JS Promise instead?

[terencehonles]

@nexushoratio because iitc is centered around jQuery - I didn't want to add a different dependency and I didn't check if JS Promises are used elsewhere.

[terencehonles]

@nexushoratio I just checked and they're not, so it's safer to use jQuery.Deferred rather than add an implicit dependency. Originally it was because I would be returning ajax().promise(), but since I was making the change I figured it would be better to handle the RETRY scenario and return a separate promise.

[nexushoratio]

Using a built in feature of the language is not an implicit dependency.

Neither of jquery's promise or deferred is currently used in the code base either.

[terencehonles]

/respectfully: I'm not sure how long you have been doing software engineering, but it is indeed creating a dependency.

Promises are only a language feature for certain versions of JavaScript. I am personally not sure which versions iitc is targeting and by using window.Promise in an arbitrary file I would be creating a hard to find(/what I meant by implicit) dependency.

I did check the code and it does seem that deferred is not being used, but the difference between that and window.Promise is that I know for sure that the dependency will be fulfilled because jQuery provides it (so it's not a new dependency).

[nexushoratio]

Likely longer than you've been alive.

JS Promises have been out for a while now.

All of the major browsers, desktop and mobile, except for IE, support it. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise#Browser_compatibility

[terencehonles]

Unless you're the owner/maintainer of the project (and you haven't said that yet) you haven't given me a concrete reason why your opinion is more correct than mine. A decision like this should not be just made by a couple of contributors. Unless you want to point me to a document that describes what browsers iitc is targeting or bring in another contributor I have no reason to change the code as proposed.

If you would rather see window.Promise being used you may re-write my code and propose that PR instead. I really don't care, the point is I made the decision based on the information I had and you still haven't given me a good reason to think otherwise.

Until any promise like solution is achieved I am currently pleasantly using window.activeRequests, but I do look forward to not having to use that work around.

[McBen]

let's get back to a basic question: For what is it good for? What's the difference to the "portalDetailLoaded" hook?

[terencehonles]

@McBen personally I need the value for multiple portals.

$.when(
      portalDetail.request(guid1),
      portalDetail.request(guid2), 
      ...portalDetail.request(guidN)).then(function(p1, p2, ...pN) { ...do work... })

is much easier to express/more elegant than:

portalDetail.request(guid1);
portalDetail.request(guid2);
...
portalDetail.request(guidN);
addHook('portalDetailLoaded', function(...) {
     if (...not all loaded...) return;
     ...do work...
});

[dingram]

My concern is that this would introduce some inconsistency to the IITC API. Why should just one call use promises, instead of all of them?

Perhaps an alternative way to do this would be to provide a plugin that provides a promise-based wrapper around IITC's existing API. That would also avoid the issue of which style of promises to use: two libraries could be written, one for jQuery and another for native ES6.

My feeling is that promises may otherwise have to wait until the IITC core gets rewritten.

[terencehonles]

@dingram do you know the time frame or progress for a potential IITC core re-write? I agree it may be inconsistent, but it would be nice if we document and move towards providing promises instead of solely on hooks. I do prefer ES6 promises, but until we have some sort of roadmap we're just kinda floating around.

[pleasantone]

I like @dingrams idea of a hook wrapper so you get your promise support before a core rewrite and we get an API for all hooks, not just one.

Would you care to submit a PR that covers all hooks? It's more likely to be accepted than this one.