Skip to content

Skip shape props in flow #1066

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

Skip shape props in flow #1066

wants to merge 1 commit into from

Conversation

Agontuk
Copy link

@Agontuk Agontuk commented Feb 9, 2017

Fixes shape type props issue for flow.

@Agontuk
Copy link
Author

Agontuk commented Feb 9, 2017

I forgot to run the unit test, I'll check and try to update as soon as possible.

@Agontuk
Copy link
Author

Agontuk commented Feb 9, 2017

There's some test for flow which give error for unused shape prop types. Example:

class Hello extends React.Component {
    props: {
        name: {
            unused: string
        }
    }

    render () {
        return <div>Hello {this.props.name.lastname}</div>;
    }
}

But in my case, even if I use the prop it gives me error. Here's a example of my component:

type Props = {
    position: {
        x: number,
        y: number
    }
};

class Example extends Component {
    props: Props

    render() {
        const { position } = this.props;

        return (
            <div style={ { left: position.x, top: position.y } }>Hello World</div>
        );
    }
}

This gives me error position.x prop is defined but never used. Since currently there's a issue with detecting shape props, should I remove those test cases ? Or just use eslint-disable-line from my end ?

@ljharb
Copy link
Member

ljharb commented Dec 13, 2019

@Agontuk hi! this PR still needs some tests; any interest in adding some?

@ljharb ljharb added the flow label Dec 13, 2019
@Agontuk
Copy link
Author

Agontuk commented Feb 3, 2020

@ljharb sorry for the late response, unfortunately I'm not using flow for quite a while. So it's difficult for me to add the required test cases. I hope someone can pick it up and make the required changes.

@ljharb ljharb marked this pull request as draft October 15, 2020 18:35
@ljharb ljharb force-pushed the master branch 6 times, most recently from 59af733 to 865ed16 Compare November 11, 2022 02:45
@ljharb ljharb force-pushed the master branch 4 times, most recently from 069314a to 181c68f Compare November 18, 2022 17:19
@ljharb ljharb force-pushed the master branch 2 times, most recently from 380e32c to 51d342b Compare July 4, 2024 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants