Skip to content

fix(loading-spinner): stuck spinner after rapid concurrent requests#486

Merged
nicdavidson merged 1 commit intodevelopfrom
fix/loading-spinner-race
Apr 22, 2026
Merged

fix(loading-spinner): stuck spinner after rapid concurrent requests#486
nicdavidson merged 1 commit intodevelopfrom
fix/loading-spinner-race

Conversation

@nicdavidson
Copy link
Copy Markdown
Contributor

Summary

DfLoadingSpinnerService.active decided whether to schedule a BehaviorSubject emit by comparing shouldBeActive against active$.value — stale during rapid toggles because emits are deferred with setTimeout. When three concurrent requests started and finished inside one tick (event-script create form: services list + two source control,file lookups), the final decrement was evaluated while pending setTimeouts hadn't fired, so the false !== false check evaluated false and the deactivate was skipped. The earlier queued next(true) then fired and the spinner was stuck on until navigation.

Fix

Decide transitions from the counter directly (wasActive vs isActive) and use queueMicrotask. queueMicrotask still runs after the current change-detection pass (no ExpressionChangedAfterItHasBeenCheckedError) but before the next macrotask, which keeps emit ordering consistent with request ordering.

Also rebuilds dist/ so composer consumers pick up the fix.

Test plan

  • Reproduced on event-script create form (stuck spinner after 3 concurrent GETs)
  • Fix verified on local dev docker (spinner clears as soon as last request resolves)
  • Shipped to customer (Triskele) via pinned composer lock, confirmed working
  • CI passes on this branch

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.
@nicdavidson nicdavidson merged commit 592631f into develop Apr 22, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant