[WebProfilerBundle] Live duration of AJAX request #26668
Conversation
|
This looks pretty useful! |
| @@ -158,6 +158,7 @@ | ||
| durationCell.className = 'sf-ajax-request-duration'; | ||
| durationCell.textContent = 'n/a'; | ||
| row.appendChild(durationCell); | ||
| request.updateDuration = setInterval(request.updateDuration, 100, durationCell); |
There was a problem hiding this comment.
using request.updateDuration to store both the callback and the timer identifier after that looks wrong to me. It will make it much easier to introduce bugs by mistake in the future.
| @@ -264,6 +267,9 @@ | ||
| url: url, | ||
| method: method, | ||
| type: 'fetch', | ||
| updateDuration: function(element) { |
There was a problem hiding this comment.
you need to do the same in the code handling XMLHttpRequest. This code does it only for fetch (which will break stuff as startAjaxRequest is used by both)
| @@ -264,6 +267,9 @@ | ||
| url: url, | ||
| method: method, | ||
| type: 'fetch', | ||
| updateDuration: function(element) { | ||
| element.textContent = (new Date() - stackElement.start) + 'ms'; |
There was a problem hiding this comment.
instead of using a closure to keep a reference to stackElement.start here (which forces you to create the callback in 2 places to make it available for startAjaxRequest), you could instead also pass the start time as an argument in the setInterval call, as you do for the DOM node (and then, you can define the function only once as a sibling of startAjaxRequest and finishAjaxRequest, and reuse it for all requests).
This would also solve the re-purposing of the field.
240cfd5 to
f303e5b
Compare
|
Ok how about this? Even less lines and no need to pass arguments to closure \o/ |
f303e5b to
7696bf9
Compare
| @@ -159,6 +159,10 @@ | ||
| durationCell.textContent = 'n/a'; | ||
| row.appendChild(durationCell); | ||
|
|
||
| request.liveDurationHandle = setInterval(function() { | ||
| durationCell.textContent = (new Date() - request.start) + 'ms'; | ||
| }, 100); |
There was a problem hiding this comment.
Looks like there is an indentation issue here (tabs?).
7696bf9 to
1bfee88
Compare
|
@ostrolucky can you rebase your branch to fix conflicts ? |
1bfee88 to
eb6974d
Compare
|
sure, done! |
|
Thank you @ostrolucky. |
…rolucky) This PR was merged into the 4.1-dev branch. Discussion ---------- [WebProfilerBundle] Live duration of AJAX request | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #26645 | License | MIT | Doc PR | - This will spit out current calculated AJAX request duration, before it finishes. Solves my DX issue about not knowing if request is still in progress.  Commits ------- eb6974d [WebProfilerBundle] Live duration of AJAX request
This will spit out current calculated AJAX request duration, before it finishes. Solves my DX issue about not knowing if request is still in progress.