More on reusability for custom components.
Closed this issue · 32 comments
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, a reducer.js and an index.jsx for example. The developer who wants to use it combine its reducer in some part of his redux store via combineReducers provided by redux (or redux-immutable). Then when an action gets dispatched the reducer perfectly changes the relevant part of state in store. 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 of Container-2, Component and OtherComponent-2 and Container-2 combines reducers of Component and OhterComponent-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 same elementId field in initial state for relevant parts of state. For example my initial state for above configuration would be like this:
{
"container-1": {
"component": {
"elementId": "topRightCornerAvatar",
...
},
"other-component": {...}
},
"component": {
"elementId": "topLeftCornerAvatar",
...
},
"other-component-2": {...}
}
Then I check if the elementId provided in action matches the elementId 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 its elementId doesn't match. It can do it only if the elementId is provided.
A couple quick thoughts:
- Remember that
combineReducersis simply a utility for the most common use case. Redux users are expected and encouraged to write custom reducer logic for their own specific use cases. - This seems like a fairly niche use case, and is not likely to be something that would be built into Redux itself.
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:
- Consider you want to make something like Material-UI, Zurb Foundation or Bootstrap and you want its elements to use the global redux store as their storage. Obviously users want to have multiple instances of an element in their page simultaneously (the element being a menu, a button, a slide show or whatever else). Currently the the library uses redux it should ask its users to assign an unique
elementId(or something like that) to the relevant part of state for each element they instantiate.
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 combineReducers.
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:
- Generate an
idin combineReducers and provide it in the state. The wrapper incombineReducerthat dispatches actions to its child reducers should take care of theidchecking and don't dispatch action to reducers with differentid-s than the one specified in the action. Then add to documentation how developers can enable thisidgenerator for their reducer (probably as an option forcombineReducersor a property of the reducer function) and how they should provide thisidin their actions. - Ease the action creation by something like modifiers that automatically assigns the
idin the action. If we have any utility that automatestypeassignation, it should give options to automateidassignation too, that's enough.
That's all I did manually in my current project and it was enough for me to have redux as storage backend of my classes.
P.S. Of course id is supposed to be unique, something like uuid.
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 combineReducers which can easily live in separate package.
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 combineReducers generated an ID for a given "instance" of a component, how does the consuming component get that ID?
It would be helpful to see an example of what your arguments to a modified combineReducers would look like.
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 combineReducers.
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 combineReducers should be generating IDs, that's a no-no, because that means that the reducers are not pure functions any more.
On top of that, this really sounds like a very specialized use case, and not at all the sort of thing that combineReducers was intended for. In addition, the Redux team aims to keep the core of Redux as minimal as possible, and provide enough of a base to allow additional concepts to be built on top of that minimal core. You may want to read #1768 and #1813 to get an idea where Dan stands on changes to the core Redux APIs (including some examples of how a change that can be done "in just a few lines" actually has very wide-reaching implications).
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 combineReducers itself (as a factory) being impure, and the reducer that it outputs being impure.
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 combineReducers as @naw pointed out it's our factory function, we should keep combination (defined here https://github.com/reactjs/redux/blob/master/src/combineReducers.js#L122) pure not the combineReducers itself.
About it sounding like a very specialized use case, well it may not be a use case for a todo app or other web apps that don't need oop, but we're talking about combing redux with oop architecture. I can't call it a specialized use case. If we support oop well I'm sure there'll be lots of applications using redux and oop together and it'll provide better, cleaner ways of coding for web apps.
@naw The ids should be generated in combineReducers function (https://github.com/reactjs/redux/blob/master/src/combineReducers.js#L100) but don't mistake it with combination function (https://github.com/reactjs/redux/blob/master/src/combineReducers.js#L122).
Quoting from your comment:
In order for any given component to be connected to redux, it has to know where in state to look.
Currently combineReducers already provides it, for example when you do this:
const reducer = combineReducers({a:redcuerA, b: reducerB})
the whole state is not passed to reducerA, only store.getState().a is passed to reducerA and its results will be replaced in store.getState().a so it can't change other parts of state. That's 80% of job combineReducers is already doing. The only missing thing here is a problem with dispatching actions that is even though a reducer only applies to its relevant part of state, currently there's no way in redux to make an action only be sent to a specific reducer. So if you dispatch an action to focus a button for example, it'll be dispatched to all reducers in reducers tree generated by combineReducers. Fortunately we can manage to apply this action only on button reducers with the help of type field in the action (for example setting type to "BUTTON_FOCUS") but in order to make it only apply on an specific part of state that's bound to our desired button, we need to prevent other reducers in reducers tree to reduce this action. To achieve this we need to assign an id to the action and to the reducer and make the combination function (https://github.com/reactjs/redux/blob/master/src/combineReducers.js#L122) to only apply the action on reducer if these ids match.
This is how it goes (just a sample implementation):
combineReducers({
a: reducerA,
b: {
reducer: reducerB,
reusable: true,
},
b2: {
reducer: reducerB,
reusable: true,
}
})
now combineReducers generates an unique id for b and another one for b2 storing it in its context when and it checks here https://github.com/reactjs/redux/blob/master/src/combineReducers.js#L140 if there's an id assigned to reducer, if so it next checks if this id matches the action.id and will apply action on reducer only if these ids match. That's all this way we have the remaining 20% done.
Look at one of my reducer functions in my current project, this is the reducer of my Menu component:
export default (state = new Map(), action) => {
if (action.elementId === state.get('elementId')) {
switch(action.type) {
case actions.SET_ACTIVE_PATH:
state = state.set('activePath', action.activePath);
break;
}
}
return state;
};
I have that if statement in all of my reducer functions, because I'm using redux as storage backend of an oo application. Now I can have 6 menus simultaneously, all I have to do is:
const topBarReducer = combineReducers({
leftMenu: menuReducer,
rightMenu: menuReducer,
})
const finalReducer = combineReducers({
loginDialog: dialogReducer,
registrationDialog: dialogReducer,
confirmDialog: dialogReducer,
stickyMenu: menuReducer,
topBar: topBarReducer,
leftSideMenu: menuReducer,
contextMenu: menuReducer,
})
and my initial state would be something like this:
{
loginDialog: {elementId: 'LOGIN_DIALOG', ...},
registrationDialog: {elementId: 'REGISTRATION_DIALOG', ....},
confirmDialog: {elementId: 'CONFIRM_DIALOG', ...},
stickyMenu: {elementId: 'STICKY_MENU', ...},
topBar: {
leftMenu: {elementId: 'LEFT_MENU', ...},
rightMenu: {elementId: 'RIGHT_MENU', ...},
},
leftSideMenu: {elementId: 'LEFT_SIDE_MENU', ...},
contextMenu: {elementId: 'CONTEXT_MENU', ...},
}
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 combineReducers. If you want your slice reducers to only respond to specific actions, you can just write your own little higher-order reducer that does that. In fact, here's a quick little generic filtering higher-order reducer to give you an idea:
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 combineReducers is not the place to put that.
@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 combineReducers actually call filterById for you and inject an elementId based on the position in the state tree so that you don't have to spell it out explicitly, then that's certainly a viable convention for your app . Personally I'm doubtful that's a good convention for all redux apps, but that's just my opinion. Bear in mind redux doesn't even force you to use a hash with keys as your state object -- your object could literally be any JavaScript object (although it's recommend it be serializable)
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, combineReducers (or your replacement) is just a factory that builds a reducer from a given abstraction. If you think you've landed on a particular abstraction that works well, could I suggest that you release your custom combineReducers function in a package and see if it's useful to anyone else? --- you don't need buy-in from redux core to achieve this, since no changes to redux code are necessary --- on the contrary, redux is completely plugable in this regard since anyone can choose any abstraction they want and write a factory to build a root reducer to give to createStore.
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 <Dialog id="CONFIRM_DIALOG"/>. I hope it's clear that anything you do in combineReducers is only the reducer side of things ---- you still have to have a way to get the id into the components themselves (which isn't shown in your example). Are you hardcoding it?
@markerikson @naw Thanks for your detailed comments and opening the issue in details. Personally I think the current combineReducers is "partially" oop friendly, if it was the case that redux core should stay minimal and features should be added by external extensions then I think combineReducers shouldn't apply state changes only to a part of state. If it was the case combineReducers itself shouldn't exist in redux code at all, or at most it should just combine reducers taking them as array (not hash map) and just apply them in order (or even to keep core more minimal, don't even guarantee the order) to whole state not just part of it. The fact that it's applying the reducer only on an specific part of state makes it half way compatible with oop and I don't like incomplete things personally so I'd either complete it or remove it altogether and people would make different packages with different combineReducers for community and plug it to redux. But I'm sure there were good reasons to have combineReducers this way in core, I'll be glad to hear about these reasons.
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 combineReducers in core while it could be plugged later and to know how you see 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:
export default TopBar = props => <span>
<Menu {...props.leftMenu} />
<Menu {...props.rightMenu} />
</span>;
export default Application = props =><span>
<Dialog {...this.props.loginDialog} />
<Dialog {...this.props.registrationDialog />
<Dialog {...this.props.confirmDialog />
<Menu {...this.props.stickyMenu} />
<TopBar {...this.props.topBar} />
<Menu {...this.props.leftSideMenu} />
<Menu {...this.props.contextMenu} />
</span>;
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 react-redux repo because what I wanted to achieve was involving React. I wanted to discuss an architecture of reusable React+Redux components -- complex views backed by a Redux store, not just Redux components.
@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 react-redux because I feel guilty participating in adding another "x-y-z-w-q" package to npm and I feel guilty solving this issue only for react while it that can be solved for all libraries (including react) by adding a package like "redux-reusable-components" (it has one dash not two 😆 , it's just "x-y".). Consider it as a personal preference. I hope someday I can discuss this issue in detail that why it's harmful for community. I have 15 years background of coding in Python and C++, everything there is intuitive, you don't need to read long discussions in github to construct a simple app (or even a complex one) usually your first guess is the right answer, you don't have to use a brand new architecture that released 5 months ago or so for every new project you start. All of these are opposite in JS community, here there's a brand new mainstream architecture every year, not only you can't guess how to solve your problems by intuitive solutions but also you need to read long discussions in github to find out how to solve it. I'm trying to find out why this is like this, so far I found one of the reasons is the "x-y-z-w-q" packages. As I said I hope I can discuss this later when I have enough knowledge about it.
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 combineReducers isn't really redux-core (in fact, I've said that before in another thread), and from a principled standpoint, you could definitely make the argument that it should not be in this repository. However, I can also see the pragmatic argument that it is a reasonably good tool for many initial use cases, and is perhaps a helpful tool for beginners. Overall, I share @timdorr's thoughts that it is often over-used.
Personally I think of combineReducers as just a "sane default" or an "example", not part of redux-core, notwithstanding which repository it actually lives in. I'd encourage you to think of it the same way, and not get too caught up in which repo it lives in.
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 combineReducers.
Can you imagine how awful redux would be if it forced you to use combineReducers without providing any lower-level API? The good news is that redux does provide a lower-level API as its primary API (i.e. createStore accepts any root reducer you want to give it).
One problem I've seen with having it in this repo is that programmers have trouble thinking outside the combineReducers "Box". On the contrary, you've done a great job thinking outside of the box --- rather than being limited by combineReducers, you've imagined a higher-level abstraction that would be a better fit for the conventions you want to follow in your app. This is a good thing. However, I hope you can have enough humility to see that just because you found an abstraction that you like, doesn't mean it's an abstraction that anyone else will find useful? This is why I've encouraged you to pursue your abstraction and release it publicly --- who knows, it might catch like wildfire?
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 (createStore), release it as an external package, and see if it's popular or not. Now, if you had an idea that could only be implemented by changing something in the lower-level redux api, then that might be a different story.
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 combineReducers API. If we accommodated each of those suggestions, combineReducers would be a bloated frankenstein utility with an incomprehensible API, when all it's meant to be is a tool for the common pattern of wanting to delegate work from the root reducer to smaller reducers. This is completely analogous to having different OOP ORM classes handle the instances for different tables, rather than a single class handling updates for all tables for all classes. In other words, it already does what it is meant to do. Anything beyond that, is really a different tool, and the community has lots of room for innovators (such as yourself) coming up with different tools. Who knows, someone might stumble on something that completely eclipses combineReducers in the future.
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 dialogsReducer and a menusReducer, each of which knows how to update an individual "record/instance" by ID, in the same way that an OOP ORM would update one row in a table.
So combineReducers is seen more as a way to divide up your "tables", not as a way to divide up your "instances". In other words, idiomatic redux is actually very complimentary with your anology to OOP, just perhaps not in the exact way that you're thinking about it.
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 confirmationDialogs table/class and a registrationDialogs table/class in your OOP world, which I'm sure you would agree is odd. And if that's the pattern you wanted to follow, then a factory (to create almost identical classes/tables/reducers) pattern would indeed be a good approach, as we've suggested.
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:
- singletons
- "tables" of instances.
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, redux itself doesn't make such decisions for us.
@naw I really enjoy and benefit from reading your ideas.
you could definitely make the argument that it should not be in this repository. However, I can also see the pragmatic argument that it is a reasonably good tool for many initial use cases, and is perhaps a helpful tool for beginners.
I strongly believe that the use cases with a fully oop compatiblecombineReducers is more than a partially oop compatible combineReducers. I don't consider oop as my personal convention. Outside js community almost every app is written oo. Reusing a component (whether it's react component or whatever else), reusing a class is, instantiating a class multiple times is as common as calling functions in C++, Python, .net, etc. And I think it'll be as common in future of js too. Js won't be used as a functional language forever. ES6 already has first class classes. It's proven that oop makes reusability easier by an order of magnitude. Come on when you write <div><Component/></div> you're instantiating Component. $('.component').component() has no room in future of web app development although it's instantiating too. One of the things redux provides among others (dispatching actions, reducing, etc) is providing an storage for the application. I don't know why should its dispatch and reduction be compatible with oop, but its store need another external package. I totally understand (not totally to be honest but lets assume totally 😄 ) if it puts all for external packages but this partial support is what bothers me. But it's alright from your comments I conclude that I can ignore the existence of combineReducers.
However, I hope you can have enough humility to see that just because you found an abstraction that you like, doesn't mean it's an abstraction that anyone else will find useful? This is why I've encouraged you to pursue your abstraction and release it publicly --- who knows, it might catch like wildfire?
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 combineReducers exists in this repo there should be complete oop support, this is what I think is a "should" but the convention to achieve this, well mine was a suggestion and I'm ready to hear others suggestions. If you say that combineReducers doesn't belong to this repository then I can accept that elementId (or any other convention for full oop support) doesn't belong here too, but all I say is that combineReducers is offering incomplete oop utilities. Anyways lets just ignore combineReducers's existence. I know it's not practical to eliminate combineReducers from code base now and I can just not use it (or use my own edition of it that supports oop completely.) -> it's alright.
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.
Well you're just a part of front-end community, many applications in the community don't use redux nor react. Many still use $('.x').y() many still use window.x = y. I'm sorry if you I wrote it in a way that you thought I'm criticizing community around redux, but I really meant the whole frontend community.
Some reducers are designed to handle singletons. Other reducers are designed to handle several instances of a class. combineReducers is a tool to help you delegate to other reducers, but is completely agnostic about how/what those reducers do, and whether or not they handle singletons or instances.
It is a lower-level tool, and in that sense, completely accommodates OOP by virtue of being agnostic about it. Saying that combineReducers is faulty because it doesn't handle OOP is like saying HTTP is faulty because it doesn't handle OOP (OK, that's a bit of an exaggeration, but the point is it exists on a lower-level plane).
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 -- combineReducers handles both ---- just build on top of it.
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?
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:
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 TopBar component that had its own Menus and Dialogs. How would you support it? Just consider it as a C++ code, it's like you're instantiating all your instances on top of int main() body. Maybe you want to instantiate in a function in another class that is instantiated in another function of another class that is instantiated ...
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 confirmationDialogs table/class and a registrationDialogs table/class in your OOP world, which I'm sure you would agree is odd. And if that's the pattern you wanted to follow, then a factory (to create almost identical classes/tables/reducers) pattern would indeed be a good approach, as we've suggested.
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):
+------------------------>-----------------+
| |
Dialog registrationDialog; |
... v
Dialog confirmationDialog; |
| |
+------------------------->------------+ |
| |
Redux: v v
combineReducers({ | |
confirmationDialog: dialogReducer, <---+ |
registrationDialog: dialogReducer,<---------+
})
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 confirmationDialog and a registrationDialog, these are names of instances not classes.
Consider my example, there were a TopBar component that had its own Menus and Dialogs. How would you support it?
Right, so topBar is a singleton, which knows which menus belong to it.
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 combineReducers and let another external package do the rest.
Considering my experiences this is what I'd decide, considering yours you may decide differently. Arguing about the reasons behind it is way out of the context of this discussion.
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?
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 combineReducers for myself and ignore current combineReducer and then it'll do "perfectly fine".
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.
Right, so topBar is a singleton, which knows which menus belong to it.
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 }
}
}
What if I want two TopBars? (rename TopBar with MenuBar then it seems more rational to have multiple instances of it. For example http://www.clemson.edu/ has the black menu bar which consists of two menus and a purple menubar that has a single menu)
The architecture should support regular (not singleton) instances as children of regular instances. And there may be infinite levels of these regular instances with having regular instances children.
The architecture should support regular (not singleton) instances as children of regular instances. And there may be infinite levels of these regular instances with having regular instances children
Yes, I agree, and it does.
If you want two instances of a Bar, then you have this state:
{
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 menus and bars are your tables, each of which has as many instances as you want. Somewhere else, you have actual application state (which is a singleton) which defines the overall structure (which is separate from the data).
@naw maybe I'm not understanding something here, let me demonstrate it with an example.
file path: components/c1/reducer.js
...
export default combineReducers({
m1: menuReducer,
d1: dialogReducer,
m2: menuReducer,
})
file path: reducer.js
...
import c1Reducer from './components/c1/reducer.js';
export default combineReducers({
c1: c1Reducer,
m1: menuReducer,
d1: dialogRedcuer,
contextMenu: menuReducer,
})
In the above example the top level reducer.js doesn't need to know about the components inside c1, c1 provides a reducer and top level reducer uses it. If c1 is a package installed via npm and decides to change its structure in its next version, the top level reducer doesn't need to change according. Is it possible in your design?
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 }
}
}c1 is completely free to change its storage structure at any time, and the parent component doesn't have to know anything about it.
As for the reusability of dialogs and menus --- in this case, you are mounting a generic menus reducer in two places --- once for the child, and once for the parent, and likewise, you're mounting a generic dialogs reducer in two places ---- once for the child, and once for the parent.
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.
Thanks for sharing your ideas and criticizing my ideas. I hope I see a package providing this functionality in near future. I'll create one if I find the time for it.
@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.
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 combineReducers as a tool to implement simple reducer composition, but I remember we had to make our own "combiners" that work similarly to Elm architecture. The parent reducer unwraps actions for the child (nested) reducers. For example, the parent reducer accepts a hashmap of states, and an action that contains an identifier into that hashmap, and a payload for a child reducer action: { type: 'ACTION_X', payload: { key: '123', payload: { foo: 'bar' } } }, the child reducer accepts the state found by the key from the parent action, and an action: { type: 'ACTION_X', payload: { foo: 'bar' } }.
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.
@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.