SAP / openui5

OpenUI5 lets you build enterprise-ready web applications, responsive to all devices, running on almost any browser of your choice.
http://openui5.org
Apache License 2.0
2.96k stars 1.24k forks source link

sap.ui.model.json.JSONModel: Return Promise before ajax request starts/finishes #2282

Closed cmastrandrea closed 5 years ago

cmastrandrea commented 6 years ago

OpenUI5 version: latest

Browser/version (+device/version): N/A

Any other tested browsers/devices(OK/FAIL): N/A

URL (minimal example if possible): https://github.com/SAP/openui5/blob/master/src/sap.ui.core/src/sap/ui/model/json/JSONModel.js

User/password (if required and possible - do not post any confidential information here):

Steps to reproduce the problem: This is more of a question/request than a problem. In the loadData function of JSONModel, it relies on firing events to indicate when an ajax request has started/finished, rather than using deferreds/promises.

Could the code be modified to return a promise before the ajax request starts/finishes, so that the caller can add their own handler for the request? Something like:

        if (bAsync) {
            pImportCompleted = new Promise(function(resolve, reject) {
                var fnReject =  function(oXMLHttpRequest, sTextStatus, oError) {
                    reject({request: oXMLHttpRequest, textStatus: sTextStatus, error: oError});
                };
                _loadData(resolve, fnReject);
            });

            this.pSequentialImportCompleted = this.pSequentialImportCompleted.then(function() {
                //must always resolve
                pImportCompleted.then(fnSuccess, fnError).catch(function(oError) {
                    Log.error("Loading of data failed: " + oError.stack);
                });
                return pImportCompleted;  // NEW
            });
        } else {
            _loadData(fnSuccess, fnError);
            return new Promise(function(resolve,reject) { resolve(); });  // NEW
        }

That way, the caller can do something like: var oPromise = oModel.loadData(sUrl).then(fnSuccess,fnError);

What is the expected result?

What happens instead?

Any other information? (attach screenshot if possible)

boghyon commented 6 years ago

There is a similar issue requesting a promise for when the data have been loaded: https://github.com/SAP/openui5/issues/2213

mauriciolauffer commented 6 years ago

Hi @cmastrandrea

Basically, you have two options:

  1. Using events from JSONModel (requestCompleted, requestFailed, requestSent) https://openui5.hana.ondemand.com/#/api/sap.ui.model.json.JSONModel/events

  2. Using a custom library (OpenUI5 CRUD JSON Model) which uses JSON Model and promises https://github.com/mauriciolauffer/openui5-model-json-crud

cmastrandrea commented 6 years ago

Understood, but my question is if this small change can be made to the sdk to allow consumers to use the simplicity of promises without requiring every developer to resort to using a third-party library.

The openui5-model-json-crud description seems to be implying that it can return Promises because it's using fetch vs ajax, and I'm not understanding that.

mauriciolauffer commented 5 years ago

@cmastrandrea https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API

cmastrandrea commented 5 years ago

I understand fetch uses Promises, but so can Ajax.

mauriciolauffer commented 5 years ago

No. Fetch is Promise based. Ajax is event based. Fetch always returns a Promise. Ajax always returns an event. You can wrap an Ajax request in a Promise or raise events from a Fetch request, however it doesn't make them the same thing.

cmastrandrea commented 5 years ago

The question remains - can this small change be made to the SDK to let consumers use the promise that the code is already using?

trevornc commented 5 years ago

@mauriciolauffer Thank you for taking the time to respond. The point of this question is that the JSONModel code as shown above creates Promise objects, however they are never exposed (shared) with the calling code. If the Promise object was returned then the consumer would be able to choose between listening to the events (requestCompleted/requestFailed) and/or the resolve/reject of the Promise.

Is there a good reason not to return (share) the Promise object ?

Thodd commented 5 years ago

Hi guys,

sorry but this thread somehow slipped through our fingers :) We are currently looking into this, and we will most likely introduce a promise as proposed by @cmastrandrea . Please also see the issue linked by @boghyon .

Thanks, Thorsten

IlianaB commented 5 years ago

Hello @cmastrandrea ,

Thank you for sharing this finding. An internal incident 1880574213 has already been created as mentioned previously. The status will be updated in GitHub in the above mentioned issue: https://github.com/SAP/openui5/issues/2282 Please, follow the progress there.

Thank you, guys, for handling this problem.

Regards, Iliana