New `prefer-arrow-children` rule

[astorije]

Consider the following snippet:

export const BarComponent: React.FunctionComponent<{
  children(arg: number): React.ReactNode;
}> = ({ children }) => <div>{children(42)}</div>;

export const FooComponent: React.FunctionComponent = () => (
  <BarComponent>
    {function bar(x) {
      return <>This is x: {x}</>;
    }}
  </BarComponent>
);

We are currently transitioning from TSLint to ESLint, and the no-function-expression rule of https://github.com/microsoft/tslint-microsoft-contrib was catching this in favor of:

export const FooComponent: React.FunctionComponent = () => (
  <BarComponent>{x => <>This is x: {x}</>}</BarComponent>
);

There are a bunch of rules helping in some ways for this, but not this specific situation:

• prefer-arrow-callback helps ensuring [1, 2, 3].map(x => x * 2) is preferred over [1, 2, 3].map(function (x) { return x * 2; }) but as far as I know has no effect on JSX props/children
• react/jsx-no-bind (which we set up with allowArrowFunctions: true) helps ensuring something similar with props: onClick={() => setFoo(true)} is preferred over onClick={function () { return setFoo(true); }}

I am not aware of a rule or configuration that would ensure arrow functions are used with children props expecting functions. Is there such thing? If not, would it make sense as a new rule?

_I know of eslint-plugin-prefer-arrow which enforces all functions to be arrow functions. It is however very strict and we cannot use it as is because we have hundreds of top-level function declarations (we were using allow-declarations and allow-named-functions of only-arrow-functions)._

[ljharb]

Why would you prefer children to be an arrow function? prefer-arrow-callback makes sense because the whole point of arrow functions is to replace inline callbacks (for example, Components should ideally be normal non-arrow functions, despite that not everybody uses that style).

Props are also inline callbacks in a way that components are not.

[astorije]

Correct me if I'm wrong, but it seems you are talking about 2 separate things: components and children props.

Regarding components, the main (if only) reason why I'm seeing people define them as arrow functions is because of type definitions in TypeScript:

 export const MyComponent: React.FunctionComponent<MyComponentProps> = (props) => (/* ... */);

By doing so, I ensure that not only MyComponent accepts a set of props + the children props defined in the FunctionComponent type, but also that it has the correct return type. I haven't looked into it in a while, but last I checked there was no convenient way to do this with arrow functions.

But my ticket wasn't about components so I'll go back to my initial topic 😅

Why would you prefer children to be an arrow function?

For the same reasons we prefer to use arrow functions for props: children is merely a render prop. To make sure I understand you correctly, is the first snippet above what you suggest we should prefer?

<BarComponent>
  {function children(x) {
    return <>This is x: {x}</>;
  }}
</BarComponent>

That seems unnecessarily verbose to me, don't you think? What does using function bring here over an arrow function?

[ljharb]

I'm not suggesting you should prefer either one - I'm asking you why you want to have a preference.

[astorije]

Apologies if I'm missing something, but didn't I answer the question above?

For the same reason prefer-arrow-callback exists, it's preferable to use arrow functions for children and other render props. children is not a component, it's a render prop with a special syntax. As you mentioned above, arrow functions are well-suited for replace inline callbacks and props are also inline callbacks.

For example, if someone opened a PR with a children shaped like my last snippet from my last comment above, would you be ok with it or would you request them to use an arrow function instead? Since we know we would always request such change, we think it would make more sense if the linter was enforcing it.

[ljharb]

What about for non-children render props?

[astorije]

I'm not following, can you clarify what you're asking?

[ljharb]

<Foo render={function () { return null; }} render2={() => null}>
  <Bar>
    {function () { return null; }}
  </Bar>
  <Baz>
    {() => null}
  </Baz>
</Foo>

There's 4 interesting places here; which ones would you want to error with your rule?

[astorije]

Ah, excellent, thanks!

So for archive / repro purposes, I used the following TypeScript code to run the linter against:

import React, { FunctionComponent, ReactNode } from 'react';

const Foo: React.FunctionComponent<{
  render(): ReactNode;
  render2(): ReactNode;
}> = ({ children }) => <>{children}</>;

const Bar: React.FunctionComponent<{
  children(): ReactNode;
}> = ({ children }) => <div>{children()}</div>;

const Baz: React.FunctionComponent<{
  children(): ReactNode;
}> = ({ children }) => <div>{children()}</div>;

export const Example: FunctionComponent = () => (
  <Foo
    render={function() {
      return null;
    }}
    render2={() => null}
  >
    <Bar>
      {function() {
        return null;
      }}
    </Bar>
    <Baz>{() => null}</Baz>
  </Foo>
);

As expected, render causes react/jsx-no-bind to complain (with JSX props should not use functions) but render2 is valid.

So in a similar fashion to what react/jsx-no-bind currently does, my suggestion here was that some mechanism would disallow the child of Bar (function) in favor of the child of Baz (arrow function). However, instead of a new rule, I would be even happier if react/jsx-no-bind treated children like any other props. That way all render props would be treated the same way using the existing rules and recommendations.

[ljharb]

That seems like a reasonable option to add to jsx/no-bind, something like includeFunctionChildren?

[astorije]

That would be great!

I'm assuming you won't want it enabled by default right away because it could be a breaking change, but for the sake of consistency, it would be even nicer if it could be enabled by default at the following major bump.

[ljharb]

I don't anticipate a major bump for a long time; an option is indeed required in the meantime.