[New] no-children-prop: Add allowFunctions option

[alexzherdev]

Resolves #1803

[ljharb]

I'd also like a way to forbid functions from being children at all - ie, maybe an option functions that can be an enum: child, prop, forbid, or allow?

[alexzherdev]

Sounds good 👍
Although I'm curious, what would be the use case for not allowing functions at all?

[ljharb]

My personal belief is that function children are an abomination that destroys the entire purpose for using jsx, and I never want them to exist in code I maintain.

[alexzherdev]

Ok, I'll have to do my research 😃 But immediately, forbid would not even allow a children prop, I'm just not sure how that is different from e.g. onClick={() => {}} (which I know the downsides of, but that's already handled by a different rule)

[ljharb]

Oh sure, I just want an enum value that's equivalent to "not specifying the option"

[alexzherdev]

Ok, I don't think I'm on the same page. Let me go over it once again.

Current rule: children prop is a warning no matter what.
My PR:

• allowFunctions: false (or not specified): no change to current rule
• allowFunctions: true: functions in the children prop are not a warning. Functions in the child position are a warning - isn't that what you were going for originally?

And then extending that, the enum would be the 4 possible combinations of either allowing or forbidding functions in a prop or in the child position. Not specifying the enum value retain the current behaviour of the rule, which would be equal to the child option: functions are allowed in the child position but not as a prop. prop would be what allowFunctions: true does in my PR.

[ljharb]

ahh i see what you mean - ok, fair. then true/false is probably fine.

altho i can also see people asking for a third option, which is requiring that functions be in the child position.

[alexzherdev]

But don't they already have it via the current rule. As soon as you do children={() => {}}, it will warn, thereby forcing you to move the function into the child position.

[ljharb]

Right - but what if they want all non-function children in the prop position, and function children in the child position?

Or, all non-function children in one position or the other, but function children disallowed?

[alexzherdev]

That sounds like a complicated schema, so maybe we start small to make sure those options are actually in demand? Also the rule being called no-children-prop kinda implies we should be discouraging the children prop 🤔

[ljharb]

ha, true. the main purpose is to force child position.

[alexzherdev]

Ok, let me know if any other changes are necessary.

[ljharb]

let's say this instead: when true, and passing a function as children, it must be in prop position and not child position.

[G-Rath]

Any update on this? I'd love to see this feature land, since libraries like react-router seem to recommend this pattern.

[EvHaus]

LGTM. Fans of FaCCs will be screaming in horror at the idea that this rule exists. :)

[codecov-commenter]

Codecov Report

Merging #1903 (241c38b) into master (e0c0812) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #1903   +/-   ##
=======================================
  Coverage   97.40%   97.40%           
=======================================
  Files         111      111           
  Lines        7428     7444   +16     
  Branches     2715     2723    +8     
=======================================
+ Hits         7235     7251   +16     
  Misses        193      193           

Impacted Files	Coverage Δ
lib/rules/no-children-prop.js	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e0c0812...241c38b. Read the comment docs.