Framework: dispatchRequest update (sites users select)
#27615
Conversation
|
That's a great PR description, thank you so much for your effort! Generated by 🚫 dangerJS |
| dispatch | ||
| ) | ||
| ); | ||
| return map( normalizedUsers, flow( receiveUser ) ); |
There was a problem hiding this comment.
I think we can remove the flow() call because we're not composing functions anymore.
| ...omit( action, 'meta' ), | ||
| page: page + 1, | ||
| perPage, | ||
| } ); |
There was a problem hiding this comment.
Here, fetchUsers used to actually dispatch the action, but now it just returns the action object. We either need to dispatch it or return it from the handler to really execute it.
| dispatch | ||
| ) | ||
| ); | ||
| return map( normalizedUsers, flow( receiveUser ) ); |
There was a problem hiding this comment.
The returned array should optionally contain the fetchUsers action that is dispatched when there are more pages of results available -- we store the users we just got into Redux and at the same time issue a request for the next page.
An idea for a big improvement: add a new action, RECEIVE_USERS, that will store all users in an array as part of one state update.
Dispatching an array of N RECEIVE_USER actions will cause N state updates and N React rerenders of the app.
|
Thanks @jsnajdr for the explanation! I took another look at this but I can't figure out how to make it connect properly using the new dispatch method. I'm going to hand it back to you so I don't hold this process up, and keep an eye on the ticket to see how you do it. :) |
…uing request for next page Improve the `receiveSuccess` handler to first store the just-received page of user results using the `USERS_RECEIVE` action, and then optionally issue a request for the next page.
|
Thanks @sixhours for working on this issue! I added a few finishing touches and I believe it's ready to merge now. Details on what I did:
@flootr Can you have a quick look at the PR? I don't want to approve and merge my own commits 😉 |
jsnajdr
dismissed
their stale review
The issues got fixed by myself, someone else will need to review
Changes proposed in this Pull Request
See #25121
In this patch we're replacing the use of
dispatchRequest()in the data layer handler to use the newer API exposed asdispatchRequestEx()This should have no change in actual effect or interaction.(I still need to finish updating the Jest tests, this is a WIP.)
Testing instructions