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

Synchronous AJAX should not be used #822

Open rasmk opened 8 years ago

rasmk commented 8 years ago

UI5 uses synchronous AJAX to load modules.

This effects in a warning in Chrome console:

Synchronous XMLHttpRequest on the main thread is deprecated because of its detrimental effects to the end user's experience. For more help, check https://xhr.spec.whatwg.org/.

The things have got much worse in the most recent release of Chrome, which has a bug in caching synchronous AJAX responses (https://code.google.com/p/chromium/issues/detail?id=583214). This caused my UI5 app not to bootstrap anymore.

Tried data-sap-ui-preload="async", but this did not solve the issue. Still the same warning, still the same caching issue. It seems, currently the only solution is to disable caching, but then the app will have to download several MB of libs at every startup.

Although it's a bug in Chrome, they (Chrome devs) could legally drop support for synchronous AJAX at any time. According to the AJAX spec, section 4.5.1:

Synchronous XMLHttpRequest outside of workers is in the process of being removed from the web platform as it has detrimental effects to the end user's experience. (This is a long process that takes many years.) Developers must not pass false for the async argument when entry settings object's global object is a Window object. User agents are strongly encouraged to warn about such usage in developer tools and may experiment with throwing an InvalidAccessError exception when it occurs.

As such, using synchronous AJAX seems to be a bug in the library implementation. Please switch to async.

emoticrab commented 8 years ago

Interestingly this is also what stops you being able to build FirefoxOS and Chrome Apps using UI5. Also why you have to comment those calls out to get it working in Atom.

codeworrior commented 8 years ago

No, it's not a bug, it's heritage.

When spec owners decide to change the spec in an incompatible way, you can't blame projects that have built something against the old spec.

However, my perception is that both, WhatWG as well as browser vendors, are quite aware of the effort that is necessary to move away from sync XHR to async. That's IMHO why they've chosen this unusual approach of logging a warning in the console when such usage is detected for the first time. Apps that use sync behaviour in only a handful of places can relatively easy switch to async. Others (like DOJO or UI5) have built a full set of synchronous APIs around the sync XHR and moving such frameworks towards async mode involves a lot more activity than just flipping the flag on the XHR.

In JavaScript, you can't transparently move away from sync to async execution, only the other way around. Instead, UI5 has to establish async alternatives step by step for all its sync APIs. New APIs that involve backend communication are only designed for the async mode. At the end we'll allow apps to switch off sync usage of XHRs (obviously, apps also have to use only async APIs then). For compatibility reasons we might even have to keep the sync way alive as long as browsers support it (Firefox is an exception, they've made sync XHRs useless on their platform years ago, but that's a different story).

Nevertheless, there is still some way to go before we can provide that switch.

Kind Regards, cw

P.S.: According to recent comments in the Chromium issue report, they regard this as a serious issue that'll be fixed soon.

emoticrab commented 8 years ago

Thank you for that detailed response.

matz3 commented 8 years ago

Related issue: #777

codeworrior commented 8 years ago

Just an update: Chrome 48.0.2564.103 is available now and according to first tests, it fixes the sync XHR 304 response.

rasmk commented 8 years ago

Thanks for the update @codeworrior. Chrome update seems to solve the issue indeed.

DerGuteWolf commented 8 years ago

The console warning now reads "Invoking 'send()' on a sync XHR during microtask execution is deprecated and will be removed in M53, around September 2016. See https://www.chromestatus.com/features/5647113010544640 for more details." and we have September 2016... What now?

codeworrior commented 8 years ago

First, the Chromium project postponed the removal by one milestone (M54, planned for October 2016), then abandoned it due too much negative feedback. See https://bugs.chromium.org/p/chromium/issues/detail?id=605517#c25 .

akudev commented 6 years ago

To avoid the impression nothing is happening here: getting rid of synchronous XHRs is not an atomic task. Every single usage has other effects on existing applications and public APIs when replaced by async calls. Work on this is in progress, some recent example commits are:

jonaszuberbuehler commented 4 years ago

@akudev Is it still the case that we can't use ServiceWorkers to make an app offline available (I'm using version 1.60.18)? There are still a couple of synchronous requests. Is there any workaround we can use now?

EDIT: I'm using ui5-tooling and serving the dist directory. If I'm using https://sapui5.hana.ondemand.com/1.60.18/resources/sap-ui-core.js directly in index.html, it seems to be all asynchronous calls. Is this intended behaviour?

EDIT 2: Using ui5 build --all will create the bundles for the dependencies which eliminates close to all sync requests! Kudos to @matz3! (original slack thread: https://app.slack.com/client/T0A7MQSJ1/C0A7MHSSJ/thread/C0A7MHSSJ-1583169609.007800)

image

RobertoMalatesta commented 2 years ago

Hi @codeworrior @akudev , any update on this issue? Any definitive solution? I would like to propose OpenUI5 on some large projects but this issue is the one that hangs like a sword of damocles. Many thanks,

R

akudev commented 2 years ago

Hi @RobertoMalatesta ,

by now, no parts of at least OpenUI5 should trigger sync requests on their own anymore. Plus, for all sync APIs in OpenUI5 (which we cannot simply remove, for compatibility reasons) there are now async API alternatives. The only known exception is the CLDR content (requests to e.g. resources/sap/ui/core/cldr/de.json, triggered e.g. when the DateFormatter is used), but this last gap is on the radar and going to be addressed.

So when we are talking about the fear of new projects breaking once sync XHRs no longer work in some future, the advice is simply to to only use async APIs (see the documentation for some of them) - potentially safeguarded by tests which detect any failure to comply. And of course use the async bootstrap and the library preloads which is app development best practice by now anyway.

Right now I don't know more details about nature and timeline of fixing the CLRD loading, but as it is a single known and worked-on issue, as sync requests are apparently not going to be disabled soon&suddenly, and as UI5 won't let thousands of SAP apps break when this finally happens, I think this is more like a little Damocles pocket knife held by a pretty solid rope.

Regards Andreas

P.S.: providing async APIs and avoiding sync loading +eval was also required by the Content Security Policy efforts.

RobertoMalatesta commented 2 years ago

Hi Andreas @akudev, thank you for the prompt answer and for all the work done your team did in this project, the most solid and rich enterprise library for Web UI.

R

Thodd commented 2 years ago

Hi @RobertoMalatesta,

there is also another github issue that we track wrt. the async CLDR json loading: https://github.com/SAP/openui5/issues/2345

There is also an internal backlog item for this topic: CPOUI5FOUNDATION-77

The CLDR data is json based and does not violate current CSP guidelines (meaning: no JS sources are loaded synchronously and no eval() is involved). So please understand that I can't comment on a timeline when this gap will be closed.

Adding to akudev's comment, there is also another exception/issue to the async story: https://github.com/SAP/openui5/issues/3331

There are scenarios that might lead to a sync. loading of i18n resources during app-start.

BR, T

codeworrior commented 2 years ago

3331 unfortunately does not mention in what context the i18n file still was requested synchronously. Was it due to texts in the manifest?

The newest internal backlog item is CPOUI5FRAMEWORK-303 which should establish async APIs for the framework-owned, local-related parts, incl. CLDR, calendar implementation and, to some degree, translated texts.

Thodd commented 2 years ago

@codeworrior: it wasn't part of the initial issue, but I discussed this with @pubmikeb a bit. To my understanding it is related to the usage of {{placeholder}} texts in the manifest and the way the Component factory handles these. I created an internal backlog item for this issue: CPOUI5FRAMEWORK-286 That should describe the problem in more detail (hopefully ;).

pubmikeb commented 1 year ago

The newest internal backlog item is CPOUI5FRAMEWORK-303 which should establish async APIs for the framework-owned, local-related parts, incl. CLDR, calendar implementation and, to some degree, translated texts.

@codeworrior, I'm just curious, if is there any update on the subject?

codeworrior commented 1 year ago

No substantial progress. Reg. the LocaleData part, it was picked up and discussed again in December by another team (responsibilities have been shifted internally) and is now tracked as CPOUI5MODELS-1092. As the new team is busy working on other topics with higher priority, I guess this cannot be addressed soon. Reg. the i18n texts, I'm not aware of any progress.

flovogt commented 9 months ago

Just wanna post some updates here: