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

Check for undefined values of defaultProps #1153

Closed
NikolayFrantsev opened this issue Apr 18, 2017 · 5 comments
Closed

Check for undefined values of defaultProps #1153

NikolayFrantsev opened this issue Apr 18, 2017 · 5 comments

Comments

@NikolayFrantsev
Copy link

Using require-default-props rule following code is considered as valid:

Component.propTypes = {
  value: PropTypes.string,
};

Component.defaultProps = {
  value: undefined,
};

But actually this code works like:

Component.propTypes = {
  value: PropTypes.string,
};

Component.defaultProps = {
};

So it would be great if require-default-props rule will also check that case and throw errors for defaultProps keys with undefined values.

@ljharb
Copy link
Member

ljharb commented Apr 18, 2017

Why is that a problem?

The point of the rule is that defaultProps are explicit' it doesn't matter at all what their value is.

@NikolayFrantsev
Copy link
Author

Specifying undefined as default prop value is not required since it always undefined by JS architecture:

const Component = ({ min, max )} => (
  <div>{typeof min}/{typeof max}</div>; // <div>undefined/undefined</div>
);

Component.propTypes = {
  min: PropTypes.number,
  max: PropTypes.number,
};

Component.defaultProps = {
  min: undefined,
  // max: undefined,
};

require-default-props rule asks you to set something for non required props, so to suppress eslint varning you may use undefined when it's not actually required by React.

@ljharb
Copy link
Member

ljharb commented Apr 18, 2017

It's not required by JavaScript. It's required by this rule, since an explicit undefined is much much better than an implicit one.

@enricotelesca
Copy link

enricotelesca commented Aug 9, 2017

TBH I think @Shutnik is right and sorry, but, @ljharb , explicit undefined is much much better is your opinion. would you ever do something like this because explicit undefined is much much better?

const obj = {
  foo: 'something',
  bar: undefined
}

I don't see the foundation of @ljharb explanation... I think a good trade-off is leave the possibility to omit the default prop only if the propType is object, node, element and maybe an array

@ljharb
Copy link
Member

ljharb commented Aug 9, 2017

@enricotelesca yes, and in fact that would increase performance of obj if obj.bar were ever set to something else, because it would be doing it without changing the "shape" of the object.

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

No branches or pull requests

3 participants