-
-
Notifications
You must be signed in to change notification settings - Fork 15.3k
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
More on reusability for custom components. #1850
Comments
A couple quick thoughts:
|
Well I think if we implement it in combineReducers we give the developers the ability to write reusable redux components (not necessarily react components) that can have multiple instances. Currently redux architecture has no solution (not a clean one that doesn't add lots of overhead to developer) for instantiating classes using its store as storage multiple times. The class could be anything. It's such a common use case to instantiate a class multiple times. Just a simple example:
But it's not limited to above use case. "Every" class that's not to be used as a singleton will benefit from it. And it's such a huge use case. Besides I think it can be done with less than 20 lines of code in redux core and then we have the ability to instantiate multiple times from a class using redux store as its storage "easily" with no overhead or dirty hacks. |
@sassanh How do you propose the id be generated? Is the component creating a unique id at mount time or something? Are you recommending redux handle things only on the reducer side, but the rest of the reusuable redux component (action creators, component, etc.) has to be handled by the component author? |
@naw I think it's risky to let the component author generate the id himself. It may not be unique. Besides other than being risky the id generator is another repeated code that can be integrated. So I think it's better that we generate at in I think this is the "least must have" for redux to support multiple instantiations of classes using it as their storage, but it could be improved if you agree with me that multiple instantiations can be a huge use case of developers using redux in future. Currently redux is usually used for singletons but if we support multiple instantiations better and better it can be used as backend storage for applications with full oop architectures (not just those using singletons.) So based on how much we think it's important we can add another level of support for it. This is the 2 levels of full oop support I can think of. I'd be glad if others can complete/improve it:
That's all I did manually in my current project and it was enough for me to have P.S. Of course |
I believe the redux ecosystem has room for innovation in the "reusuable redux component" problem space, although I don't necessarily see such innovation as belonging in redux core, especially if it's just a replacement for With that said, I do want to understand your proposal better, but I'm afraid I don't get it. In order for any given component to be connected to redux, it has to know where in state to look. If this location depends on an ID, then the component (or some singleton wrapped instance of its action creators) must have knowledge of which ID to use. Supposing It would be helpful to see an example of what your arguments to a modified Also, a good reusuable redux component pattern should be able to handle dynamically added components, which you don't necessarily know at the time that you call I can write more on this topic, but first I'd like to better understand what you're actually proposing. Not trying to throw water on your idea --- I just don't understand it. |
I'm not really clear on what's being proposed here. If you're really suggesting that On top of that, this really sounds like a very specialized use case, and not at all the sort of thing that There's already numerous examples of utilities that can generate more specialized reducer functions, and addons that implement some approach to reusable component state. The Reducers and Component State pages in my Redux addons catalog list most of the options that are currently out there. |
As a point of clarification, there is a difference between I agree with @markerikson this is unlikely to be a good candidate for redux core, but I really do want to understand and dialog about the idea. I suppose we could move the discussion elsewhere if that would be better. |
@markerikson about purity of @naw The ids should be generated in In order for any given component to be connected to redux, it has to know where in state to look. Currently
the whole state is not passed to This is how it goes (just a sample implementation):
now Look at one of my reducer functions in my current project, this is the reducer of my
I have that
and my initial state would be something like this:
I hope I could make it clear. Now I'm all ears to hear your ideas. |
There's no reason to put something like that in function createFilteredReducer(originalReducer, predicate) {
return (state, action) => {
if(predicate(action) || !state) {
return originalReducer(state, action);
}
return state;
}
}
function reusableReducer(state, action) {.....}
const item1Reducer = createFilteredReducer(
reusableReducer,
action => action.someField === "id1"
);
const item2Reducer = createFilteredReducer(
reusableReducer,
action => action.someField === "id2"
); I understand what you're saying about "targeting" actions to a specific "copy" of some reducer logic, but |
@sassanh Thanks for your detailed example. Yes, your desire to reuse reducer logic for multiple instances of the same component is definitely commendable. I believe a higher-order reducer (or a reducer factory, as some people might call it) as @markerikson wrote is a good pattern for solving this problem. A more specific case of that would look like this: filterById(originalReducer, elementId) {
return (state, action) => {
if (elementId === action.id) {
return originalReducer(state, action);
}
return state;
}
} Then, you setup your reducer like this const topBarReducer = combineReducers({
leftMenu: filterById(menuReducer, 'LEFT_MENU'),
rightMenu: filterById(menuReducer, 'RIGHT_MENU')
})
const finalReducer = combineReducers({
loginDialog: filterById(dialogReducer, 'LOGIN_DIALOG'),
registrationDialog: filterById(dialogReducer, 'REGISTRATION_DIALOG'),
// ... rest skipped for brevity
}) Now, if you want to go one step further and have I'm definitely in agreement that you should reuse your reducer logic, and I'm definitely in agreement that passing an identifier in the action is a good way to do that. I'm with @markerikson that a factory is a good pattern for this. Ultimately, As a final point: Your menu and dialog components need to know which id they correspond to so that they can put the correct id into the actions that they dispatch. So in your example, your components must have the id, perhaps passed into them as a prop like |
@markerikson @naw Thanks for your detailed comments and opening the issue in details. Personally I think the current Other than that I totally understand that redux wants to be minimal and I can see this minimalism in other parts more or less. It's unfortunate for me that frontned community doesn't like oop as a first class architecture (or at least optional first class architecture) coming from a 15 years C++ and Python programming background I never felt comfortable with javascript and always found it loose till I read about redux and es6 classes 3 months ago. The discipline redux enforces to webapp development is really what it needed in my opinion but I wish it enforced more discipline with forcing use of immutable and I wish it supported oop better by applying above suggestion for example. For someone coming from C++ and python this use case is the main use case for a program with more than 100 lines of code as writing the whole program with singletons is not a good pattern there, ~%99 of the time you implement a class not to instantiate it only once, you implement it instantiate it multiple times. ~%99.99 of usages of storage backends in those languages (such as Sql, Nosql etc or Redis for an in memory example as it's the case with redux) use an id field for storing each instance. Currently redux is like a storage engine that supports multiple classes/tables internally but if you want to have more than one object/row in your table you need to plug something else to its core if you wanna avoid rewriting same code again and again. I try to understand that you put supporting classes/tables in core but you think supporting multiple instances/rows shouldn't be first class option here. I guess you don't see it as classes/tables and instances/rows. I'll be glad to know the philosophy behind having this I hope this thread helps someone trying to achieve this functionality in his app. I really appreciate your thoughts and ideas. @naw about providing id to elements, I should've include this in my above example:
So elements indeed have access to their id to use it in their action dispatches. I provide their relevant part of state as their props. Again I'm thankful for sharing your ideas on this with me. |
@sompylasar thanks for the link but I think it should be fixed in redux (either in its core or as an external package providing this feature for redux) and not in react-redux as it's a problem with redux handling multiple instances of same class/component. I guess we have enough of packages with names like "x-y-z-w-q". "react-redux-reusable-components" would be my nightmare, lets keep it "redux-reusable-components" if not "redux". It's not only react that needs to reuse its components. |
@sassanh, it was @gaearon who told me to start an issue in @timdorr has just published this article which looks related: https://github.com/reactjs/redux/blob/master/docs/recipes/IsolatingSubapps.md |
@sompylasar different stores for different instances seems to be a good idea, I thought I read somewhere it's an anti-pattern to have multiple stores in an app and googling "multiple stores redux" I found this http://stackoverflow.com/a/33633850/1349278. I don't know maybe someone with enough knowledge about the store implementation and its overheads can tell us if it's still anti-pattern or not. I guess we lose time travel anyway. About not discussing this issue in react-redux, I don't know the context that made you move the discussion there, but I prefer to not discuss this issue in I should add to this that among all this mess react, redux and packages related are really a great improvement. If these packages didn't exist I'm sure I wouldn't do anything related to frontend. I'm really thankful to community, specially community that's formed around Facebook packages for developing these good quality packages and architectures. |
@sassanh Yes, I definitely have some sympathies with the idea that Personally I think of There is always some tension between flexibility and conventions, and good software finds a way to balance these nicely ---- often in the form of low-level API's with higher-level API's layered on top as good defaults. Layering abstractions on top of one another is good, and that's exactly what redux does in this case. I wish other packages did the same. In other words, the beauty here is that you can opt-out of Can you imagine how awful redux would be if it forced you to use One problem I've seen with having it in this repo is that programmers have trouble thinking outside the I have several abstractions in my own redux app, but none that I feel are elegant enough and general purpose enough to be useful to the wider community yet, especially not in this repository. I would give myself the same advice I'm giving you --- as long as it's a change that plugs into the underlying redux core API ( If you get 10 programmers in a room, they'll come up with 10 different abstractions. There are quite a few suggestions for modifying the As for your complaint about front-end community not liking OOP architecture, I think that's probably a bit of a straw man, and perhaps even some misunderstanding in how a redux app would idiomatically handle multiple instances. No one here is suggesting everything be a singleton. Quite to the contrary, we regularly have many instances of the same component, each with individual state stored in redux. A more typical way to handle this in redux is for a given slice of state to represent a "table", and for that slice to be either an array or a hash that holds multiple individual instances of the same kind of component state. This is just like rows/records in a table. To use your menus and dialogs as an example, that would be like this: initialState = {
menus: {
LEFT_SIDE_MENU: { // state goes here },
RIGHT_SIDE_MENU: { // state goes here }
},
dialogs: {
confirmationDialog: { },
registrationDialog: { }
}
} Then you would have a So On the contrary, mounting a reducer for each particular instance (as shown in your original example) would actually be more analogous to having both a I can definitely acknowledge this is a matter of perspective (i.e. where/how you conceptualize "instances" living), but at the very least, it is inaccurate to say that the redux community only embraces singletons and resists OOP. |
To elaborate on what I mean by "perspective" Do you see the state tree as a mixed bag of instances of disparate kinds of things (i.e. a Menu and a Dialog both can live together as siblings), or do you see the state tree as a set of tables, each of which holds the instances for a given class (i.e. Menus live in a menus key, and Dialogs live in a dialogs key). An idiomatic redux app generally has two kinds of keys:
So if you have a bunch of instances, you'd store them nested inside a "table" key. If you have singletons, they'd be their own key. The state tree you showed in your example, is actually a bit of a hybrid, where you have instances sprinkled around the tree as if they were singletons. If they're truly singletons (which happen to have a bunch of common logic), then use the factory pattern. However, if what you're really after is instances of the exact same class, it would be more idiomatic to put them all together in the state tree, and have one reducer mounted to handle all instances, not a separate reducer mounted for each instance. Of course, the single reducer can delegate to a helper function (which might itself look like a reducer) which only knows how to handle a single instance, rather than the collection. Of course, idiomatic just means common patterns the community has unofficially "adopted" as best practices. We can deviate from these patterns whenever it makes sense, and fortunately, |
@naw I really enjoy and benefit from reading your ideas.
I strongly believe that the use cases with a fully oop compatible
I'm not trying to catch wildfire, I'm not highly active open source guy, for me knowing my code is written in a way that matches my intuition is more than enough, I have no passion for writing a code that everyone uses. I don't want to force community to use my convention too, I just gave a suggestion on how oop can be supported, I still think that if
Well you're just a part of front-end community, many applications in the community don't use redux nor react. Many still use |
Some reducers are designed to handle singletons. Other reducers are designed to handle several instances of a class. It is a lower-level tool, and in that sense, completely accommodates OOP by virtue of being agnostic about it. Saying that So if the question is "should combineReducers be higher-level than it already is?", I think the clear answer is "No, because it is better to plug something on top of it". Some people want to use OOP, some people don't -- A better question might be "should redux provide higher-level tooling for OOP". Perhaps, but lamenting that redux doesn't have higher-level support for OOP is like lamenting that postgres or MySQL don't have support for OOP. Of course they don't, because they're lower-level tools. They could, and maybe someday they will, but in the mean time, external libraries (like ORM's) serve that purpose handsomely as external packages layering a higher-level abstraction on top of the storage. Do you think MySQL and postgres are inherently anti-OOP? |
It's clean. The only problem with it is that your application needs to know about menus used in child components. Consider my example, there were a
I'm not mounting a different reducer for each particular instance, so it's not analogous to having both a confirmationDialog class and a registrationDialog class, I'm mounting the same reducer for both. Look at this diagram: C++ (or ES6 js): +------------------------>-----------------+ In the top code the word "Dialog" tells the compiler that registrationDialog is of type Dialog and it should apply all the behavior related to dialogs to this instance. in the bottom code the word dialogReducer is telling redux that it should apply all the behavior related to dialogs to this instance. So I don't think that it's analogous to having both a |
Right, so So your state could look like this: {
topBar: {
leftMenuId: 'LEFT_MENU',
rightMenuId: 'RIGHT_MENU'
},
menus: {
'LEFT_MENU': { // state for the menu },
'RIGHT_MENU': {// state for the menu }
}
} |
I accepted that redux don't want to implement it. But don't expect me to say that it's better for redux to not support oop. If i was to decide I'd definitly add full oop support or moved toward supporting it OR I'd remove
I never said that redux is anti-OOP, currently I'm using it in my oo code and it's doing great though it's not doing "perfectly fine" as it makes me rewrite some code. I can avoid rewriting by providing another edition of postgres is indeed fully supporting oop. It even supports inheritance: https://www.postgresql.org/docs/8.4/static/tutorial-inheritance.html what part of postgres do you think is not supporting oop. Even mysql can be considered fully compatible with oop but postgres is absolutely definitly fully supporting oop. Consider that postgres and mysql are storages. Supporting oop in context of a storage means how much it provides in its api that the programming language can apply oop paradigms with ease on the storage. What postgres offers in its api for the programing language to ease oop is just state of the art. |
What if I want two |
Yes, I agree, and it does. If you want two instances of a {
application: {
topBarId: 'top',
purpleBarId: 'purple'
},
bars: {
'top': { menusIds: ['left', 'right'] },
'purple': { menuIds: ['purpleMenu'] }
},
menus: {
'left': { // ...state for menu },
'right': { // ...state for menu },
'purpleMenu': { // ...state for menu }
}
} So |
@naw maybe I'm not understanding something here, let me demonstrate it with an example. file path: components/c1/reducer.js
file path: reducer.js
In the above example the top level |
Yes, thanks for the example, that's helpful. It really depends on how you intend for c1 to be reused. If it expects that its parent provide a certain storage structure for it, your implementation would be different than if the child expected to be completely autonomous. There might be use cases for both, but going with what I believe is a more likely (and less coupled) approach, let's assume the child component is completely autonomous. In such a case, the fact that both the parent and the child use identical objects (dialogs and menus) is incidental, not contractual. Therefore, their storage for those would be separate from each other. Within each autonomous "application", they would each use collections of instances, rather than individual instances mounted in specific named places in the state tree. So, I would have single dialogs collection for the parent, and a separate single dialogs collection for the child, and similarly, a single menus collection for the parent, and a separate menus collection for the child. Four slices of state for four collections. Here is the parent state: {
application: {
m1: 'someMenuId',
d1: 'someDialogId',
c1: childStateBlackBox,
},
menus: {
'someMenuId': { // menu state },
},
dialogs: {
'someDialogId': { // dialog state }
}
} The black box state for your child would look like this: {
application: {
m1: 'rightMenu',
m2: 'leftMenu',
d1: 'aDialogId'
},
menus: {
'leftMenu': { // menu state },
'rightMenu': { // menu state },
},
dialogs: {
'aDialogId': { // dialog state }
}
}
As for the reusability of dialogs and menus --- in this case, you are mounting a generic Is it clear that mounting two menus reducers for two menu collections in two autonomous (but nested) "applications" is different than mounting a menu reducer for every menu instance? Finally, my use of "application" as a subkey for the singleton data is a convenience, but not particularly important to the overall approach. |
@naw I like this. Both this and my proposal need unique ids instead of user defined ids though to prevent duplications if they want to be distributed as a package in npm. |
We've been using reducer composition inside the per-component Redux stores, as our components actually have multiple reducers which use each other. I don't remember now if we used P.S. Sorry tl;dr the whole discussion, so maybe losing some context here. |
@sompylasar thanks for sharing this. Did you experience any negative side effects using multiple stores? For example I guess time travel shouldn't work alright with multiple stores. |
Yes, we haven't used the DevTools and hot reload on this project, our setup is somewhat custom, not mainstream. But nothing stops from making time travel manually, this just would require more work because there will be no ready-to-use tools. There is no magic actually. |
I really enjoyed reading this discussion and share many sentiments with @sassanh. I've came here my during research for the same problem - thanks for inspiration to all of you! |
@qbolec if you're still concerned about this problem you can check this repo: https://github.com/foxdonut/meiosis it's just a pattern that you apply in your code base, not a library. I loved their ideas and we had a great discussion about the issue here over there: foxdonut/meiosis#23 (comment) I thought it may be interesting for you too. |
Suppose that I'm trying to publish a react component to be used in redux applications. I want the react component to be stateless and keep its state in redux. Suppose that my component consists of an
actions.js
, areducer.js
and anindex.jsx
for example. The developer who wants to use it combine its reducer in some part of his redux store viacombineReducers
provided byredux
(orredux-immutable
). Then when an action gets dispatched the reducer perfectly changes the relevant part of state instore
. So good so far. But what if the the developer wants to use it multiple times?Suppose that the component is named
Component
and this is the desired component tree of the developer:Container-1
|
|--
Container-2
| |
| |--
Component
| |
| |--
OtherComponent-1
|
|--
Component
|
|--
OtherComponent-2
And suppose that
Container-1
combines reducers ofContainer-2
,Component
andOtherComponent-2
andContainer-2
combines reducers ofComponent
andOhterComponent-1
.Now when an action from
actions.js
is dispatched, both reducers will apply it to their state (if developer doesn't do anything to prevent it.)Currently I'm solving this by providing an
elementId
field in my actions related to reusable components and providing the sameelementId
field in initial state for relevant parts of state. For example my initial state for above configuration would be like this:Then I check if the
elementId
provided in action matches theelementId
of the state in the beginning of the reducer function. This way the action only affects desired instance of element.It'd be great if we could have some utilities provided in redux for this purpose. For example combineReducer can take these
elementId
-s (or whatever else) and check it automatically when it receives an action and ignore the action if itselementId
doesn't match. It can do it only if theelementId
is provided.The text was updated successfully, but these errors were encountered: