-
Notifications
You must be signed in to change notification settings - Fork 642
Clarifying the goals of this project #257
Comments
For a number of reasons (bundle size, code complexity), I've always thought it would be interesting to isolate the Dev Tools integration into its own thing somehow. Not having to worry about it really simplifies the functions used for production. Unfortunately, it's necessary for the current setup to couple the two things, but a split would probably simplify a lot of the confusion. |
What I’m trying to understand is what exactly is the benefit of the rest of the project other than patching usability holes with making |
We (@ory-am) are basically only using the action creators and the DevTools. Most of the time, we use
In fact, we do exactly that. We handle side effects with |
Dispatching an action instead of using directly |
At least in my use case it was simply a matter of "knowing where to put things". The second point |
Doesn’t this achieve the same goal? history.listen(location => store.dispatch({ type: 'NAVIGATE', location })) |
Let's denote those points as (1), (2), (3). (2) is actually just its own little thing – I'm quite happy with what we have via #141 from my perspective, though you're correct in that this is not strictly tied to the rest of the package. (1) and (3) are where it gets interesting and messy. I think something like my proposal in #248 (comment) would get us to (3), but it's still a bit of a hack, and it's not perfect. I think the first-best solution to (3) is actually to solve (1), by having Redux drive the router, in which case (3) naturally falls out just by virtue of Redux being Redux. While the router state itself is not serializable, we can digest it into just the location, and use This is much more like redux-router, though, but I think a "simple" version of redux-router might be a viable approach. It would clean up (3) a lot. |
What have we learned from Redux Router? What parts of it were complicated, why, and has this changed? |
cc @mjrussell, @acdlite on the Redux Router question I don't want to sidetrack this too much, but I think the way this API wants to look is that I think we can solve the serializability-for-persistence issue with a custom |
Got it. Thanks for the information. I’d say let’s not get too far in this thread with implementation thoughts, and rather figure out the focus better. What do we want to solve with this library and why. |
I would say simplicity is a feature, not a requirement of the project, so if we have to get more complex, that is fine. Maybe not redux-router levels of complex, but not redux levels of simple 😉 |
The problem is that without a clearly defined pain we’re solving, it’s hard to focus.
This is when I realized many people might be installing this project and setting up all this stuff without actually needing it. For example, from the same exchange:
But this is incorrect because you can do import { browserHistory } from 'react-router' and call Similarly, from the exchange in this thread:
But again, it is a matter of import { browserHistory } from 'react-router'
browserHistory.subscribe(location => store.dispatch({ type: 'NAVIGATE', location })) when creating a store if you want it. So this is why I want to get a better understanding of what the goals of this project are. Because if it’s persistence and DevTools-friendliness, perhaps it’s easier to achieve in other ways, without providing |
I think that's a point that's worth making either way – in general you're not going to need anything to use React Router with Redux, at least when starting out. Certainly I've never felt the need to put router stuff in any of my Flux stores or actions. |
Actually, up until 2.0.0-rc1 (a month ago), you could not. And you still cannot without running non-final packages. You can certainly create your own singleton, but I don't think most people know to do that. Hence, the popularity of this library lies in the limitations of React Router 1.0. |
That's not that much of a limitation, no? // history.js
export default createBrowserHistory(); If you want to use browser history, you're doing this anyway – just need to expose it, is all. |
I think what I'm saying is the goals have shifted. What was once a major requirement is now more of a convenience. I noticed a lot of our discussion on Reactiflux was centered around Router 2.0 concepts and internals. I think we should be aware that most people aren't using that version yet, so the necessity of this library is going to diminish. |
@taion Yes, it's easy to do so. But I don't think most people know they can do that. We weren't often giving that kind of guidance to users. Most people treat module imports like functions or classes, not singleton instances, so I don't think that behavior is commonly considered. Anyways, now that router is advocating their use, one of this library's big advantages will be going away. |
The thing is, once 2.0 is out, we can put anything we think we need to into “Usage with Router” official doc. We can teach people to use singletons (whether exported by RR or manual) there just fine. People treat Redux docs with respect :-). So let’s take this into account. If it’s an education problem, we can fix it. Is it an education problem, or are we missing essential use cases (beyond DevTools and persistence)? |
Do we advocate its use? We merged remix-run/react-router#2992. |
It's sounding more and more like the version of this lib that best matches with router 2.0 is one that only deals with devtools stuff. |
I don't understand, though. We merged a PR with what seemed like the rationale that using singleton histories was fragile and undesirable. It can't be the case that it's not okay to use singleton histories in that context, but is okay in this context. |
@taion remix-run/react-router#2991 has some more on the subject. I still think it's weird for presentational components (especially functional stateless components) to reach into the outside world. That LinkButton case should have gotten at it via |
It may be that built-in action creators are beneficial, but only as a way to dependency-inject the history. This is part of what I’m trying to understand about this project. If this is the case, it’s fine, but we need to understand whether or not this is one of the important conveniences it brings. |
@gaearon Time travel is useful for way more than just developer experience. It is a core feature for production use as well. Without this library these types of use cases would be missing a significant portion of the flow. |
I think the action creators are sort of a "throw them in" thing. I don't think users want to have to use all of |
@danmartinez101 Thanks for your input! I definitely understand this use case. I probably shouldn’t have said “dev” and rather just called it “time travel support”. What I mean is that it might be easier to do in a separate project and in a different way, and this got me wondering what exactly is the scope of this project if we remove that. |
Definitely! This is why I’m trying to figure out the scope that we really want to support :-) I don’t want to split projects for modularity’s sake—rather, I want to understand which of these features are essential so that we can approach them more holistically and ideally without big hacks or glitches. For example we may decide “ |
About |
I understand more clearly now. /me fades into the background |
Just a general user here. I installed RR in 1.0 days and using redux router helped fix some API issues, keep up with the redux way and use the dev tools. After the jump to 2.0 I didn't spend much time re-learning, so just upgraded this as well. Like many redux libraries I have discovered them in These are some things I learned in this discussion alone, hopefully they make it to the docs somewhere.
I haven't looked at the entire API surface of RR, Redux, Redux Router and it would be beneficial to know that this could be used a replacement for some functionality of this library. Some libraries have the "do you really need this?" on the README.
The discussions in remix-run/react-router#2992 and remix-run/react-router#2991 led me to realize you can access context in functional components. I also realized why the history singleton is bad and how to work around it. I typically only read about the library / API when implementing it and possibly again on a major version update. All of these discussions and document improvement merges I will likely miss until I catch @gaearon next tweets (thank you for your effort, I have learned so much by following you). |
@patrickheeney History singletons aren't bad. They simply don't work in the server context, so you have to be using them thoughtfully and not for every single case where you're trying to navigate. |
@timdorr IMO, that is what makes them bad. I will be converting my app to universal at some point and that would have ended up an unpleasant surprise. Following what I learned above I would rather use context in functional components or pass the router change function down as a props. It has the benefit of using less of the API so I won't break if that singleton changes and works on the server. Based on the discussion above and from a user point of view, I'd like the see the repo evolve into replay functionality and not provide any convenience methods or other API surface area if I can get similar functionality from the RR API directly. |
@ryanflorence has made some great, user-friendly API changes for 2.0, so a lot of pain points this library used to resolve are now obsolete. It sounds like we're evolving the focus to more specific needs, like time travel, as router is now significantly better to work with. |
Please see reduxjs/redux#1362 for something I hacked together based on this discussion. Big thanks to everyone. |
From a maintenance/contribution point of view: It´s great to reduce the complexity of a project, though I don´t think this project is in the need of that. If I would come in with the intention to contribute to this project, I can get an overview of the inner workings pretty quickly. What we may have learned from From a (unexperienced) users point of view: The JS ecosystem right now is pretty complex, so I will appreciate if I don´t have to get three libs to get the full API and functionality of this project, staying up-to-date on all three of them etc. It is okay for me to use maybe only 60% of the API a library offers in production, and I can see the rest as a nice-to-have, if a may run into problems during development. Also, if the project fits well into how I do things with other libs (e.g. using redux actions for every state change including routing), I can easily wrap my head around that. In general I think extracting the Just my 2 cents. |
@gaearon There is already so much discussion here T_T I think there are 2 use cases this solves for me:
For everything else I think the raw react-router API should be used. I don't really know if you area solving #2 with your solution, Dan, but I hope we can collaborate on this within this project instead of diverging. EDIT: The above features are naturally express as the URL in the state, because the user can debug by looking at it and use actions to feel like they are "changing" that URL state even though they are changing history. It's entirely possible that #2 could be solved by passing |
To answer the original comment clearly: 1 was not a goal of this project (for me), but just came out of it, 2 yes, in certain cases providing actions was easier, and was mostly inspired by redux-router. But I don't care much about this, I'll pass history around. Also, react-router singletons make this pain go away completely. 3. Yes this was the So your approach is fine to me. |
@jlongster With the v2 API, you generally won't need to pass around |
@taion Yeah that's what I meant by "react-router singletons make this pain go away completely". I realized that while typing it out. EDIT: (and that feature wasn't available when this project started, hence why it works the way it is now. But Dan's new direction is a welcome change in response to RR 2.0) |
Yeah, I think 95% of users would be ill-served to use action creators v the singletons. |
I think I have a good enough understanding of the situation now. I think having two exports in the future and downplaying the role of the The focus of this project would then be the history enhancer that routes updates through Redux store, as prototyped in reduxjs/redux#1362. I don’t have time to contribute right now but I’ll try to allocate some time to it in the coming weeks if nobody beats me to it. (Please do beat me to it! 😉 ) |
I don't fully understand the dislike directed towards action creators. I get that the singletons are easy to use. But here's a perfect example of where I'd prefer action creators over singletons (even if all it does is called a history singleton to perform the transition). Consider you are writing a saga in which you want to listen for a specific action say I get its a bit contrived and there are many other ways to handle that scenario, but my point is that so many cool things in redux are built around action creators and there will always be use cases for them. If its simple to add, throw them in and just document that they are not necessary in most cases |
Otherwise I really like the direction of all of this. I think it provides a great solution while still being true to the goals of this project and not subjecting users to potential pitfalls |
You can certainly come up with use cases for them, but the vast majority of users can avoid them and use just the history singletons. To be clear, action creators will remain in the next version of react-router-redux. They will simply be an optional thing to use and we will deemphasize their use. |
This is exactly what we plan to do. What I’m proposing is decoupling two unrelated parts (“actions as DSL for Don’t forget that many people don’t use sagas, don’t yet write tests for action creators, are just starting, and are completely overwhelmed by how many libraries they need to get something running. So we want to emphasize why you’d want this lib, and what you can do instead if you’d rather not bring it in. |
@gaearon yeah I just saw that proposal in your PR, I think thats a great approach. Just saw the trending discussion towards being very anti-action creator and wanted to provide a perspective of a user who is not just using them because its an easier mental model, but actually because they provide a concrete benefit. |
👍 thanks for chiming in! This discussion informed my perspective both in how action creators are not essential and how they are still quite damn useful to some people in some situations. |
I'm new to redux and react-router-redux and I'm concerned about the state.routing concern (1) that gaearon raised. Should I avoid reading the location from state? Can someone please expand on when using this would be a bad idea? |
@jackgeek Nothing inherently wrong with it. The issue comes when you use If you're accessing state elsewhere, it's not a bad thing. |
I think I’m getting a little confused by the goals and the scope of this project. I’d appreciate @jlongster weighing in on this.
I see a few things it does right now:
state.routing
for other middleware, components, and action creators to read. I believe this is problematic: it doesn’t match the rendered route because of the potential for async transitions in between. This is the reason we warn people not to usestate.routing
inmapStateToProps()
which I believe is confusing, and even if they don’t use it in components, they still might use it in action creators and expect it to correspond to the router’s current state. So I’m not sure whether this is a good feature to keep.history
. While this may be convenient, I think the reason people want this is not because actions are the best interface for causing routing changes, but rather because people don’t want to deal withcontext
or passhistory
around. Many people are also feeling pressured to express everything with Redux primitives (actions) even if the problem at hand is not expressed with them well. Consider thathistory.pushState()
is not much different fromfetch()
(it’s a side effect) but nobody expects us to provide “fetch
action creators”. The real problem here seems to be the perceived difficulty of passinghistory
around or importing it, rather than the need for history updates to be actions.As we evolve (or not evolve) this library, I want to better understand which of these points are actually important to the author, and which are just a byproduct of current user habits and their desire to express everything “the Redux way”.
Essentially, I want to understand what value
react-router-redux
brings overreact-router
+history
in production mode other than the familiar mental model (“just dispatch actions”), the ability to reach out intostate.routing
(which can be dangerous and may not reflect the UI but can be convenient for apps without async transitions), and not having to think about how to accesshistory
without passing it around.In no way am I trying to diminish the work being done here. I just want to figure out for myself whether presistence/time-travely part of this library could be a separate dev-only project, and whether what would be left is not essentially a feature request for better documentation and less ceremony when accessing
history
from action creators and other middleware.Thank you!
The text was updated successfully, but these errors were encountered: