-
Notifications
You must be signed in to change notification settings - Fork 0
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
Idiomatic Redux: The History and Implementation of React-Redux #30
Comments
Original author: Ahmed Mahmoud Thanks for your precious time and effort! |
Original author: ddj0jyy @ddj0jyy hi i guess the third line of ‘subscribe’ function missed keyword ‘function’ function subscribe(return unsubscribe() { |
Original date: 2018-11-27T02:53:59Z Good catch. Fixed, thanks! |
Original author: Adrien LEMAIRE @adrien_lemaire Lovely blog post, thank you so much for all the time and efforts you're putting into react-redux <3 |
Original author: Alex Alex @AlexOshchepkov All this redux and actually react's non dom related (like createContext, hooks..) look like a bunch of hacks for the "state problem". Just use streams, cyclic way. |
Original author: Ryan O'Connor This was one hell of a post, thanks I enjoyed reading it! |
Original author: Dave @daveschinkel This is complete bullshit calling the ability to pass in store as legacy and forcing people down one path. I do not want to use mount or provider in my tests. Nor if I have to use provider for integration tests after I TDD do I want to be constrained to the context API and prevented from passing in a store. Not everyone designs "cargo cult" surprisingly because some of us understand design and TDD to see design pressure that others aren't getting who are not test driving. We don't all view context API as a good thing. Not being able to keep my tests simple by passing in the store is ridiculous which is a major reason I don't have provider in my tests to begin with. Simplicity! To be forced down this past is a very bad play on the React and Redux team. My tests should not be coupled to the implementation this much forcing one way to do it all. This one's pretty simple. If I have a connected list component, and it renders 10 connected list item children, that's 11 separate calls to store.subscribe(). That also means that if I have N connected components in the tree, then N subscriber callbacks are run for every dispatched action!. Well, that quote ultimately comes back to Simple Good Design Practices: Then decouple your design more! Break stuff down! Make groups of connected components grouped by Business/Domain Units more so that you're only effecting the area in your app you care about effecting on a specific page! Break your tree up into Groups of Connected components related to parts of your Business domain! That's called simple design and decoupling and driving your design to Domain / Business. So I have a 4 connected components somewhere in the tree, and they represent a business domain concept. I only worry about that when working in that area of code. Performance can be managed as it should be with any kind of code you work with (backend or front-end) However, the React team has continued to innovate
I use shallow when I TDD connected components. Sounds like we can still send in store as a prop from a shallow test to a connected component. If so good:. Please do not ever change this Redux. I do not want to rely on context for the design of my application nor tests, nor do I rely on mount or provider. If I do, this is becoming a very opinionated framework and a symptom of React and Redux becoming more of a mess and way too opinionated. In fact I see React as a framework with all the clutter that's being added to it as of late. I can't call it a lib. Keep the simplicity. Stop trying to add more and more magic. |
Original author: Dave @daveschinkel You know what has "rippling effects"? Our inability now to send in store as a prop in shallow tests. That's a breaking change and also prevents us who like to shallow with React no longer able to do that at all through Redux now. |
Original author: selonmoi @selonmoi Thanks for this post Mark! Some Googling eventually led me to it after my update to v6 broke a bunch of tests. I was using Enzyme shallow rendering + store as a prop to unit test connected components. It was really helpful to be able to read about and understand the change, and also very interesting to learn so much history of this library I've been relying upon. Any thoughts on good alternatives to my testing approach? For example, I had tests like this to test mapStateToProps:
And like this to test mapDispatchToProps:
For now, I've switched from shallow() to mount() and wrapped my connected component in a Provider. I'm thinking shallow rendering is out of the question. It's unfortunate, though, since now I have to worry about providing the necessary state to allow the whole descendent tree to successfully render. Is that my best option, or is there some other way to establish the needed context and still do a shallow render? Or is there some altogether different approach you'd suggest? Or do you think this type of testing is just more trouble than it's worth? |
Original date: 2019-01-11T22:29:13Z Hmm. I've seen a few people make similar comments about shallow tests not working, but no one has yet specifically filed an issue to complain :) Tell you what. File an issue on the React-Redux repo and let's discuss it there. |
Original date: 2019-01-11T22:42:50Z I just went ahead and opened up https://github.com/reduxjs/... for discussion. Please comment there. |
Thanks for the awesome content! I've been working on revitalizing fluxible react implementation and you have no idea how much your work has been helpful to understand the optimizations we are missing and the pitfalls we may encounter on the way. Again, awesome job! |
@pablopalacios : nice, glad to hear it! uh... in all seriousness, is anyone still using Fluxible at this point? :) |
@markerikson oh, believe me or not, my company has a huge application built on top of fluxible. When I joined the company I tried to migrate it to something else but I realized that it's easier to improve fluxible itself. It's that kind of legacy where the people that built it is already gone and it has more than 5 years of business logic built into it and so on and on... So I would say that besides my company and perhaps some legacy yahoo projects, I have no idea who would still be using it :D But in all seriousness as well, it might be that a multiple store architecture might work better with the new React server components. But it's more likely that I will wait an update on your article to see how you did it and, in 5 years, I might give a try to do the same with fluxible :p. |
GREAT BLOG! NOW I SEE REACT-REDUX IS MAJIC! |
Finished reading the whole thing in a row. Thank you very much for the effort, especially in understanding the path how we comes this far. |
No description provided.
The text was updated successfully, but these errors were encountered: