[New] add no-object-type-as-default-prop rule

[cyan33]

Discussed in

#2847

Test

npm run lint
npm run test

shows no errors and warnings

---

• Added it in README

[cyan33]

cc @ljharb

[ljharb]

LGTM minus the naming/prose concerns, the question about new Symbols, and the test parser comment

[AriPerkkio]

This new rule looked useful so I quickly tested it against +150 repositories. It seems to be crashing quite often on isReactComponentName method. This test should cover at least one of those cases

export default function() {
    return <div />;
}

I'm not sure if all crashes were caused by the same issue. You can find all results from here and log containing all crashing cases from here.

[ljharb]

@cyan33 ping :-) there's a few things to address

[cyan33]

Sorry it took me a bit to get back to this 🙂

Updated:

• Tweaking the name and docs of the rule
• Adding support to check for Symbol('foo')
• Add duplicated test case for default parser, babel-eslint, and typescript-eslint.
• Add test case for anonymous function
  • Simply skip as it's hard to guarantee if it's a real React component just based on its content. I think some false positive is fine in this case since it's not encouraged to not provide a name to the component?

[cyan33]

Ping @ljharb :)

[ljharb]

Overall, I think this looks good. After this pass, if my only feedback is docs prose, I'll tweak it myself prior to landing.

[fengkx]

Is there anything more to be done before this rule can be release? It looks very useful. cc @ljharb

[ljharb]

@fengkx the review comment can be addressed either by @cyan33, or, by someone commenting a link to their branch (NOT another PR) that addresses them.

[fengkx]

@ljharb https://github.com/fengkx/eslint-plugin-react/tree/pr-2848-continue I rebased @cyan33 's branch to master branch and make chage according to the review comment.

[ljharb]

This rule should use the Components.detect utility rather than trying to implement its own component detection.

[ljharb]

@fengkx thanks! this PR has been freshly rebased, and I updated the test conventions as well. The only missing piece here is to use Components.detect. Please ping me when your branch link is updated and I'll be able to pull it in and rereview.

[codecov]

Codecov Report

Merging #2848 (4cfe5d2) into master (bf59919) will increase coverage by 0.00%.
The diff coverage is 97.56%.

@@           Coverage Diff           @@
##           master    #2848   +/-   ##
=======================================
  Coverage   97.54%   97.54%           
=======================================
  Files         127      128    +1     
  Lines        9066     9107   +41     
  Branches     3307     3320   +13     
=======================================
+ Hits         8843     8883   +40     
- Misses        223      224    +1     

Impacted Files	Coverage Δ
configs/all.js	100.00% <ø> (ø)
lib/rules/no-object-type-as-default-prop.js	97.56% <97.56%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[fengkx]

This branch switched the component detection to use Components.detect. Because the Component.detect function no only check the function has a leading upper case letter but also return value( to be jsx or null), I change all test cases to return a null.

I can't find any doc about the Components.detect function. Should we mention this kind of utils in CONTRIBUTING.md ?

ping @ljharb

[ljharb]

Sure, it couldn't hurt to mention it in CONTRIBUTING.md.

Thanks, the added commit looks good; i'll pull it in shortly.

[fengkx]

Could you release a new version for this? @ljharb

[ljharb]

When I’m prepared to field bug reports from the unreleased changes, i will.