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

react/require-default-props is giving a false-positive for no clear reason #1159

Closed
gajus opened this issue Apr 21, 2017 · 4 comments · Fixed by #2001
Closed

react/require-default-props is giving a false-positive for no clear reason #1159

gajus opened this issue Apr 21, 2017 · 4 comments · Fixed by #2001

Comments

@gajus
Copy link

gajus commented Apr 21, 2017

Plugin v6.10.3.

I am getting an error:

/Users/gajus/Documents/dev/applaudience/applaudience-com/src/app/components/Modal/index.js
  7:3  error  propType "+" is not required, but has no corresponding defaultProp declaration  react/require-default-props

Though children is clearly defined:

// @flow

import React from 'react';
import './styles.scss';

+type ModalPropsType = {|
+  +children?: React$Element<any>,
+  +onHide: () => void
+|};

export default class extends React.Component {
+  static props: ModalPropsType;

+  static defaultProps = {
+    children: null
+  };

  handleKeypress (event: KeyboardEvent) {
    if (event.key === 'Escape') {
      this.props.onHide();
    }
  }

  componentWillMount () {
    document.addEventListener('keydown', this.handleKeypress.bind(this));
  }

  componentWillUnmount () {
    document.removeEventListener('keydown', this.handleKeypress.bind(this));
  }

  render () {
    return (
      <div styleName='backdrop'>
        <div styleName='modal'>
          <div onClick={this.props.onHide} styleName='close-button' />
          {this.props.children}
        </div>
      </div>
    );
  }
}

@gajus
Copy link
Author

gajus commented Apr 21, 2017

Reading the error more closely, it is looking for + property in defaultProps, not children.

This appears related to #961.

A bug.

@gajus
Copy link
Author

gajus commented Apr 30, 2017

@yannickcr can this be assigned a bug label?

@Dimon70007
Copy link

Hi there. I think, that eslint a good package for development. And will be better to disable "react/require-default-props" by default, because Eslint should help to develop apps, instead to obstruct.

@ljharb
Copy link
Member

ljharb commented May 18, 2017

@Dimon70007 this does help to develop apps - and bad practices (like not providing default props for optional prop types) need to be obstructed.

Regardless, this opinion and debate do not belong on this issue, or on this repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

3 participants