React: Display a warning if Component is called without props

[bisubus]

Do you want to request a feature or report a bug?

This is a feature (enhancement).

What is the current behavior?

props parameter isn't validated in Component and PureComponent. Omitted props is a common mistake that results in undefined this.props in constructor. This may result in a problem:

class MyComponent extends Component {
  constructor() {
    super();
    this.state = { a: 1, b: this.props.b }; // cannot read b property of undefined
  }
}

A problem may be harder to determine if previously working code stops working when refactored:

class MyComponent extends Component {
  constructor() {
    super(); // no error
  }

  componentWillMount() {
    // rewriting this to constructor code will result in situation above
    this.setState({ a: 1, b: this.props.b });
  }
}

What is the expected behavior?

Component and PureComponent validate props parameter to be an object, or arguments > 0 at least and display a warning in development mode in case props isn't passed from child constructor.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

React 16.7.0-alpha.0

[sagar-gavhane]

Hi @bisubus,
I agree with you but how can it helps to developers. You can also try static state or methods for the same.

Check out this article for more reference. https://michalzalecki.com/react-components-and-class-properties/

import React, { Component } from "react";

class App extends Component {
  state = {
    appName: this.props.appName
  };

  render() {
    return (
      <React.Fragment>
        <div className="container my-5">
          <h2>{this.state.appName}</h2>
        </div>
      </React.Fragment>
    );
  }
}

export default App;

[sophiebits]

This is a good idea, but since we made super(); work if you don't access this.props in the constructor, and using this.props in the constructor is relatively rare, I don't think it is worth it to add this at super(); time.

However we could perhaps have a warning when accessing this.props if we make it a getter?

[bisubus]

@sagar-gavhane That's the style of state declaration I personally prefer because the problem with super argument just doesn't appear with class field.

@sophiebits A getter is a reasonable solution. I don't see nothing wrong with a warning in constructor though. It's simpler to implement and promotes a good practice to provide parent constructor with correct argument, even if this.props is secured in lifecycle hooks . A lot of nagging React warnings have recommendatory nature.

[open-source-explorer]

@gaearon @bisubus @sophiebits I'm a novice here. Can you guide me?

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution.

[bisubus]

The issue is still relevant.

[necolas]

I don't think anyone on the core team will work on adding this as longer term we're moving away from classes altogether. It's likely this issue will be closed as if there's no community PR for this by the time this it's marked again as stale

[bisubus]

After some research it seems that making props a descriptor will cause more problems than it will solve. It's a downer that super() without args has been already allowed, so disallowing it would result in breaking changes. For anyone having mistakes described in the issue, they can be prevented by enforcing the use of super(props) with TypeScript or Flow.