-
Notifications
You must be signed in to change notification settings - Fork 24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feature/onParamsActionsCompleted #33
base: master
Are you sure you want to change the base?
feature/onParamsActionsCompleted #33
Conversation
…er all params action creators are dispatched.
Thanks for the suggestion. The use case seems completely valid, you may want to know when all URL parameter changes have been handled. Our current approach is lacking, as it just dispatches the actions one by one, without e.g. indicating their relatedness nor telling how many there will be in total. However, I wonder whether your solution, i.e. dispatching an additional action to signal that the URL parameter changes hawe finished, is the best solution to this. On the upside, it is intuitive: a signal that the URL parameters have changed. Simple. On the downside, it feels like it encourages bad code structure (though I may be too opinionated): in your example If I understand you correctly, the problem currently is that if you would react both to a change in
Would such an approach seem equally helpful to you, or inconvenient? A remaining issue, raised in the quote above: how to make a nice API for this. I imagine an option (e.g. One question that this idea also raised: Can one specify both a bulk handler, as well as individual action creators per parameter? Would those actions then be executed before the bulk handler, and the bulk handler only gets a list of remaining changes? Perhaps it is easiest to say instead that if the bulk handler is defined the individual parameter's actions are not dispatched. (for advanced usage we could pass those individual actions to the bulk handler as a second argument, which could then return a thunk that dispatches each of them; assuming redux-thunk is used). |
Thanks for the reply!
You're right that in my case my main goal is avoid re-fetching data more
than once. My refetch operation is expensive, potentially a 3-5 second call
sometimes. I could throttle it, but I'm trying to speed up this whole
process, so ideally I'd like to fire off the refetch as soon as possible
without the possibility (though slim) of having it called twice via a
throttle or have to wait longer than I need to via a debounce.
The solution of having an additional single action which handles all of the
changes I could see being valuable for a lot of use cases. In my case
having a separate action for each change actually works out great. I have
reusable react higher order components which each update different parts of
the redux store (one for paging, one for sorting, that sort of thing). So
having separate actions works well in having each component and its
associated actions and reducers be responsible for their own part of the
redux store. Then when all those are completed, having some sort of final
action ends up being the simplest route. You did bring up some fair
drawbacks with that approach though.
So the approach you mentioned where one bulk handler receives all the URL
parameter changes instead of the individual actions is a bit inconvenient
for my scenario - but only if I'm just using straight actions. That could
work really well when using a thunk! In that case in my bulk handler I
could theoretically receive all the changes and their associated actions
and dispatch them myself and then at the end of that fire off my action
that does a refetch. A potential API then could have a payload which is an
array where each item contains the parameter, its old and new value, and
the action its mapped to. That would make it straightforward to dispatch
them as needed.
You bring up a good question about whether or not individual actions should
be invoked if a bulk handler is specified. If I can throw my opinion into
the hat, I would vote for not invoking the individual actions if a bulk
handler is specified. Otherwise the interaction starts getting pretty
complex as highlighted by the different scenarios you mentioned.
.
…On Thu, Apr 11, 2019 at 4:02 AM Gerben ***@***.***> wrote:
Thanks for the suggestion. The use case seems completely valid, you may
want to know when all URL parameter changes have been handled. Our current
approach is lacking, as it just dispatches the actions one by one, without
e.g. indicating their relatedness nor telling how many there will be in
total. However, I wonder whether your solution, i.e. dispatching an
additional action to signal that the URL parameter changes hawe finished,
is the best solution to this.
On the upside, it is intuitive: a signal that the URL parameters have
changed. Simple.
On the downside, it feels like it encourages bad code structure (though I
may be too opinionated): in your example refetchData should ideally be
invoked *because* the state's values for pageNumber and/or sortBy have
changed. You might end up adding another input that can change the page
number, and then you would have to ensure you don't forget to call
refetchData again; not great.
If I understand you correctly, the problem currently is that if you would
react both to a change in pageNumber or sortBy, you would trigger
refetchData twice, which would be wasteful (unless it is throttle
<https://lodash.com/docs/#throttle>d). I have a hunch this problem is
better approached differently: conceptually, you want the ability to regard
the change of URL parameters as a single action, not a series of actions.
This action will cause several state variables to be updated, at once.
This idea came up in #18
<#18>:
One other solution that comes to mind is to rather let the user specify a
single action, which will get an object with all changed(?) params. You
would specify the action in the top-level config rather than in each of the
params configs. Not sure if this is a nice API, or what would be the
desired semantics if actions are specified both at the top-level and for
some individual parameters. But it seems a simple solution to give full
freedom in how to handle location updates.
Would such an approach seem equally helpful to you, or inconvenient?
A remaining issue, raised in the quote above: how to make a nice API for
this. I imagine an option (e.g. bulkHandlerAction, better name needed),
which upon location change is invoked once with a list of changes, e.g. {
pageNumber: { old: 2, new: 3 }, sortBy: { old: 'name', new: 'date' } }.
It then returns a single action that is to be dispatched, which could be
used to update the whole state at once.
One question that this idea also raised: Can one specify both a bulk
handler, as well as individual action creators per parameter? Would those
actions then be executed before the bulk handler, and the bulk handler only
gets a list of remaining changes? Perhaps it is easiest to say instead that
if the bulk handler is defined the individual parameter's actions are
*not* dispatched. (for advanced usage we could pass those individual
actions to the bulk handler as a second argument, which could then return a
thunk that dispatches each of them; assuming redux-thunk is used).
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#33 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AA3WcXsR4NNMXeFrdlVClb6FRebEokK4ks5vfwhMgaJpZM4cn2d5>
.
|
So.. sounds like either solution would work for you? I am still poised towards the bulk hander approach, especially as it may also enable many other types of customisation. I like your suggestion to pass, for each (changed) parameter, the handler the changes and actions together in one object. Though I'd probably concatenate these objects in an object, not in an array:
In case you would like to work this out into a PR, you are most welcome; but of course no worries if not. I likely will not get around to it soon (partly since this module lacks tests, so it's a hassle to convince oneself things work as intended). |
Hi @Treora, are there any new on this? |
No news; I have not touched this module in ages as I have not worked on any redux-based projects, and have too many other things to do. If it would be of help to people I could merge a fix or even add a feature, if it does not require much involvement from my side (ideally other users would also help with reviewing, though I am not sure if/how many people might be interested). |
Adds an onParamsActionsCompleted action creator which is dispatched after all params action creators are dispatched.
This is useful for situations where different you need to issue one final action after all param actions have run and the redux store is synced with the query string. For example, say pageNumber and sortBy both have their associated actions which update the redux store. However after those run you need one final action - say refetchData - to run after both of those items have been updated.