-
Notifications
You must be signed in to change notification settings - Fork 47k
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
Should I use Statefy? #3653
Comments
Seems like bad practice to me (you're exposing internals), the general advance AFAIK is to create stateless components and provide a stateful wrapper components for them. This may be preferable even if you're only interested in stateful components as it separates logic from presentation. |
Not an actual issue so closing out. Feel free to discuss further here or (ideally) the mailing list. |
@andrerpena Yeah, I agree with @syranide; sounds like a bad practice. The design I use in my React code is to have a mutable AutoCompleteState object that is created&held by your controller-component, and pass that down to your autocomplete component. Your autocomplete component is free to manipulate that state object, and the API of that state object can expose any state that the autocomplete desires to expose. That way, the state is still all held by the controller-component, but the internal state is also opaque to the controller and is a single variable instead of a whole mess of internal variables that the controller doesn't know/care about. Personally (and note that this isn't the opinion of the React team/community at large, just my extremist view), I think that all components should be either: 1. purely state+logic (no user interface, behave like a controller) or 2. purely render functions (a stateless view of the data). I always pull a component's internal state into a mutable object and keep it in the controller component, but I do it as a single "internalState" variable for each subcomponent to avoid long chains of callbacks and variables-I-dont-care-about. My model (same that @syranide mentioned) turns out to be particularly good for examples like the one you highlighted, because it means the controller can easily "read" the "value" of the text component (from the public API of the component's state variable) rather than responding to callbacks (to set a variable in the controller that remembers the value). The parent never needs a reference to the child component and never needs to respond to the nightmarish hell of callback chains. It also gives the parent an easy way of "resetting" the child component (throw away the internalState object and create a new one); something that is not easily achievable when the children have state. Of course, there are still plenty of use cases that warrant callbacks and individual props, so use your good-engineering-discretion; don't go crazy with the single-variable idea. Generally, anything that a parent will likely control/edit should be separate from anything that is "internal component state" |
@JSFB 👍 Just a quick side-question, do you guys have a good component naming convention for that pattern? |
Usually we just try to pick a name that makes sense for the component we're building. The biggest recommendation I can give is that for a given component (eg. We generally use the word "container" to describe a controller that holds the UI components, but that's not common in the community (that's mostly our word internally at Facebook). |
Posting here, mostly so I don't loose the link when people inevitably ask me about this again. A good article about state hosting: https://www.safaribooksonline.com/blog/2015/10/29/react-local-component-state/ |
Hey folks. We're implementing a set of React components and, because of what I explain in the
motivation
section, we created Statefy.The problem is: I'm not sure if it's a good practice. I'm afraid of using it in a large number of components just to find out it would be way better to do it in another way.
Could you please, give your feedback on this project? That would be much appreciated.
Thanks.
The text was updated successfully, but these errors were encountered: