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

default-props-match-prop-types not working with flow #1593

Closed
danrot opened this issue Dec 11, 2017 · 19 comments
Closed

default-props-match-prop-types not working with flow #1593

danrot opened this issue Dec 11, 2017 · 19 comments

Comments

@danrot
Copy link

danrot commented Dec 11, 2017

We are using flow in our project, also for typing the properties. And it seems as if the default-props-match-prop-types rule doesn't recognize flow types. Is that possible? Is that something that could be easily implemented?

@ljharb
Copy link
Member

ljharb commented Dec 12, 2017

hmm - in flow, do you just use default arguments?

This rule isn't actually checking types; it's just checking that defaultProps has a matching entry in propTypes when the propType isn't required. Would you want it to enforce default arguments on any non-required prop?

@danrot
Copy link
Author

danrot commented Dec 12, 2017

I know that it is not about checking types, I actually want what you've described, but instead of checking if there is a matching entry in propTypes it should use the flow types instead. I'll paste an example:

type Props = {
    size: number,
    value: Point,
    active: boolean,
    onClick?: (value: Point) => void,
    arrowDirection?: 'left' | 'top-left' | 'top' | 'top-right' | 'right' | 'bottom-right' | 'bottom' | 'bottom-left',
};

export default class ImageFocusPointCell extends React.PureComponent<Props> {
    static defaultProps = {
        active: false,
        showArrow: true,
    };

    // other code not important for this rule
}

The showArrow key in the defaultProps doesn't exist in the flow type, and it would be nice if this rule (or another rule, if they are not easy to combine) would give me a hint. This often happens for us during refactorings / reviews, and can easily be overseen.

@ljharb
Copy link
Member

ljharb commented Dec 12, 2017

Gotcha. That seems like a reasonable enhancement for this rule.

@PoliakovMaksym
Copy link

I faced the same issue today but only with functional (stateless) components.

Stateless component:
selection_002

Stateful component:
selection_003

@ljharb
Copy link
Member

ljharb commented Dec 20, 2017

@PoliakovMaksym the stateful component error seems correct to me; you're not using anything there that requires it be stateful.

@PoliakovMaksym
Copy link

@ljharb the issue is not with the prefer-stateless-function. This error is totally fine. I showed it to point out that it is not the default-props-match-prop-types error. The thing is that default-props-match-prop-types is present in stateless component and absence in stateful one with the exact same code (except stateless vs stateful differences)

@ljharb
Copy link
Member

ljharb commented Dec 21, 2017

gotcha, thanks for clarifying.

@mike1808
Copy link

what's the status of this issue?

@pbambusek
Copy link

pbambusek commented Jan 23, 2018

Is there any progress on this? We use this quite extensively in our projects.

@vojtakopal
Copy link

Any updates? This would really help us. Thanks.

@tuanphungcz
Copy link

it would be really nice to have this feature. thanks ;)

@ritchieanesco
Copy link

may i ask how everyone is getting around this issue? Are you guys still using flow for stateless components?

@ryami333
Copy link

Still no movement here?

@ljharb
Copy link
Member

ljharb commented Feb 25, 2018

The status of this issue is that it's labeled "help wanted", and nobody has submitted a PR for it yet.

For all those folks that'd like to see this land, please feel free to take a crack at fixing it :-) The maintainers of this project are humans and do not have infinite time at their disposal.

@ardok
Copy link

ardok commented Jun 5, 2018

@ritchieanesco I just disable that eslint rule 🤣

@Elf2707
Copy link

Elf2707 commented Jun 6, 2018

You could just use this configuration for rule:
"react/default-props-match-prop-types": ["error", { "allowRequiredDefaults": true }]

This will allow you to have defaultProps for not optional (required) props without warning and also this will work fine with flow, because for flow prop which has corresponding default prop is not optional prop.

@techieshark
Copy link

One note on the suggestion by @Elf2707 - that appears to work so long as your flow types are in the same file as your component; it does not appear to work if you have flow PropType in another file (like a ./types.js) and import it (import type { ButtonProps } from './types');.

@kevinbarabash
Copy link

"react/default-props-match-prop-types": ["error", { "allowRequiredDefaults": true }]

works in most cases for me, but it doesn't expand type spreads, e.g.

export type SharedProps = {|
   disabled: boolean,
|};

type Props = {|
   ...SharedProps,
   focused?: boolean,
|};

class Foo extends React.Component<Props> {
   static defaultProps = {
      disabled = false;
   };
};

eslint produces the following error:

error  defaultProp "disabled" has no corresponding propTypes declaration

@ljharb
Copy link
Member

ljharb commented Feb 4, 2022

@kevinbarabash your code example doesn't parse in babel-eslint; but when i fix the invalid static defaultProps definition, it gives the expected error (defaultProp "disabled" defined for isRequired propType.).

@ljharb ljharb closed this as completed in 8d8ca3f Feb 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests