abdulhaq-e / ngrx-json-api

A JSON API client for ngrx
MIT License
60 stars 28 forks source link

concurrent requests getting cancelled due to switchMap #207

Open remmeier opened 7 years ago

remmeier commented 7 years ago

running two requests in parallel leads to cancellation of first request.

ghost commented 7 years ago

Is this fixed by now? If so, I'd be glad to receive at least a v1.1.1 or v1.2 ...

remmeier commented 7 years ago

Partially, I changed to mergeMap so it allow concurrent requests again. So the worst part is fixed.

But what it should further do is in case of concurrent requests to the same resource (multiple updates of same resource, multiple queries) it should cancel the old observables/effects. Not yet quite sure how, it would be something like a switchMap grouped by resource/id resp. queryid.

ghost commented 7 years ago

I do get the point, but this could be another issue, right? So, how about moving this to a new ticket and releasing the current solution. Your fix would help me out big time...

rkrisztian commented 7 years ago

I have a problem with mergeMap used again. Once I reported https://github.com/abdulhaq-e/ngrx-json-api/issues/128 which I can no longer reproduce. But now I have a new problem that when I navigate away from the page and return back to it, I see the old resource returned for this.newAnimal$, which is the one that I previously added to the list, rather than a completely empty one. With switchMap this does not happen...

abdulhaq-e commented 7 years ago

We need some expertise in rxjs to find a compromise...

remmeier commented 7 years ago

maybe an additional takeUntil(...) can save the day.

  1. For API_GET_INIT is only active as long no other API_GET_INIT with the same queryId was executed and the queries[queryId] still exists in the store.
  2. API_PATCH_INIT and API_POST_INIT are only active as long as the same operation with the same type and id is executed.
  3. LOCAL_QUERY_INIT same as API_GET_INIT, the gets cancelled by a subsequent LOCAL_QUERY_INIT or if that particular query was removed from the store.

this solution would be more specific that just the generic switch map.