fix(useAxios): ignore the result of a superseded request - #5576
Conversation
@vueuse/components
@vueuse/core
@vueuse/electron
@vueuse/firebase
@vueuse/integrations
@vueuse/math
@vueuse/metadata
@vueuse/nuxt
@vueuse/router
@vueuse/rxjs
@vueuse/shared
@vueuse/skills
commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5576 +/- ##
==========================================
+ Coverage 68.01% 68.03% +0.01%
==========================================
Files 349 349
Lines 8282 8283 +1
Branches 2549 2550 +1
==========================================
+ Hits 5633 5635 +2
+ Misses 2164 2163 -1
Partials 485 485 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
MILLERMARRU
left a comment
There was a problem hiding this comment.
Traced this through and it holds up. Confirmed the finally block already uses currentExecuteCounter === executeCounter as the staleness guard, so extending the same check to .then/.catch is consistent with the existing convention rather than introducing a new one.
I pulled the counter logic out into a standalone script to stress a couple of orderings that aren't obvious from reading alone (stale resolves after fresh, stale rejects after fresh, and both settling in the same microtask batch). All three land on the fresh result with no leakage from the stale one:
Case 1 (stale resolves after fresh): { data: 'data-fresh', onSuccessCalls: [ 'fresh' ] }
Case 2 (stale rejects after fresh): { data: 'data-fresh', errorVal: null, onErrorCalls: [ 'stale' ] }
Case 3 (same-tick race): { data: 'data-fresh0', onSuccessCalls: [ 'fresh0' ] }
Worth calling out since it's easy to second-guess: leaving onError unguarded (case 2 above, onErrorCalls still contains 'stale') is correct, not an oversight. The existing should be loading on re-execute test asserts onError fires twice for two superseded default-abortPrevious calls, so guarding it the same way as data/onSuccess would break that test. Good that the PR body calls this out explicitly rather than leaving it to be rediscovered.
execute()only guarded its.thenwithisAborted, and any newerexecute()call resets that ref tofalse, so a superseded request could still write its response intodata/responseand callonSuccessafter the newer request had already settled. The.catchbranch had no staleness check at all, so a late rejection overwroteerrortoo.Both branches now compare
currentExecuteCounteragainstexecuteCounter, the guard the.finallyright below already uses.onErrorstill fires for a superseded request, since the abort behaviour covered by the existing tests depends on it.Fixes #5575