-
Notifications
You must be signed in to change notification settings - Fork 639
Add route data to reducer action #205
Comments
Yes please! For us the param is actually also present somewhere else in the state tree so I can get it from there, but it'd seem more natural for the parsed route to exists there, if nothing else than convenience. Just realized this issue is only the action and not the state, but both apply, I think. |
This sounds sensible. I was just looking for how to access the url params but cant see where to get them from. When hitting my app I need to get a route param to work out what data to start fetching. (React Router exposes it to |
@rockingskier The param is passed to your navigation component as props, so you can access them there |
I have access to |
You should, |
facepalm, of course, thanks @SimenB |
Yes, you can use mergeProps to get all the data from the router and from redux into your component. However, as it stands (unless I am wrong?) you are pretty much left to fetch your async data in your component. I would rather my route change, trigger a redux reducer that can fetch the appropriate data based on the route using params, query, etc. and then pass the fetched data to the component. |
@jisaacks another option is to write a middleware that listens for the simple router action type and dispatches a subsequent action. |
Yes, |
Unfortunately, this isn't going to be possible within this project. We don't talk to You should be getting this directly from |
@timdorr so the recommended way to load data that is specific to a route is in the component (not a reducer)? That feels so anti react and anti redux with no single source of data and one way data flow. I think infinite loops and double renders would be due to a poor implementation. I don't think it would be impossible to achieve passing the router data to a reducer and let redux pass the data or processed data to the component. It may be very difficult based on how this project currently works without talking to react router. The package says that this is supposed to let you use the router as documented but I think it is more important, when accessing data to use redux as documented. Since the router's primary goal is responding to routes, and redux's goal is handling the data the app is concerned with. |
Loading data from components is super common and, taken to an extreme, is basically what Relay embraces. Ideally you'd even have a way to statically analyze what data you need before you render all your components, which is exactly the kind of stuff that react-router enables you to do. It's possible we may eventually have router params in the state, but we're still ironing out the current technique so we want to make this stable first. It's deceptively tricky to keep this stuff in sync, so if it introduces too many places to trip up, we aren't going to do it. There are many other ways to integrate data loading, and now that I think about it, you can't trigger data loading from a reducer anyway, so providing routing params doesn't help. |
Thanks @jlongster, for clearing that up. Isn't redux thunk and redux promise for triggering data loading in a reducer? Hmm, I'm not a redux expert so I could be wrong on that. Also as far as "it's possible we may eventually" how could I get involved in that process? |
Redux thunk and promise are for action creators, not reducers. |
SimenB is correct, those are for action creators, not reducers. You cannot do anything like that in reducers, no side effects at all, which is a critical part of the design.
There isn't really any work going into it right now, so there's not really much to do. You can present a real-life use case that cannot be solved by other data loading patterns, but as it seems you don't understand basic patterns of redux, I suggest you do some research about existing patterns first. You'll realize that you don't need this. |
@jisaacks It's not so much a poor implementation as it is a convoluted one. The infinite loop problem involves 4 separate libraries (redux, react-redux, history/react-router, and this one). When you connect a component to location state and then issue a route action within componentWillRecieveProps, you can trigger it. Who is ultimately responsible for fixing that? It can be any of them, which makes this so tough. The double render is more of an issue with history. When the history listener fires, react-router issues one setState to update props and if you're connected to location state, you'll get a second setState from react-redux. We can solve that by batching updates in history, but it's not aware of React itself and the batching is limited to react-dom, which would drive us further away from eventual proper react native support. It's tough stuff that may not even end up in this library. If we can make it work here without workarounds, then we should totally do that. However, doing that will require coordination, new APIs, and, most importantly, a lot of time. :) |
Ok took another look at things with fresh eyes, y'all are correct. It is the action creator where I really need the params and I can pass them to it in the component, makes perfect sense. In that case, yes it makes needing the data in a reducer much less necessary. Sorry y'all, thanks for bearing with me. |
My route that I am using looks like this:
In my
Message
component I have access tothis.props.params.id
.In my reducer my
action
looks like this:So a couple issues I see here.
First, using this relies on my reducer knowing first hand whether the router is using
hash
orpushState
. I shouldn't have to change my reducers if I change how my router is configured.Second, there is a params object given to my component via the router but not present in the action.
To solve these I think there needs to be an extra
route
object on the action so it looks like so:If I am just completely off base on this or using this library wrong then please point me in the right direction. Also I would be more than willing to help out with this, I have experience hacking on routers
The text was updated successfully, but these errors were encountered: