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

add forbid-prop-types rule that by default forbids "any", "array", and "object" #225

Merged
merged 1 commit into from Sep 23, 2015
Merged

add forbid-prop-types rule that by default forbids "any", "array", and "object" #225

merged 1 commit into from Sep 23, 2015

Conversation

pwmckenna
Copy link
Contributor

No description provided.

@pwmckenna
Copy link
Contributor Author

@jimbolla made a new branch/pr (all the naming changed)
this pr still addresses #215

@yannickcr
Copy link
Member

Looks great! Can you add some documentation ?

Also, does it handle nested propTypes ?

class Foo extends React.Component {
    render: function () {
        return <div>{this.props.bar.baz.something}</div>
    }
}
Foo.propTypes = {
    bar: React.PropTypes.shape({               // ok
        baz: React.PropTypes.object.isRequired // not ok, PropTypes.object is forbidden
    })
};

@pwmckenna
Copy link
Contributor Author

@yannickcr just added a test that verifies support for nested props. I'll get started on some documentation now. I'll flatten these commits once the docs are looking good.

@pwmckenna
Copy link
Contributor Author

documentation added!

@yannickcr
Copy link
Member

👍

If it is good for you, can you squash your commits ? Then I will merge the PR.

@pwmckenna
Copy link
Contributor Author

updated the docs to mention the suggested replacements for the default forbidden prop types, then squashed. should be good to merge once checks pass.

yannickcr added a commit that referenced this pull request Sep 23, 2015
Add forbid-prop-types rule (fixes #215)
@yannickcr yannickcr merged commit c791caa into jsx-eslint:master Sep 23, 2015
@yannickcr
Copy link
Member

Merged, thanks !

@pwmckenna pwmckenna deleted the forbid-prop-types branch December 31, 2015 00:38
@mydearxym
Copy link

this forbid-prop-types seems not work in this situation:

class Foo extends React.Component {
    const bar = this.props.bar;

    render: function () {
        return <div>{bar.baz.something}</div>
    }
}
Foo.propTypes = {
    bar: React.PropTypes.shape({               // ok
        baz: React.PropTypes.object.isRequired // not ok, PropTypes.object is forbidden
    })
};

@ljharb
Copy link
Member

ljharb commented Sep 16, 2016

That's correct behavior - you should use a shape, or objectOf, on baz.

@mydearxym
Copy link

@ljharb sorry for not make my self clear

class Foo extends React.Component {
    const bar = this.props.bar;

    render: function () {
        return <div>{bar.baz}</div>
    }
}
Foo.propTypes = {
    bar: React.PropTypes.shape({  
        baz: React.PropTypes.string      // not ok, Prop defined but not used
    })
};

this is ok:

class Foo extends React.Component {
    render: function () {
        return <div>{this.props.bar.baz}</div>
    }
}
Foo.propTypes = {
    bar: React.PropTypes.shape({  
        baz: React.PropTypes.string      // ok
    })
};

@ljharb
Copy link
Member

ljharb commented Sep 17, 2016

Gotcha, can you file a new issue for that?

@mydearxym
Copy link

mydearxym commented Sep 17, 2016

yes, i opend #847

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

Successfully merging this pull request may close these issues.

None yet

4 participants