Skip to content
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

Open
markerikson opened this issue Oct 5, 2020 · 16 comments
Open

Idiomatic Redux: The History and Implementation of React-Redux #30

markerikson opened this issue Oct 5, 2020 · 16 comments

Comments

@markerikson
Copy link
Owner

No description provided.

@markerikson
Copy link
Owner Author

Original author: Ahmed Mahmoud
Original date: 2018-11-26T09:58:15Z

Thanks for your precious time and effort!

@markerikson
Copy link
Owner Author

Original author: ddj0jyy @ddj0jyy
Original date: 2018-11-26T21:49:10Z

hi i guess the third line of ‘subscribe’ function missed keyword ‘function’

function subscribe(return unsubscribe() {
listeners.push(listener)
return function unsubscribe() {
var index = listeners.indexOf(listener)
listeners.splice(index, 1)
}
}

@markerikson
Copy link
Owner Author

Original date: 2018-11-27T02:53:59Z

Good catch. Fixed, thanks!

@markerikson
Copy link
Owner Author

Original author: Adrien LEMAIRE @adrien_lemaire
Original date: 2018-12-06T01:25:06Z

Lovely blog post, thank you so much for all the time and efforts you're putting into react-redux <3

@markerikson
Copy link
Owner Author

Original author: Alex Alex @AlexOshchepkov
Original date: 2018-12-07T16:18:12Z

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.

@markerikson
Copy link
Owner Author

Original author: Ryan O'Connor
Original date: 2018-12-15T23:17:02Z

This was one hell of a post, thanks I enjoyed reading it!

@markerikson
Copy link
Owner Author

Original author: Dave @daveschinkel
Original date: 2018-12-20T21:42:04Z

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


They're not innovating with testing or simple design in mind IMO. Because they only promote integration tests which do not put design pressure on your components. So we end up with stuff like "use context API", "Use mount and write integration tests", and so you end up with a majority of devs who have never written isolated tests, they don't know how, and so they shun stuff like shallow or don't see the need to send in state as a prop anymore. This isn't a good thing. The innovation only drives creating more magic and one way to do things instead of flexibility, lightweight, and letting people design and test drive or write tests in different ways.

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.

https://www.infoq.com/prese...

@markerikson
Copy link
Owner Author

Original author: Dave @daveschinkel
Original date: 2018-12-21T04:43:42Z

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.

@markerikson
Copy link
Owner Author

Original author: selonmoi @selonmoi
Original date: 2019-01-11T21:18:17Z

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:

test("maps count from state to prop", () => {
  const store = mockStore({ counter: { count: 42 } });
  const wrapper = shallow(<connectedcountdisplay store="{store}"/>);
  expect(wrapper.find(CountDisplay)).toHaveProp("count", 42);
});

And like this to test mapDispatchToProps:

test("maps onIncrementClick prop to INCREMENT_COUNT dispatch", () => {
  const store = mockStore({});
  const wrapper = shallow(<connectedcounter store="{store}"/>);
  wrapper.find(Counter).props().onIncrementClick();
  expect(store.getActions()).toEqual([counter.incrementCount()]);
});

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?

@markerikson
Copy link
Owner Author

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.

@markerikson
Copy link
Owner Author

Original date: 2019-01-11T22:42:50Z

I just went ahead and opened up https://github.com/reduxjs/... for discussion. Please comment there.

Copy link

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!

Copy link
Owner Author

@pablopalacios : nice, glad to hear it!

uh... in all seriousness, is anyone still using Fluxible at this point? :)

@pablopalacios
Copy link

@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.

Copy link

GREAT BLOG! NOW I SEE REACT-REDUX IS MAJIC!

Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants