Skip to content

fix: check to ensure forbid-prop-types throws only off of React.PropTypes #2739

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

fix: check to ensure forbid-prop-types throws only off of React.PropTypes #2739

wants to merge 1 commit into from

Conversation

nonewcode
Copy link

@nonewcode nonewcode commented Aug 3, 2020

An issue was raised where using an alternative library with a similar syntax (in this case Yup) resulted in the rule forbid-prop-types stating that Yup.array() was an invalid proptype. Adding another check to ensure pragma is used and the check is off of PropTypes, removes the false erorrs thrown up when using those other libraries.

Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! We'll need some regression tests.

@nonewcode nonewcode marked this pull request as draft August 3, 2020 20:35
An issue was raised where using an alternative library with a similar syntax (in this case Yup) resulted in the rule `forbid-prop-types` stating that `Yup.array()` was an invalid proptype. Adding another check to ensure pragma is used and the check is off of `PropTypes`, removes the false erorrs thrown up when using those other libraries.

Signed-off-by: Josh Kelly <josh@codecog.dev>
@nonewcode nonewcode changed the title fix(callee): check to ensure callee object name is PropTypes fix(): check to ensure callee object name is PropTypes Aug 4, 2020
@nonewcode nonewcode changed the title fix(): check to ensure callee object name is PropTypes fix: check to ensure forbid-prop-types throws only off of React.PropTypes Aug 4, 2020
@nonewcode nonewcode marked this pull request as ready for review August 4, 2020 08:18
@nonewcode nonewcode requested a review from ljharb August 4, 2020 08:20
Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'd still need some regression tests to prove the fix is working.

@@ -170,6 +172,7 @@ module.exports = {
CallExpression(node) {
if (
node.arguments.length > 0
&& `${pragma}.PropTypes`
Copy link
Member

@ljharb ljharb Aug 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be comparing this string to anything? otherwise it's just a noop and could be deleted.

Suggested change
&& `${pragma}.PropTypes`
&& node.callee.object.name === `${pragma}.PropTypes`

@ljharb ljharb assigned ljharb and unassigned ljharb Oct 15, 2020
@ljharb ljharb marked this pull request as draft October 15, 2020 17:54
@ljharb
Copy link
Member

ljharb commented Sep 20, 2021

@nonewcode are you still interested in completing this PR?

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.

3 participants