Rule proposal: jsx-no-undef-props

[beefancohen]

This rule enforces that no props have the value undefined. In React, these props will not be passed down to the instance of the component, so just suggest to the user to leave off the prop instead of passing undefined as its value.

Not sure if this is wanted or needed, but I thought it could be useful! Don't be afraid to say you think it's stupid though 😝

[ljharb]

How else would you make a prop conditionally present?

[beefancohen]

True - you can move the logic outside of the prop value and just pass a var, but should probably just special case this to ignore. In this case, the rule is kinda pointless :)

render() {
   const prop = foo ? undefined : bar;

   return <Hello name={prop} />;
}

[ljharb]

This seems like it adds a little value when the prop value is solely the literal undefined - but it wouldn't be able to catch any variables whose contents were undefined, and the conditional case seems like a useful way to conditionally specify a prop.

[beefancohen]

Yeah that was kind of the point to check for literally undefined - this was more for newcomers to React to say you don't need to pass down an undefined prop that you've maybe defined in propTypes. I can see, also, that this conflicts with #611 which I think is more valuable, anyway.

[ljharb]

I don't see how it conflicts with #611 - that's checking for unused propTypes within the same component file, at the definition site, while this one checks for undefined props at the rendering site.

We already have a rule that checks for literal true, so I think it's helpful to have one for literal undefined as well.

[beefancohen]

Ah you're right I misunderstood - it would only conflict if someone defined a prop in propTypes and then weirdly just passed it down with undefined as a value, but I don't think that's much to worry about. I'll update to just check for the literal undefined.

[ljharb]

LGTM

[petersendidit]

This should use the new eslint rule format

[ljharb]

@evcohen this seems like a bad merge - can you force push to the branch and update the PR in-place?

[ljharb]

@evcohen I think it needs a rebase, NOT a merge, so as to reduce the 56 commits included in this PR to a more manageable number

[beefancohen]

@ljharb as soon as i get a few mins i will rebase and squash

[ljharb]

i think you need to fix some merge conflicts :-)

[beefancohen]

that's not supposed to be there?!? 😛

[ljharb]

I think this rule should NOT warn when a null/undefined prop follows a spread prop - I often do <div {...obj} override={null} /> to do an "omit".

[ljharb]

should this check for null also, since React ignores them both?

[beefancohen]

i didnt think react ignored null? i was under the impression that when passing down null, if you have a defaultProp set, it won't be set; whereas if a prop is undefined, the defaultProp is applied.

for reference: http://stackoverflow.com/questions/34809178/react-js-default-prop-is-not-used-with-null-is-passed

[beefancohen]

also re: following spread operator- if the above is true and you want to omit a value, we should probably ignore in the case that you want the defaultProp to be applied. or maybe this renders this rule futile because of that distinction.

[ljharb]

ah, good to know, thanks

[ljharb]

right, i'm suggesting always ignoring cases or either that appear after a spread prop.