Eslint-plugin-react: Rule idea: no spreading props

[mockdeep]

One thing we've noticed tends to make code a little opaque is when we destructure the entire props for a sub-component:

function MyComponent(props) {
  return <MySubComponent {...props} />
}

It would be great to have an eslint rule to force expanding individual props that should be passed down:

function MyComponent(props) {
  return <MySubComponent one_prop={props.one_prop} two_prop={props.two_prop} />
}

[ljharb]

In other words, forbidding spread props, but just of the entire props object?

This seems pretty useful. I'd actually want to forbid spread props in all cases, so that maybe could be configurable.

[mockdeep]

Yeah, a configuration would be good, I think. Sometimes we like to assign local props or use a function to pull them out, which I think we'd like to still be able to do.

function MyComponent(props) {
  return <MySubComponent {...subComponentProps(props)} />
}

function subComponentProps(props) {
  return {
    one_prop,
    two_prop: two_prop * 2
  };
}

Haven't landed on a clear style for that, though.

[thoiberg]

If no one else is actively working on this I wouldn't mind taking a shot. I've run into a few cases where as components have grown using {...this.props} has ended up passing in 10 props to a child component when it only needs 3.

[ljharb]

@thoiberg you can use https://npmjs.com/prop-types-exact on each of your components to help catch that, but the linter rule is also great :-)

[sanjayp]

Just adding that I'd like to see this rule added as well since there are potential performance implications, as noted here: https://flexport.engineering/optimizing-react-rendering-part-1-9634469dca02#63e7.

[benmonro]

@thoiberg did you ever get anywhere on this?

[thoiberg]

@benmonro no sorry, I got a little way's in but work got in the way 😞. Feel free to take over if you have the desire 😄

[iegik]

I think that rule was about to not use this syntax:

<Foo {...{
  bar: 'tar',
  moo: 0,
}} />

and use that instead:

<Foo
  bar="tar"
  moo={0}
/>

Reasons:

1. as mentioned in @sanjayp's comment
2. only one syntax must be preferred

[iegik]

@mockdeep

function MyComponent(props) {
  return <MySubComponent {...subComponentProps(props)} />
}

->

function MyComponent(props) {
  const { one_prop, two_prop } = subComponentProps(props);

  return <MySubComponent one_prop={one_prop} two_prop={two_prop} />
}

[mockdeep]

@iegik yeah, that could work if it's too hard to implement my suggestion above.

[KutnerBitsrc]

yes this is needed.
A lot of places in our code have {...props} and it makes the code unmaintainable

[ashbhir]

I can take this up, will raise a PR soon

[KutnerUri]

This is more or less solved by Typescript.
A year ago, when I posted the last comment, I was working on a legacy project with half working Flow types. The _no-jsx-spread rule would still be useful in that project_.

In my current project, I no longer have this problem, as I am passing and combining types for props.

In fact, in a type-safe world, it actually makes more sense to spread props to allow the full range of native props with auto complete.

type LoaderButtonProps = ButtonHTMLAttributes<HTMLButtonElement>
  & { customProps... };

export default function LoaderButton(props: LoaderButtonProps) {
    const [ isLoading, setLoading ] = useState(false);
    //(this could be optimised, I know)
    const handleClick = props.onClick && (e: MouseEvent<...>) => {
        setLoading(true);
        props.onClick(e).then(() => setLoading(false));
    }

    if(isLoading) return <Loader/>;

    return <button {...props} onClick={handleClick} />
}

[ljharb]

This presumes that every component is defined to only allow exact props (which they’re not by default in TS) and that TS types are capable of fully expressing the type for all JS values (it isn’t), and that all components you use are authored in TS (unlikely if you use anything from npm).

[KutnerUri]

I agree.
When these conditions are not met the _no-jsx-spread_ rule should apply. (even in entire project)

in TS files, I found this pattern to be useful enough:

import Rainbows ...;
import { LoaderButton, LoaderButtonProps } from '../LoaderButton';

type SuperProps = {
    rainbow: boolean;
}

type SuperButtonProps = LoaderButtonProps & SuperProps;

function SuperButton(props: SuperButtonProps) {
    //in case of conflict, composing component should decide what props go where
    // e.g. in case of className
    const { rainbow, className, children, ...baseProps } = props;

    return <LoaderButton {...baseProps} >
        {rainbow && <Rainbows className={className}/>}
        {children}
    </LoaderButton>;
}

what do you think?

[ljharb]

I think you should explicitly pass the props you need, only.