does the same thing need to be done/checked on the system dashboard?

Yes. Now that you mention it, we do the same thing there too. Let me take a look and amend the PR.

Thanks. FYI the discover check on that page does not work correctly anyway as it only displays the existing value. If the component hasnt been opened yet then the check reports nothing to discover

@brianteeman I fixed the AJAX badges' requests.

About the Discover check, can you please open an issue and at-mention me? I will take a look but it's a completely different thing than what I am solving in this PR. You know what they say about scope creep ?

I only mentioned it so that you didnt waste time wondering why it didnt work ;)

The discover check issue comes from our new bonus program "pay for one bug and get the other one for free" ?

@brianteeman I was vaguely aware something is wrong but it never bothered me enough to deal with it. Well, I seem to have a couple of days with nothing overly pressing to do so I might just as well use them to fix some Joomla issues. Report it, tag me and I'll bring you its head on a stick ?

@richard67 I don't think I signed up for this bonus programme ? ?

As this is a bug fix shouldnt this pr be against 4.1?

It does add a new feature in the Joomla.Request JavaScript API. Sure it's fully b/c but I don't want to waste time arguing whether it's genuine new feature (goes to 4.2) or bug fix (goes to 4.1). Besides, 4.2 is two months out; that's nothing, especially considering that August everyone — including me, after 3 years — will be on vacation... If we were 3 or more months out I'd definitely try to push it for 4.1.

stop talking about vacations - you make me sad

I have tested this item ✅ successfully on 7bb0534

tested both home and system dashboards and confirm that the waterfall now shows as sequential

I think it is better to make a new method, kind of Joomla.enqueueRequest, that should collect list of Joomla.request, and call them one by one.
That will be much better than trying to make a Joomla.request monster.

@Fedik What you propose is a worse monster. We'd have to duplicate Joomla.Request's code because that function does not return the options or the data, it only returns the XHR. Moreover, XHR does not allow you to extend the onreadystatechange event handler, only to replace it. So at the very least we'd need to replicate the bulk of that method to get access to the newOptions to get access to the data AND the onreadystatechange handler — that's 80% of the Joomla.Request code. Moreover, this would mean that any change in Joomla.Request would have to be mirrored in the new method. This is really bad architecture.

Further to that, what you propose would not work for the simple reason that you can and should be able to enqueue a request while another one is in progress. What you propose would only work if you enqueue requests before triggering any of them. In fact, the way you describe it you could only enqueue requests coming from one source. Each plugin enqueueing its own requests would result in all queues being processed in parallel... which is the exact problem we are trying to solve here!

I took all of these issues into account before deciding to go for a minimal modification of Joomla.Request. Sure, we could pull the processQueuedRequests function outside of Joomla.Request but IMHO it's bad architecture as this method is not used by anything else. Having it inside the Joomla.Request scope makes more sense.

In any case, I just added 10 lines of code (count them!) in Joomla.Request. It's a far cry from making Joomla.Request a monster. Doing duplicated code would add well over 100 lines to core.es6.js, most of which duplicated, something which I would totally agree is a bloody monster! I chose DRY over the typical Joomla PR approach of "bash something together and pretend there's nothing wrong with it".

const queue = [];
const checkQueue = () => {
  const options = queue.shift();
  if (!options) return;
  options.perform = true;
  Joomla.request(options);
};

Joomla.enqueueRequest = function (options) {
  const onSuccess = options.onSuccess;
  const onError = options.onError;

  options.perform = false;
  options.onSuccess = (response, xhr) => {
    onSuccess && onSuccess(response, xhr)
    checkQueue()
  };
  options.onSuccess = (xhr) => {
    onError && onError(xhr)
    checkQueue()
  };
 
  queue.push(options);
  checkQueue();
}

How much I duplicate of Joomla.Request? ?

@Fedik You are a release lead and have commit rights. You can't possibly be writing this kind of bad code, can you? Sheesh! Okay, let me tell you everything that's wrong with your code just by reading it twice over.

If the onSuccess or onError handler throws then you never call checkQueue(). That's a brand new bug. In my code the next request is launched before we call the onError / onSuccess handlers exactly so we avoid this problem.

Further to that, what happens if I enqueue a request while another is running? Both requests run in parallel because checkQueue only checks if there are requests in the queue, not if there are other requests already executing. Therefore you are NOT fixing the problem we started to fix with this change. That's why I have a flag for request execution.

You have added more code than I did. By the time you fix the issues I told you about you are at twice the number of lines of code. Your solution is not economical.

You require the developer to use a different method (Joomla.enqueueRequest) which also does not return the XMLHttpRequest object. When used outside of core code (which admittedly does not use the XHR object as far as I remember) it's useful to have a reference to the XHR object.

And now let's close in for the kill. You have created a magic, invisible tie-in between Joomla.enqueueRequest and Joomla.request. If at some point Joomla.request is rewritten to use fetch() instead of XMLHttpRequest your Joomla.enqueueRequest fails and can't be easily refactored.

So, let's recap all the problems your code has and mine does not:

• More code
• New bug introduced
• Does not solve the original problem
• Does not work the same as Joomla.Request
• Makes future refactoring a minefield

@Fedik You are a release lead and have commit rights.

@nikosdion He's just a maintainer like I am, not a release lead.

@richard67 Dang, right. Tobias is the release lead for 3.10, not Fedir. My apologies @Fedik I misremembered. It's hard to remember the release leads with 3.10, 4.1, 4.2 and 5.0 being in active development. I am bad at remembering names as it is...

I'm here with @nikosdion. Better to introduce a new flag as 10 new lines is not that worse for queue support. I was just wondering if this can also be solved with promises in less than 10 lines ?

If the onSuccess or onError handler throws then you never call checkQueue().

That details, just switch positions: call checkQueue() first, and the calback is second, easy.
That was quick example.

Further to that, what happens if I enqueue a request while another is running? Both requests run in parallel because checkQueue only checks if there are requests in the queue, not if there are other requests already executing.

Who need queued request, they do queued reques, other request going as normal.
It not need to be queued when component need to make one ajax request in own view.

If at some point Joomla.request is rewritten to use fetch() instead of XMLHttpRequest your Joomla.enqueueRequest fails and can't be easily refactored.

At some point Joomla.request should return Promise, or be removed at all (in favor to use fetch()). Here queued: true does not helps either.

Any Queue or Ordering manipulation should be handled outside of Request.

I do not mind if it will be merged as it is, I have wrote my opinion ?

At some point Joomla.request should return Promise, or be removed at all (in favor to use fetch()). Here queued: true does not helps either.

THIS! Joomla 4 should never had rolled a wrapper on a legacy JS method. Fetch is the way to go. Also doing sequential fetches is as easy as reducing the array of fetches...
Also this functionality SHOULD NOT be part of the core.js, core.js at some point should be only the essential API methods and not a garbage can.

@laoneo I believe promises can be more economical but I don't have enough hands-on experience to confidently say I can do it right.

@Fedik you missed something important when you said this:

Who need queued request, they do queued reques, other request going as normal.
It not need to be queued when component need to make one ajax request in own view.

Please re-read the issue referenced in this PR. Okay? You did? Good.

The problem is that we have a bunch of QuickIcon plugins which do not know about each other, all sending a request one after the other. Their JavaScript files which do not and must not know about each other are executed faster than it takes for a request to finish executing. Therefore we end up having 4-5 requests being sent from the browser within half a second. This triggers the DoS protection of the server, some of these requests fail and users perceive Joomla as broken.

We need a magic way which will "hold onto" these requests and only fire them after the previous one has finished executing. That's the purpose of the queueing.

As implied from the above description, a new request may be added to the queue while a previous one is still executing. You must not execute it just yet! This is what my code does and yours does not. We need to execute the new request after the previous one finishes, be it successful or failed.

If we need to have the caller determine if the request needs to be executed our plugins are no longer DRY. They need to know about each and every other element on the page which might be executing requests, somehow access these xhr objects, inspect their status and decide if they are going to execute their own request. Not only this is convoluted and bad architecture, it also makes it impossible to work with third party plugins which by definition cannot know about each other and the core code cannot know about them!

Again, this is only possible with a FIFO queue which waits for request execution.

@dgrammatiko Yup, this kind of refactoring is on my radar. I need some more experience with fetch though before doing that ? I also don't think that reducing the array would work because we need to wait for each request to finish before sending the next one. Unless you mean using await in the reducer? This is the part I don't have a lot of hand-on experience with and why I didn't dare convert this to fetch() just yet.

I fully agree that core JS should be much lighter. Ideally all that code would be modules and the 3PD and core code would add them as dependencies in the joomla.asset.json. There is a small problem, though, in that it would be a b/c break and requires a lot of though about how to handle legacy. Probably core would have to be made into depending on all individual modules for b/c reasons and then we need to educate core developers and 3DPs on which modules contain which parts of the former core. This is something I'd like to work on my own stuff before making core suggestions, though. It sounds good in theory but having never tried it in practice I have no idea how it would work and what real-world problems I can't foresee I might bump into, you know?

@nikosdion there's already a piece of code in Joomla showcasing this reducing promises:

But fwiw this kind of functionality should not be part of core.js, it's extremely rare the need of sequential fetches and when it's needed the solution is couple of lines (the Array.reduce() part)

You must not execute it just yet! This is what my code does and yours does not.

Hm? My code do queued request in FIFO order.

@Fedik I was trying hard to give you an out without embarrassing you too much (not more than posting obviously broken code with a snide remark already did) but you won't let it go. OK, if you insist...

As stated in #38001 the root cause of the problem is the execution of multiple requests overlaps. This results in too many requests being executed in a short period of time which is perceived by the web server as a Denial of Service attack. As a result the server blocks requests from that IP address for a few seconds to a few hours. Therefore the requests fail to execute and the user perceives Joomla as broken. The solution to that is to somehow make these requests NOT overlap by executing them consecutively AND NON_OVERLAPPING, ergo each request CANNOT start before its previous has finished executing.

You are not doing that. You are immediately processing the queue, plucking the next item from the top of the queue as soon as you add it and immediately executing it. There is no check that the request you've already started has finished executing, therefore your requests overlap just like if you were using Joomla.Request directly.

If you try your code and look at your browser's dev tools Network pane you will see the same graph as the "Actual result BEFORE applying this Pull Request" in this PR. Let me give you an example using these timings and what would happen with YOUR code:

At 1.4s after the page loads, a piece of JS code adds options for a new request (A) which will take 0.135 seconds. You are immediately calling checkQueue(). This method plucks the topmost request of the queue and executes it.

The request A is currently running and the queue is empty.

At 1.45s after the page loads a piece of JS code adds options for a new request (B) which will take 2.9 seconds. You are immediately calling checkQueue(). This method plucks the topmost request of the queue and executes it.

The requests A and B are currently running and your queue is empty.

At 1.5s after the page loads a piece of JS code adds options for a new request (C) which will take 0.212 seconds. You are immediately calling checkQueue(). This method plucks the topmost request of the queue and executes it.

The requests A, B and C are currently running and your queue is empty.

At 1.55s after the page loads a piece of JS code adds options for a new request (D) which will take 0.306 seconds. You are immediately calling checkQueue(). This method plucks the topmost request of the queue and executes it.

The requests A (about to finish), B, C and D are currently running and your queue is empty.

This is exactly the same effect as calling Joomla.Request directly! You did not implement a queue, you did not solve the problem. Congratulations for drilling a big hole in the water.

@dgrammatiko Ah, I see what you mean.

ah, okay, missed one litle if(), but the idea still valid ?

btw, looking one step further, would be good to know a way to resolve Promise in sequence.
So we could do:

chain(fetch('foo.html')).then(... handle response);
chain(fetch('bar.html')).then(... handle response);

Use of reduce not really possible, because on the beginning the sequence is "not complete" and reduce not work with mutated array.

@Fedik It's actually possible. The code @dgrammatiko linked to shows us how (and it contains the missing piece for the puzzle for me).

The reducer runs against an array and takes two arguments, a callback and an initial value. Its callback takes two arguments, previousResult and item.

The missing puzzle piece for me was the initial value. If you use Promise.resolve(true) (documentation reference) you create an instantly resolving promise with a return value of boolean true. This is the previousResult passed to the callback the first time its called. The callback always does return previousResult.then( // Basically run the item here ). But Promise.Prototype.then returns a Promise itself. Therefore you are chaining the promises together. Each promise will execute when its preceding promise has resolved (which, for Fetch, means the HTTP request has concluded). The first Promise resolves instantly and that's how the whole shebang works.

I am still missing one little thing: what happens if you want to add a Promise to the queue after you have reduced the array. I suppose we need to hold the reduced queue somewhere.

I am not 100% sure that reducing the array would make sense in our case where we want to add Promises asynchronously to the queue. I need to think about it and run a few tests but the idea is promising (pun intended!).

In any case, I'd like this PR to be merged as-is and I'd propose moving Joomla.Request to Fetch on 5.0 since it's most definitely a b/c break! Currently, Joomla.Request returns an XMLHttpRequest object. We can't change its return type within the same major version but it's definitely ripe for an overhaul in 5.0.

I am still missing one little thing: what happens if you want to add a Promise to the queue after you have reduced the array. I suppose we need to hold the reduced queue somewhere.

This

If elements are appended to the array after reduce() begins to iterate over the array, the callback function does not iterate over the appended elements.

And this makes use of reduce not much helpfull to me.

@Fedik the fact that the array is not mutatable should not be a problem if you think how to orchestrate the whole procedure. Eg:

const queueOfFetches = [];
function enqueueFetches (fetch) {
  queueOfFetches.push(fetch);

  // Since the backend knows how many buttons were added we pass the count($buttons) to the JS
  // And we fire the orchestration when all the scripts were pushed in the queue
  // something like:
  if (Joomla.getScriptOptions('fetch.buttons') === queueOfFetches) {
    queueOfFetches.reduce(....
  }
}

Each button SHOULD call the function above instead of resoling the fetch on page load or any other event

@dgrammatiko I would very much prefer it if we did not have the backend know everything about how the frontend works and have it orchestrate things. I was thinking something along the lines of https://gist.github.com/nikosdion/908156641a83ab4b849ecaae9f42f48a (obviously needs some rejection handling, this is the minimum viable proof of concept I knocked out in like 5')

// Since the backend knows how many buttons were added we pass the count($buttons) to the JS

That not a Jedi way hehe

That not a Jedi way hehe

Here's another way:
Since all the scripts were loaded either with defer or type=module, the array would be populated before the DOMContentLoaded event:

  document.addEventListener('DOMContentLoaded', () => {
    queueOfFetches.reduce(....
  });

The list of ways to orchestrate this are many more than these two. My point is that the functionality is exclusive to this module (joomla module not js module) and the actual implementation with modern js is just a couple of lines.
The project should be looking forward to remove any functionality that is user land once the platform comes up with a native alternative, eg I added few months ago a sanitizer in the core.js but the platform will have this as a native API by the end of the year (chrome: https://chromestatus.com/feature/5786893650231296) so the plan should be to remove it (or for some time, feature detect and load it for the older browsers). The Joomla.request as I said before should already have been removed, the promise based Fetch doesn't need a wrapper and it's already supported on all browsers that J v4 is supposed to cover.

@nikosdion I was very close to what you made https://gist.github.com/nikosdion/908156641a83ab4b849ecaae9f42f48a
But you was first ?
I think that can work.

I have missed to override variable with previous Promise. That the key.

My result kind of this:

    function fakeFetch (n) {
        return new Promise((resolve, reject) => {
            //resolve('Works ' + n)
            setTimeout(() => {
                resolve('Works ' + n)
            }, 1000 * n);
        });
    }

    const chain = {};
    Joomla.chainPromise = function (group, promise) {
        if (!chain[group]) chain[group] = Promise.resolve();
        let prev = chain[group];
        prev = prev.then(() => {
            return Promise.resolve(promise)
        });
        chain[group] = prev;
        return prev;
    }

    Joomla.chainPromise('fetch', fakeFetch(1)).then((msg) => {
        console.log(msg);
    })
    Joomla.chainPromise('fetch', fakeFetch(2)).then((msg) => {
        console.log(msg);
    })
    Joomla.chainPromise('fetch', fakeFetch(3)).then((msg) => {
        console.log(msg);
    })

@Fedik Yup, I have a slightly simpler way, too. Give me a sec, I want to check something.

The bulk of it is this. Joomla.Request would use fetch and return its promise. Then all we need is:

let lastRequestPromise = Promise.resolve();

Joomla.enqeueRequest = (options) => {
   lastRequestPromise = lastRequestPromise.then(() => Joomla.Request(options));
}

I would also say that in the context of what Joomla.Request is meant to do it doesn't make sense to have multiple queues as they can be abused to the point that we come back to the problem of too many simultaneous requests. If we do want to do that we could possibly do

let requestQueue = {};

Joomla.enqeueRequest = (queueName, options) => {
   requestQueue[queueName] = (requestQueue[queueName] ?? Promise.resolve())
    .then(() => Joomla.Request(options));
}

yea

it doesn't make sense to have multiple queues as they can be abused to the point that we come back to the problem of too many simultaneous requests.

True, it does not need to be grouped when it only for request().
I made it grouped to have a "generic" helper, so it can be used somewhe else also. But not really important.

And I thought about changing Joomla.Request.
Probably to make the transition smooth as possible it is better to deprecate it, and add new method that return Promisse, instead of changing it. Or keep it as XMLHttpRequest constructor.
In one of future 4.x series. But that something for far future.
UPD: or add an option promise: true ?

Btw, fetch cannot fully replace XMLHttpRequest, it lack of progress event for upload.

@Fedik Correct, there is no way to track upload progress with fetch, only download progress. So, at the very least, we will need to keep Joomla.Request as-is because installing extensions requires upload tracking.

I will do another PR to introduce Joomla.fetch and Joomla.fetchQueued. I am adding it to my to-do list for tomorrow.

Let's leave this PR here the way it is. There are legitimate uses cases for people to want queued execution with Joomla.Request. At the very least I have one in Akeeba Backup ? If this PR works for you please do submit a successful test.

I will do another PR to introduce Joomla.fetch and Joomla.fetchQueued. I am adding it to my to-do list for tomorrow.

Please DON'T! There's literally no benefit to namespace or wrap a native method. Also the Docs both in MDN and web.dev are way better than any non existent docs of the Joomla.request (or any other method, tbh).
Just let ppl use fetch directly, nothing to be loaded in the core.js

About the Joomla.fetchQueued add it to a helper file in media/system/utils as it should only be loaded on pages that is needed.

The point is that core.js is loaded always in a render blocking fashion so basically it needs to be reduced and probably inlined (not to mention that it's already a minefield of XSS) instead of being inflated with methods that are used in few pages...

My 2c

There's literally no benefit to namespace or wrap a native method.

This is wrong, I'm afraid. We need to magically add the X-CSRF-Token HTTP header for non-GET requests.

Moreover, if we want to make it possible to queue requests we CAN NOT use fetch() directly. Once you run fetch() it returns a Promise and the request is already underway. This brings us back to the same problem we are trying to solve with this here PR!

I think can just add return lastRequestPromise in my enqueueRequest to allow something like

Joomla.enqueueRequest({
  url: 'index.php?option=com_example&view=ajax&format=json'
})
  .then(response => response.json())
  .then(data => {
    // Do something
  });

The Joomla.fetch would just be a simple stub to extract the url from the options and massage the headers as needed.

That's my overall idea, probably I need to change a few things. It's still very much in the concept stage.

I have tested this item ✅ successfully on 7bb0534

interesting discussion

This is wrong, I'm afraid. We need to magically add the X-CSRF-Token HTTP header for non-GET requests.

It's a line in the options, you seriously DO NOT need to wrap every native method but whatever...

Moreover, if we want to make it possible to queue requests we CAN NOT use fetch() directly. Once you run fetch() it returns a Promise and the request is already underway.

Wrong, all you need is an API to push the fetch in an array and that array will be then executed sequentially

@dgrammatiko Since you know better why don't you do the PR? Problem solved.

@dgrammatiko Since you know better why don't you do the PR? Problem solved.

There's no v5 Repo...

@dgrammatiko As we discussed above, we'd need to keep Joomla.Request as-is because fetch doesn't support tracking the upload progress (it can only track the download progress). The upload progress is used for the Upload & Install method. We can't change that in 4.x and probably not in 5.x either.

So whatever you add will be a new method, e.g. Joomla.enqueueFetch which accepts a Promise returned by fetch. You can redo this PR against 4.2 using fetch to show us how what you have in mind. Just remember, the server side cannot possibly know all the fetches which will be performed. It has to be a client-side-only solution. You said you know how to do it, show us how it's done, please.

And to clarify: I am not being sarcastic, I HONESTLY want to see how it's done!

I will do another PR to introduce Joomla.fetch

Other possible way, can work in 4.x (and all the way in future):

Joomla.request({
 url: 'foo-bar.json'
 promise: true // <- new option
}).then((response, xhr) => {
  console.log(response, xhr)
}).catch((xhr) => {
  console.error(xhr)
});

Then qeue (based on Nic comment):

Joomla.enqeueRequest = (options) => {
   options.promise = true; // Force promise
   lastRequestPromise = lastRequestPromise.then(() => Joomla.Request(options));
}

Joomla.enqeueRequest({
 url: 'foo-bar.json'
 promise: true
}).then((response, xhr) => {
  console.log(response, xhr)
}).catch((xhr) => {
  console.error(xhr)
});

I will say this a final time. Our requirements are really simple:

1. No request runs in parallel with another.
2. We can add requests to the queue at any point in time without the calling client-side code needing to know anything at all whatsoever about the state of other requests, without the server-side having to know about all requests in advance.

Dimitris' method cannot work without violating one of these conditions.

If you want to fulfil the first condition you need to do something like https://decembersoft.com/posts/promises-in-serial-with-array-reduce/ which only works if you know about all of the requests in advance. This violates the second condition.

If you fulfil the second condition you are pushing the promises returned by fetch() to an array. However, this does not prevent the request from starting, violating the first condition. Here's a simple POC and note that I'm not even trying to do anything with the array:

	const fetchQueue = [];

	for (let i = 0; i < 5; i++)
	{
		fetchQueue.push(fetch('https://www.exaxmple.com'));
	}

Result:

All the requests started executing immediately. We never even had the change to reduce the array.

I still don't see how you can possibly run fetch without running the request immediately when, um, this is exactly what it's supposed to do per https://developer.mozilla.org/en-US/docs/Web/API/fetch and I quote:

The global fetch() method starts the process of fetching a resource from the network, returning a promise which is fulfilled once the response is available.

Even the WhatWG specification says it will start the request immediately: https://fetch.spec.whatwg.org/#fetch-method

As for using fetch... If you don't want to enqueue, use fetch(). If you want to enqueue and/or want upload feedback use Joomla.Request. No further changes necessary in the core code. There is no point trying to write something convoluted which will reintroduce the problem we just fixed in this here PR. Just merge this PR and that's all there is to it. Over and out and have a good night, y'all!

Thanks everybody

This is still an issue in Joomla! 4.2.6