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/no-unused-proptype false positive #1183

Closed
georeith opened this issue May 9, 2017 · 3 comments
Closed

react/no-unused-proptype false positive #1183

georeith opened this issue May 9, 2017 · 3 comments

Comments

@georeith
Copy link

georeith commented May 9, 2017

The following is considered an error for the props a and b:

import React from 'react';

export default function SomeComponent(props) {
    const callback = () => {
        props.a(props.b);
    };

    const anotherCallback = () => {};

    return (
        <SomeOtherComponent
            name={props.c}
            callback={callback}
        />
    );
}

SomeComponent.propTypes = {
    a: React.PropTypes.func.isRequired,
    b: React.PropTypes.string.isRequired,
    c: React.PropTypes.string.isRequired,
};

Removing the line const anotherCallback = () => {}; fixes the false positive.
The following does not trigger an error:

import React from 'react';

export default function SomeComponent(props) {
    const callback = () => {
        props.a(props.b);
    };

    return (
        <SomeOtherComponent
            name={props.c}
            callback={callback}
        />
    );
}

SomeComponent.propTypes = {
    a: React.PropTypes.func.isRequired,
    b: React.PropTypes.string.isRequired,
    c: React.PropTypes.string.isRequired,
};
@jseminck
Copy link
Contributor

jseminck commented May 21, 2017

I have been taking a look at this issue as I wanted to familiarize myself with the codebase a little bit. I managed to reproduce the issue through tests, and found a fix for it. Though I have no idea if the fix is valid or not. It does not break any other tests, though.

I will make a PR with the fix later today or tomorrow. Then anyone more experienced with the code can take a look. Even if my fix is not correct it might help someone else figure out what the issue really is.

For clarification, this also works (declaring anotherCallback before callback):

export default function SomeComponent(props) {
    const anotherCallback = () => {};

    const callback = () => {
        props.a(props.b);
    };

    return (
        <SomeOtherComponent
            name={props.c}
            callback={callback}
        />
    );
}

SomeComponent.propTypes = {
    a: React.PropTypes.func.isRequired,
    b: React.PropTypes.string.isRequired,
    c: React.PropTypes.string.isRequired,
};

@ljharb
Copy link
Member

ljharb commented May 21, 2017

Thanks, good to start somewhere.

@ljharb
Copy link
Member

ljharb commented May 24, 2017

I believe this may be fixed with #1218. Please reopen after the next release if not.

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

No branches or pull requests

3 participants