Eslint-plugin-react: Rule proposal: prefer-default-props - Disallow ES6 default parameter syntax on component defintions

[adamterlson]

Justification: create a standardized way to define default props across stateless and class components so as to have a unified style and to allow for the inspection of defaultProps for external usage. For example, the programmatic generation of component documentation that includes default prop values.

This rule should only impact the function which defines a stateless functional component and the constructor of component classes.

---

Example of incorrect code for this rule:

function MyStatelessComponent(foo = 'bar') {
    ...
}

class MyStatefulComponent extends React.Component {
    constructor(props = { foo: 'bar' }) {
        ...
    }
}

Example of correct code for this rule:

function MyStatelessComponent(foo) {
    ...
}
MyStatelessComponent.defaultProps = { foo: 'bar' };

class MyStatefulComponent extends React.Component {
    constructor(props) {
        ...
    }
}
MyStatefulComponent.defaultProps = { foo: 'bar' };

// Also valid:
class MyOtherComponent extends React.Component {
    static defaultProps = { foo: 'bar' };
}

[ljharb]

MyStatefulComponent.defaultProps({ foo: 'bar' }); is a noop - also, defaultProps can be either a function or an object literal.

I'm still not sure what the rule you're suggesting is for.

[adamterlson]

I tried to clarify the purpose of this rule in the justification section above. I'm not sure how else to say it I guess. Can you help me out by asking some questions about what is not clear?

[ljharb]

In "examples for correct code" you have MyStatefulComponent.defaultProps({ foo: 'bar' }); which is not correct code, because that's a noop expression. Correct code would be MyStatefulComponent.defaultProps = foo where "foo" is either an object literal, or a function that returns one.

[adamterlson]

You are absolutely right, that was a syntax error on my part that I've corrected above.

I hope the rule's intent and potential impact on React development is clear regardless. If this is not being expressed well or is not self-evident, I'd love to discuss this further!

To be open, my team plans to develop an eslint rule for this regardless because we've decided it's important for our team and we are the sort of folks who tend to write rules for things we determine to be best practice. I opened this issue so I can know whether this rule is of interest to this project and the broader community, and if so we can submit a PR.

Do you (and any others that may find this discussion) feel that this rule is worth a PR or should we not bother?

[ljharb]

• a rule that requires defaultProps be present for all non-required props: strong +1
• a rule that disallows default arguments in the constructor of a React component: +1

Are you suggesting anything else?

[adamterlson]

a rule that requires defaultProps be present for all non-required props: strong +1

I think this is a valuable rule, but is not one that I am proposing here (I believe that is a duplicate of other open issues already).

a rule that disallows default arguments in the constructor of a React component: +1

This is what I am going for. :) When I said in the title "component definitions" I was trying to leave room for stateless functional components as well, but if your usage of "constructors" includes these as well then yes, this is exactly what I mean. The rule should apply to all ways a React component is defined, to disallow using ES6 default arguments on received props, instead preferring to use defaultProps.

No, I am not proposing anything else here.

[ljharb]

yes, constructor slash SFC.

OK, in that case I'm +1 on this request :-)

[adamterlson]

Awesome! Thank you. :) Expect a PR on this in the indeterminate future. Hopefully soonish.

[Jessidhia]

Not sure if out of scope, but I use this pattern often, and it sounds like the kind of thing you want to catch with this rule too:

class AComponent extends React.Component {
  /* lifecycle methods */
  render () {
    const {
      props: {
        requiredProp,
        optionalProp = '' // default value!
      }
    } = this

    return <div><h1>{requiredProp}</h1>{optionalProp}</div>
  }
}

[adamterlson]

I should think that is indeed out of scope to try and detect effective defaulting within the function body. Quite difficult to detect all edge cases, such as:

const foo = this.props;
const { bar = 'bar', baz = 'baz' } = foo;

and

const foo = { ...this.props };
foo.bar = foo.bar || 'bar';

Ideally yes, we wouldn't allow any such cases either! :)

[bl-nero]

May I ask what exactly is wrong with using default values in functional components?

[adamterlson]

@bl-nero By "default values" I assume you mean default function arguments instead of using Component.defaultProps, yes?

If yes, one answer is that a component is easiest to reason about if there's only one consistent mechanism by which props are defaulted. If DeveloperA uses defaultProps and DeveloperB uses function argument default syntax, there is no standardized pattern by which the same result is affected in the code base.

Since React has a mechanism for this, it should tend to be the default method over the other methods shown. So, another answer is, because it's statically set on the component itself, writing tooling against it is much simpler than trying to figure out what happens to the values of props internal to the component's implementation.

[dartess]

It looks relevant: defaultProps on functions will eventually get deprecated
Thence I was looking for the rule "forbid propTypes on FC")

[ljharb]

It’s still an RFC; once it’s merged and released in a non-candidate react release, the best practice may change - but for now it remains to use defaultProps and not default values.