Skip to content

Commit 592631f

Browse files
nicdavidsonnicdavidson
committed
fix(loading-spinner): stuck spinner after rapid concurrent requests
The loading interceptor toggles DfLoadingSpinnerService.active per request. The service maintains a counter but decided whether to schedule a BehaviorSubject emit by comparing shouldBeActive against active$.value — stale during rapid toggles because emits are deferred with setTimeout. Reproduction: event-script create form fires three concurrent GETs (services list + two source control+file lookups). All complete inside one tick; the final decrement is evaluated while pending setTimeouts haven't fired, so the 'active$.value (false) !== shouldBeActive (false)' check is FALSE and the deactivate is skipped. The earlier queued next(true) then fires and the spinner is stuck on forever until navigation. Fix: decide transitions from the counter directly (wasActive vs isActive) and use queueMicrotask. queueMicrotask still runs after the current change- detection pass (so no ExpressionChangedAfterItHasBeenCheckedError) but before the next macrotask, which keeps emit ordering consistent. Also rebuilds dist/ so composer consumers pick up the fix.
1 parent 7d5461c commit 592631f

4 files changed

Lines changed: 17 additions & 11 deletions

File tree

‎dist/index.html‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,5 +10,5 @@
1010
<body class="mat-typography">
1111
<df-root></df-root>
1212
<script type="text/javascript" src="https://assets.calendly.com/assets/external/widget.js"></script>
13-
<script src="runtime.52376ff5d6ec6149.js" type="module"></script><script src="polyfills.cb64ea9d35bc0a9e.js" type="module"></script><script src="main.463c52b33f808aff.js" type="module"></script></body>
13+
<script src="runtime.52376ff5d6ec6149.js" type="module"></script><script src="polyfills.cb64ea9d35bc0a9e.js" type="module"></script><script src="main.c130ec302a7a706b.js" type="module"></script></body>
1414
</html>

‎dist/main.463c52b33f808aff.js‎

Lines changed: 0 additions & 1 deletion
This file was deleted.

‎dist/main.c130ec302a7a706b.js‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/app/shared/services/df-loading-spinner.service.ts‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,21 +13,27 @@ export class DfLoadingSpinnerService {
1313
}
1414

1515
set active(value: boolean) {
16+
// Decide on the transition from the counter itself, not from active$.value.
17+
// Rapid toggles schedule emits via queueMicrotask; during that window
18+
// active$.value is stale, so comparing against it loses decrements when
19+
// several requests start and finish inside one tick (spinner stuck on).
20+
const wasActive = this.activeCounter > 0;
1621
if (value) {
1722
this.activeCounter++;
1823
} else {
1924
this.activeCounter = Math.max(this.activeCounter - 1, 0);
2025
}
26+
const isActive = this.activeCounter > 0;
2127

22-
const shouldBeActive = this.activeCounter > 0;
23-
24-
// Only defer if the value is actually changing to avoid unnecessary timeouts
25-
// This prevents ExpressionChangedAfterItHasBeenCheckedError by ensuring
26-
// the value change happens after the current change detection cycle completes
27-
if (this.active$.value !== shouldBeActive) {
28-
setTimeout(() => {
29-
this.active$.next(shouldBeActive);
30-
}, 0);
28+
if (wasActive !== isActive) {
29+
// queueMicrotask runs after the current change-detection pass (so no
30+
// ExpressionChangedAfterItHasBeenCheckedError) but before the next
31+
// macrotask, which keeps emit ordering consistent with request ordering.
32+
queueMicrotask(() => {
33+
if (this.active$.value !== isActive) {
34+
this.active$.next(isActive);
35+
}
36+
});
3137
}
3238
}
3339
}

0 commit comments

Comments
 (0)