Terra-core: CustomProps spread location Inconsistency

Created on 14 Feb 2018 · 2Comments · Source: cerner/terra-core

Issue Description

Issue Type

[ ] New Feature
[ ] Enhancement
[ ] Bug
[x] Other

Expected Behavior

Components should have consistent behavior concerning where customProps, or other such non-first-class props are spread into an underlying element.

Current Behavior

The form-textarea and form-checkbox components both accept additional props other than the first-class props explicitly mentioned in their respective definitions. To ensure consistency in behavior, after talking with @bjankord, we would like to get the opinions of those in the terra community as to whether or not we should always spread additional props before mapping first-class props, to ensure said first-class props always "win".

Source

dylanklohr

Most helpful comment

If we want to be completely consistent across components ( and not case-by-case ) I think we'll have to spread custom props first, otherwise we get into situations where components will break if a consumer spreads a prop the component needs to use internally to function correctly.

class Example extends React.Component {
  constructor() {
    super();
    this.handleOnClick = this.handleOnClick.bind(this);
  }

  handleOnClick(event) {
    // Component on click logic

    if (this.props.onClick) {
      this.props.onClick(event);
    }
  }

  render() {
    return <div {...this.props} onClick={this.handleOnClick} />;
  }
}

This component would not function if the spread was flipped and a custom onClick was passed down in the props.

<Example onClick={this.customOnClick} />

<div onClick={this.handleOnClick} {...this.props} />;

StephenEsser on 14 Feb 2018

👍4

All 2 comments

class Example extends React.Component {
  constructor() {
    super();
    this.handleOnClick = this.handleOnClick.bind(this);
  }

  handleOnClick(event) {
    // Component on click logic

    if (this.props.onClick) {
      this.props.onClick(event);
    }
  }

  render() {
    return <div {...this.props} onClick={this.handleOnClick} />;
  }
}

This component would not function if the spread was flipped and a custom onClick was passed down in the props.

<Example onClick={this.customOnClick} />

<div onClick={this.handleOnClick} {...this.props} />;

StephenEsser on 14 Feb 2018

👍4

I agree, I think we'll have to spread custom props first. I think this is a good war room topic. I also think we should look through and find components where this isn't the case, and see if there is any reason we would spread custom props last.