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

Default lifecycle methods on React.Component ? #5047

Closed
arcanis opened this issue Oct 3, 2015 · 9 comments
Closed

Default lifecycle methods on React.Component ? #5047

arcanis opened this issue Oct 3, 2015 · 9 comments

Comments

@arcanis
Copy link
Contributor

arcanis commented Oct 3, 2015

Just a small API detail: would it make sense to implement default (empty) lifecycle methods in the base React.Component class? It would allow to have homogenous implementations as far as super() calls are concerned.

For example, the following will throw due to the missing componentDidMount implementation in the parent class:

class Foo extends React.Component {
    componentDidMount( ) {
        super.componentDidMount( );
    }
};

However, when overloading the Foo class, a missing super call might introduce some bugs. That's what got me to think that maybe it was safer to always put the super-call, and then I discovered that it was not possible when heriting from the base class.

@sophiebits
Copy link
Collaborator

We don't recommend inheritance for building your components.

@jedwards1211
Copy link
Contributor

@arcanis inheritance gets pretty awkward when you have propTypes, defaultProps, contextTypes, childContextTypes, etc. I've sometimes used ES7 decorators when I need to share multiple methods between React component classes, though I usually end up finding a way to do without them.

@arcanis
Copy link
Contributor Author

arcanis commented Oct 3, 2015

@spicyj Really ? Because that's what is documented here. (edit: Oh, sorry, I misread your comment: I should have read that plain classes are fine but inherited classes aren't, right? Well, it still sounds a bit strange to restrict the language features)

@jedwards1211 Yep, I see what you mean. However, it can be solved by using static properties (already supported by Babel) and Object.assign to extend them from the parent to.

@gaearon
Copy link
Collaborator

gaearon commented Oct 3, 2015

Inheritance doesn't (and IMO shouldn't) work well for building React components.
Jordan explains why:

Many people have become accustomed to using OO inheritance not just as a tool, but as the primary means of abstraction in their application. I've you've worked at a Java shop, you'll know what I'm talking about. In my personal opinion, classical OO inheritance (as implemented in many popular languages) is not often the best tool for most jobs, let alone all jobs. But the situation should be approached with even more caution when inheritance is used within a framework or paradigm that uses functional composition as its primary abstraction (React). There are certain patterns we'll want to prevent (there are many strange things that people can come up with when combining render with inheritance that don't make sense and are addressed via simple composition). There's also risk of making mutation more convenient. It might make sense to start with ES6 classes simply as a better syntax for React component creation, intentionally limiting some use cases (limiting inheritance depth, making some React base class methods final) (only when used with React components of course - all non-React use of ES6 classes wouldn't be restricted).

What is your use case?
Are you sure you can't do the same with simple composition?

@gaearon
Copy link
Collaborator

gaearon commented Oct 3, 2015

I should have read that plain classes are fine but inherited classes aren't, right? Well, it still sounds a bit strange to restrict the language features

Inherited classes are fine per se, it's just React won't go out of its way to encourage you to use inheritance. As you noted, you can use inheritance—it's just not very convenient, and for a good reason.

For any custom logic you can use composition better, and the fact that Facebook has written a ton of React components and doesn't use inheritance is a great testament to that.

@arcanis
Copy link
Contributor Author

arcanis commented Oct 29, 2015

Just wanted to make a little follow-up here : I've tried not using inheritance, and using composition instead and ... well, I really like it, actually. I've been able to achieve the same things than before and even more, but in a much cleaner way. When I need to override a behaviour, I just add a func prop to my "overridable" components and that's it.

So I guess that as far as I'm concerned, this issue can be closed. I still think it would be nice to still add the default methods for those who prefer inheritance, but it's not something I would fight for.

Thanks for the lesson!

@jimfb
Copy link
Contributor

jimfb commented Oct 29, 2015

While it is true that we believe in composition over inheritance, I think the original question is still valid, so I'm tempted to leave this issue open.

would it make sense to implement default (empty) lifecycle methods in the base React.Component class?

The main reason I see for doing this: it allows static tooling to better reason about the code, rather than being forced to rely on implicit knowledge. For instance, I could mark a method as @Override, as I always do whenever I'm overriding a method... and then if I spell it wrong or that function gets removed from the parent React.Component in a future version or whatever, I get tooling support. Also allows code assist to give me the lifecycle methods when I ask for suggestions of things to override, etc. It might even improve performance by keeping the implicit classes more similar within the core. Overall I think this would still be a good change, even for people who are using composition over inheritance.

@gaearon
Copy link
Collaborator

gaearon commented Oct 27, 2016

We won't be doing this.
I also wrote about alternatives to inheritance:
https://facebook.github.io/react/docs/composition-vs-inheritance.html

@gaearon gaearon closed this as completed Oct 27, 2016
@dylan-shao
Copy link

dylan-shao commented Jan 17, 2018

Hi @gaearon, I saw in the 'SplitPane ' example of https://facebook.github.io/react/docs/composition-vs-inheritance.html,
the stateless component 'SplitPane' wraps other component like the 'Chat'. But usually the 'Chat' component is stateful, so it ends with a stateless component wrap a stateful component, is it a good practice?(I mean, I have been told stateless component should not have stateful child ) Thanks!

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

6 participants