-
-
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
React Elements in Redux state (docs/discussion) #1793
Comments
As an alternative solution, could you have Foo put into state just the props necessary to render the Elements, rather than the Elements themselves? So instead of: import connect from 'react-redux'
// state.elements = [Element1, Element2, Element3]
function Floats({ elements }) {
return (
<div>
{ ...elements}
</div>
);
}
export default connect(({ elements }) => ({ elements }))(Floats) You could do: import connect from 'react-redux'
import ElementClass from 'components/ElementClass'
// state.elementProps = [props1, props2, props3]
function Floats({ elementProps }) {
return (
<div>
{
elementProps.map(props =>
<ElementClass {...props} />
)
{
</div>
);
}
export default connect(({ elementProps }) => ({ elementProps }))(Floats) This approach could still work, with some adaptation, if your elements are of different types/classes. |
@wdhorton That'd be better, yes, but we still have non-serializable props such as event handlers, etc. My point is, summarising: if we are willing to sacrifice persistence and re-hydration, is the React-Elements-in-Redux-state option (or the one you propose, Element-props-in-Redux-state) acceptable, even if state is not serializable? Are we losing other benefits I'm not aware of? |
So that SO question was apparently one that I answered, and I also happen to have written the FAQ. (And thank you for actually having read and linked the relevant discussions :) ) The common pattern I've found is that most questions about Redux fall into two categories:
The point and the emphasis is not that things like that are impossible. It's that they are non-idiomatic, and in the vast majority of use cases, not a good idea. So, we tell people "don't do that". It's like many other things in life: you start off with the simpler explanation, even if it's just a bit "wrong", and then later when someone has a better grasp on the situation, fill in the details that they can now appreciate and understand. But in the end, that's merely strong prescriptive advice. I can give an opinion, Dan can give an opinion, anyone else can give an opinion, but it's all just opinions. Ultimately, since it's your app, the end decision on any aspect is indeed entirely up you. If you have considered your options, and are willing to trade off time traveling for your specific use case, then sure, go ahead, put React Elements in your store. |
@markerikson Thanks for your reply! I absolutely don't like this solution (even with the mitigation proposed by @wdhorton, which in fact I used for another part of the application), but I couldn't come up with a more idiomatic or elegant one. Of course, if somebody has an idea on how to solve this problem idiomatically in Redux, it would be great! |
You can totally do that. I prefer keeping (de)serialization possible though. One easy way to allow it is to store a string ID corresponding to the component instead of its function type. Then you can have a generic component that performs map lookup: import Button from './Button'
import Switch from './Switch'
import TextField from './TextField'
const types = { Button, Switch, TextField }
export default function Something({ typeID, ...props }) {
const InnerComponent = types[typeID];
return <InnerComponent {...props} />
} Now you can keep something like |
@gaearon What if you don't know the elements that might be rendered before runtime, such as in a plugin based system, how would you handle this with redux? |
@leonaves : Dan's approach is still valid. Plugins could potentially declare the names of the components they need to show, maybe with a prefix for the name, and the main app could add those to the lookup table of component types. Dan describes this approach in more detail in http://stackoverflow.com/questions/35623656/how-can-i-display-a-modal-dialog-in-redux-that-performs-asynchronous-actions/35641680 . |
The Redux docs recommend not putting anything non-serializable in store state, and link to related discussions about not putting React components in state (#1248) but rather identifiers of components we want rendered (#1390). What is not explicitly covered is the possibility to include React elements in state.
@gaearon explains React Elements very clearly as "plain object describing a component instance or DOM node and its desired properties" (see here and here). Conceptually, it doesn't look like a great "heresy" to put Elements in state if the need arises.
In my particular case, I have this need. I have Component Foo that wants to render a dropdown palette (Palette) but can't do it himself because of some CSS constraints (it is inside a
div
withtransform: translateZ(0)
for performance, and hence the dropdown is truncated when usingposition: fixed
). One way to solve this is to have Foo send a description of Palette to another component (call it Floats) higher up the render tree. Floats is not constrained by thattransform
CSS and can position its floating children freely. It could even receive Elements from multiple React Components and centrally manage them, keeping these Elements (why not?) in a Redux store. The store could handle actions for creation, update, removal of those floats.My question about the orthodoxy of this Elements-in-state idea on StackOverflow. was answered with a clear NO and even down-voted (!!), but no alternative way was suggested. I finally ended up doing it, since (a) I don't need to persist/re-hydrate this state; and (b) it doesn't break time travel (as stated here; I built a fiddle to show this, but Redux DevTools seem to break in JsFiddle).
Maybe we could discuss (and add our conclusions to the docs (#857)) whether this solution is bad, in which scenarios it may be OK, or even if there is a more orthodox way for transferring Elements (not Component descriptors) through state.
The text was updated successfully, but these errors were encountered: