eslint-plugin-react: Rule `require-default-props` does not support default parameters in stateless function
Hello, I just got some error from require-default-props rule:
`react/require-default-props`: propType "foo" is not required, but has no corresponding defaultProp declaration.
`react/require-default-props`: propType "baz" is not required, but has no corresponding defaultProp declaration.
Here is the code:
function Header({
foo = true,
bar,
baz = () => { /* noop */ },
}) {
return ...;
}
Header.propTypes = {
foo: PropTypes.bool,
bar: PropTypes.bool.isRequired,
baz: PropTypes.func,
};
I expected there is no error, but seems it causes the problem.
Any ideas?
About this issue
- Original URL
- State: closed
- Created 8 years ago
- Reactions: 13
- Comments: 67 (27 by maintainers)
The documentation for React suggests a number of bad practices amidst all the good ones - just because they make the framework doesn’t mean they’re always right 😃
@steida There’s no need for a link - by spec, default values are re-evaluated on every invocation of the function.
defaultProps, on the other hand, are evaluated once and cached at the module level - across every invocation of the function. Even if React does nothing with regard to them (which I doubt, but haven’t bothered to dig up evidence of),defaultPropsare more performant than default values.If you don’t want to use
defaultProps, disable this rule. If you want flow annotations to be equivalent, make sure that your Flow annotations transpile down to defaultProps, and then this rule can and should be adapted to support them - until that time, they’re not the same, anddefaultPropsis staunchly recommended.@ljharb can you please expand on your opposition to default arguments. Considering the React team seems pretty set on deprecating them for function components, and for good reason, what is your opposition to them? Or at the very least, what is your opposition to supporting it as an option? It seems you’re out of step with where the React team is going.
It seems to be that
defaultPropsare at the very least less performant, as they are resolved on every call toReact.createElement?Our team is moving to use default arguments instead of
defaultPropsfor function components, but we use this rule. It would be good for it to support default arguments too.Not “will” - that’s not merged yet.
That’s not included intentionally; react components (SFCs included) should simply not use default arguments at all.
And why does that matter? Obviously, the React team is already decided to deprecate it. By delaying it here you are only increasing the number of users who will have to refactor later because by default they will be forced to use
defaultPropson FC. It should at least inform them about the existence of that option so they can decide.Btw, don’t forget that most users are probably using not ejected CRA apps. They will have no way to enable that option or disable the rule.
I think this seems a bit misleading. Default parameters are a language feature so React and the ecosystem don’t really need to “support” them. Maybe you’re referring to the fact that if both default parameters and
defaultPropsare used,defaultPropswill “win”?Default values are not the same as defaultProps. The rule correctly reports this as an error.
@steida again, you’re talking about
propTypes.defaultPropsare different, and control runtime behavior - not type-checking. Using default values prevents React from being able to make some kinds of optimizations.FYI https://github.com/facebook/react/pull/16210
Sure, that’s a different way to look at it, why not, many ways toward the goal 😃
You might want to have a look at this which helps at least partially to cover default props in function components https://github.com/cvazac/eslint-plugin-react-perf
This is where I don’t agree. Or I mean you are generally right but in this case, it’s actually a natural part of a language to have default values for props in arguments. The
defaultPropsis just React construct that was made long before we had this new luxury.I’ve personally never liked
defaultPropsas they are too detached from the component and I was always been using default values in function components. Thank God (read Microsoft) for TypeScript.There is a problem in that logic. What if you have class components which still needs
defaultPropsbut you also have FC where it would be misleading to havedefaultPropsenforced. That’s why I am suggesting to ease up on the rule for FC components so you still can use the rule for class ones without being constraint toward FC.Ultimately, this whole rule should be deprecated and there should be separate ones for class and FC, but that’s surely a question of some semi-distant future.
Uh, what? 😃 Hooks are not components, that’s true, but hooks are used in components thus making them stateful and no longer stateless. Have you ever used hooks seriously? Cannot imagine what would you say something like that 😆
@nathggns Just a tiny detail, please stop using acronym SFC.
https://reactjs.org/docs/hooks-state.html#hooks-and-function-components
It seems like the happy medium is the following:
no-fc-default-propsrule that warns on the use ofdefaultPropson functional components.require-default-propsto not warn when missingdefaultPropson functional components.no-fc-default-propsto recommended when the change deprecatingdefaultPropson functional components lands in a public releaserequire-default-propsbut for functional components & default arguments (require-fc-default-args? probably a better name for this, naming isn’t my strong suit)While some of us including @FredyC & I may like to go further than this, if @ljharb can agree to this bit of work being done now (by pull request or by a core maintainer) it might be worth starting work on what we can agree on rather than letting “perfect” (from our perspective) be the enemy of good.
@ljharb can you confirm that this is a path you’d be supportive of?
Enabling any new rule by default is a semver-major change, that we try to avoid.
I wouldn’t reject a PR for some kind of
react/props-defaults-stylerule, I’d just want the docs to advise doing what react, and thus the react ecosystem, first-class supports - which at this time is defaultProps.Well, I got pretty used to
FCacronym because it’s available in TypeScript declarations 😃 I think it’s getting very well known, there is no need to use a whole thing in rule names for sure.What are you talking about? I am not suggesting to be forbidding to use
defaultProps. They just shouldn’t be required for FC anymore. If someone wants to use them, the rule could just WARN about future deprecation, but that’s it. I don’t even insist on that warning, but the requirement to use something that’s about to be deprecated is just wrong.I’m not delaying anything; I’m not going to add a breaking change for any reason - and for those using tools that rely on defaultProps - like storybook annotations, etc - silently no longer warning on these could break them.
This may not be your use case, but that doesn’t mean I’m going to break it.
No offense, but you might be slightly behind (perhaps even biased?) on what react ecosystem does. Have a look at any new library that uses function components. I tried a few I can recall and haven’t seen a
defaultPropsthat much. So I would dare to say that people are getting comfortable with that deprecation fairly quickly 😃https://unpkg.com/react-router-dom@5.0.1/cjs/react-router-dom.js https://unpkg.com/formik@next/dist/formik.umd.development.js (single one, it’s still WIP) https://unpkg.com/react-select@3.0.4/dist/react-select.cjs.dev.js
We are disabling the rule currently. We’d prefer for the rule to be able to check that non-required props have default arguments instead.
Not everyone uses or can use TypeScript. (Also TypeScript arguably is a super advanced linter anyway).
@nathggns what is planned to be deprecated isn’t relevant until it actually is deprecated.
However, in particular, defaultProps can be read and reflected on by HOCs and runtime tools attempting to inspect components, where default arguments can not - so i hope the RFC doesn’t land (even if it’s very likely to do so).
That’s why it’s an important rule - it’s bad to use default values instead of defaultProps.
I suppose we could make an option that allows default values but I think the existence of the option would be harmful.