Javascript: The case against react/require-default-props

[jorrit]

Ever since my project uses Airbnb's eslint preset I've been dutifully complying with the react/require-default-props rule and adding defaultProps to my components even when those default properties were nothing more than '', null or 0 - just to satisfy the linter. No actual source code got changed as a result of this rule, as I just check for any falsy value and not a specific one.

One day, while chasing a bug, I encountered the source code of React.createElement() and witnessed the consequences of my obedience: even though defaultProps has no practical benefit to any of the code in the components, I was forcing React to apply those defaultProps in a loop every time they were created. The sole purpose of a frequently recommended optimization is to remove one loop from React.createElement() and here I was, adding one pointlessly. At once, I overrode Airbnb's otherwise formidable eslint preset to stop forcing myself to waste V8's precious time.

I propose that react/require-default-props and related textual recommendation be struck from your source and documentation.

[ljharb]

No thanks. A single loop of a small, shallow object has virtually zero cost; omitting defaultProps can cause actual breakage - and only one of those bugs is worth any performance hit by using defaultProps, even thought it's negligible.

Do you have actual benchmarks that demonstrate the impact of removing the defaultProps, or are you merely theorizing about algorithmic complexity?

[StephenEsser]

@ljharb

Hey Jordan, I stumbled upon this issue trying to do a little research about this rule. Is there a thread or discussion about the decision to require defaulting all non-required props?

https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/require-default-props.md

[ljharb]

@StephenEsser Not that I'm aware of; but it's very valuable to be able to be guaranteed what "type" a prop is in your code - for example, if it's got a propType of array, and a defaultProp of [], then you can blindly use array methods on it without all the runtime guards.

Do you have any thoughts on why non-required props shouldn't be defaulted?

[StephenEsser]

@ljharb I've run into a few circumstances while developing where a prop is conditionally set, generally using a ternary expression. Using an PropType of string as an example and a reusable UI Component. ( Title being an optional prop )

const title = shouldHaveTitle ? 'Title' : null;

<Component title={title} />

This situation frequently occurs using redux, where title can be null or undefined.

import { connect } from 'react-redux';
import Component from 'Component';

const mapStateToProps = ({ title }) => ({
  title,
});

export default connect(mapStateToProps)(Component);

Assuming the Component has defaulted title to an empty string and attempts to call a function against it.

// Error if title was undefined or null
if (title.length > 0 )

In this example additional logic would be necessary to only pass down title if it has a value or strip out null / undefined values from the props object, otherwise it would error out if the title was sent down as null or undefined.

Not defaulting props increases the development time of the Component by adding additional checks, but could potentially decrease development time using that reusable Component.

Would you consider this abuse of an API or have any thoughts on this?

[StephenEsser]

This is what that Component might look like.

import React from 'react';
import PropTypes from 'prop-types';

const propTypes = {
  title: PropTypes.string,
};

const defaultProps = {
  title: '',
};

const Component = ({ title }) => {
  if (title.length > 0) {
    return <span>{title}</span>;
  }

  return <span>--</span>;
};

Component.propTypes = propTypes;
Component.defaultProps = defaultProps;

export default Component;

[ljharb]

In this case, where there's no use case for having a title as an empty string, I think what you have is fine.

However, if you wanted that to be possible, then I'd do null as the defaultProp. That way it's at least explicit that you've overloaded the type of title to be "string or null".

[StephenEsser]

I plan to think about this a little more, I think the example you gave works out pretty well. I currently have the rule turned off and only default props that have an explicit default value, but I'd like to align as much as possible with the standards of the community.

Thanks for the feedback on this, I really appreciate the quick responses.

[stefek99]

I do not understand the argument regarding performance.

I do understand the argument:

those default properties were nothing more than '', null or 0 - just to satisfy the linter.

[miraage]

React's defaultProps cause performance degradation. Can't say it's dramatical, but still causes excessive check on each render for function component.
Using es6 default function parameters is a better way to go.

[ljharb]

@miraage using language default arguments has the same theoretical performance cost, since it applies that same logic to each render.

[tkforce]

@miraage One potential pitfall is that according to require-default-props

One advantage of defaultProps over custom default logic in your code is that defaultProps are resolved by React before the PropTypes typechecking happens, so typechecking will also apply to your defaultProps. The same also holds true for stateless functional components: default function parameters do not behave the same as defaultProps and thus using defaultProps is still preferred.

So at this point, propTypes will fail to check the default value assigned w/ es6 default function parameters.