Disallow excess properties to React components (for performance)

[OliverJAsh]

Search Terms

react excess props properties component function parameters

Suggestion

import * as React from 'react';
import { ComponentType } from 'react';

type Props = { foo: 1 };
declare const MyComponent: ComponentType<Props>;
declare const propsWithExtras: Props & { bar: 1 };

// Expected error, but got none
<MyComponent {...propsWithExtras} />;

I understand this matches TypeScript's behaviour with functions:

type Props = { foo: 1 };
declare const MyFunction: (props: Props) => void;
declare const propsWithExtras: Props & { bar: 1 };

// No error
MyFunction(propsWithExtras);
MyFunction({ ...propsWithExtras });

However, with React, passing excess props to a component is a performance concern, since those excess props may break component memoization, causing the component to update more frequently than it should.

In my experience, this error most often occurs when wrapping components, where the wrapper components need to "pass through" types to a child:

type MyComponentProps = { foo: 1 };
declare const MyComponent: ComponentType<MyComponentProps>;

type MyWrapperComponent = MyComponentProps & { myWrapperProp: 1 };
const MyWrapperComponent: ComponentType<MyWrapperComponent> = props => (
    <MyComponent
        // We're passing too many props here, but no error!
        {...props}
    />
);

Workarounds I'm aware of: (1) avoid spreading, but this quickly becomes a non-option when a component has many props you have to manually pick and pass through.

const MyWrapperComponent: ComponentType<MyWrapperComponent> = ({ foo, myWrapperProp }) => (
    // Error as expected due to excess prop `myWrapperProp`
    <MyComponent foo={foo} myWrapperProp={myWrapperProp} />
);

(2) Pass through props via an object.

type MyWrapperComponent = { myComponentProps: MyComponentProps } & { myWrapperProp: 1 };
const MyWrapperComponent: ComponentType<MyWrapperComponent> = ({ myComponentProps, myWrapperProp }) => (
    // Error as expected due to excess prop `myWrapperProp`
    <MyComponent {...myComponentProps} myWrapperProp={myWrapperProp} />
);

However then we lose special JSX behaviour such as the ability to pass data attributes as props.

IIUC, this could be a use case for #12936.

Checklist

My suggestion meets these guidelines:

• This wouldn't be a breaking change in existing TypeScript/JavaScript code
• This wouldn't change the runtime behavior of existing JavaScript code
• This could be implemented without emitting different JS based on the types of the expressions
• This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, etc.)
• This feature would agree with the rest of TypeScript's Design Goals.

[ferdaber]

This should probably be renamed to be more general, which is that spread attributes are checked more strictly against JSX expressions. The change would be under the JSX type checking, not in @types/react

[OliverJAsh]

@ferdaber Could you elaborate?

[ferdaber]

I'm just pointing out that in general we have to be wary that the JSX typechecking mechanisms in the compiler doesn't only apply to React. Sometimes there are changes that require changes to the React types, in which it really is a "React types issue," and sometimes there are changes that affect all JSX typechecking, which affects React, Preact, Vue, etc.

In this specific case it seems like it is a change to the general JSX typechecking mechanism so that if this change were implemented we wouldn't need to make any changes to the React types.

[devuxer]

I find it completely baffling that...

<div garbage={3}>Garbage</div> doesn't compile

while

<div {...{ garbage: 3 }}>Garbage</div> does compile

This violates the principle of least surprise, and honestly, I would categorize this as a bug fix, not a feature request.

That said, I don't see why fixing this wouldn't be a breaking change. It would turn previously compiling code into code that doesn't compile. I would think we'd need a new tsconfig strictness setting (e.g., disallowExcessJsxAttributes).

[OliverJAsh]

Another reason not to allow excess properties to React components: it may produce invalid HTML.

[lewisl9029]

A colleague discovered this earlier and I whipped up a quick playground to reproduce:

https://www.typescriptlang.org/play/index.html?jsx=2#code/JYWwDg9gTgLgBAKjgQwM5wEoFNkGN4BmUEIcA5FDvmQFA24QB2q8AYhBAFyZUwB0AZVYBhADwBvOJQCOAV2CUAJgAViYbiyjBGAcwC+APjgBeODTgW4ACkkz5S1RDB6AlCaOjUYZIwPi7ClgqanqiAPRePkZ0AG7IUHBgauimtlhygcFO3ABEAEbxOXB6ANw0ouwQcOJ8tUlOqHpwYQYlzWFwEADWsfFwBVlgKdUBDmq5BVA5ADRwWAAeMFDIjurkkwBMZKXlldW1fAOrjc2t7XCMVVhQxFC7HFLp9kGrxvmFc4vLr+9QG0UtdrXW5AA

It certainly does look like a bug to me.

[dragomirtitian]

@lewisl9029 Excess property checks does not happen on spread to objects either. This is basically the same thing, so I definitely would not call it a bug:

var badProps = {requiredProp: "bar", extraProp: 'bar2'};
let o: { requiredProp: string} = {...badProps}

play

[lewisl9029]

@dragomirtitian I respectfully disagree that this shouldn't be considered a bug.

Consider the following slightly modified snippet from the playground link I posted earlier:

<Foo {...badProps} />; // no error
<Foo requiredProp={badProps.requiredProp} extraProp={badProps.extraProp} /> //error

I think this difference in behavior between the two cases violates the principle of least surprise.

Considering TypeScript knows exactly what properties are in badProps (we can tell by hovering over badProps), Typescript should either consistently accept excess props or consistently reject them regardless of how those excess props are passed into the component.

Exactly which way to go is up for debate. I can see some good arguments for both sides. But the inconsistency here is a problem.

[devuxer]

The biggest reason of all to make excess properties an error is that they're indicative of a bug. If you're passing an excess property to a component, you may incorrectly think you've instructed the component to do something that it's not going to do. This has happened to me before when I either (1) misspelled the name of a valid property or (2) incorrectly thought a property that worked with one component would also work with a similar component (e.g., when using a UI component library). These are the exact types of bugs TypeScript is supposed to help with!

Also, as @dragomirtitian points out, this is not just a JSX problem, it's a problem with object spread in general, as shown in this playground snippet.

While a JSX component is a very common use case where this will crop up, there are plenty of other use cases as well. In every case, though, this is a clear violation of the principle of least surprise and should be fixed.

[joslarson]

The main reason this would be of value to me is for strictly checking common composition patterns. For instance in the example below, it would be nice if this complained that input doesn't take error as a prop:

type MyInputProps = React.ComponentPropsWithoutRef<'input'> & {
  error?: string
}

export const MyInput: React.FC<MyInputProps> = (props) => {
  return (
    <>
      <input {...props} />
      {props.error || null}
    </>
  )
}

Then I would know to do something like this instead:

type MyInputProps = React.ComponentPropsWithoutRef<'input'> & {
  error?: string
}

export const MyInput: React.FC<MyInputProps> = ({ error, ...inputProps}) => {
  return (
    <>
      <input {...inputProps} />
      {error || null}
    </>
  )
}

As mentioned above, letting the error prop spread onto the input produces invalid HTML (which the code base I work in has become littered with).

[bxt]

I'm seeing this quite a bit across our code base as well, not only related to performance, but also with leftover props. Say, we have 1) <Foo exampleProp="a" oldProp="b" /> and 2) const props = {exampleProp: "c", oldProp: "d"}; <Foo {...props} /> in our code. Now someone deletes the oldProp prop from Foo, and TS complains about 1) so they go and fix it. Ofc they think "cool, great that TS helped me there" and call it a day without suspecting that 2) is still around. Now some time later another coder comes across 2) and wonders what the heck the extra prop oldProp does. Is is supposed to be handled by the component or not? The digging through the git history begins.

Also if there are only optional props, <Foo /> type checks (sure) but <Foo {...{extraProp: "b"}} /> does not suddenly?

So this is why I think this is not just a performance problem after all. Either complain about all extra props or not.

Interestingly, this bug was accidentally fixed in the past. But as people mentioned above also, fixing it is a breaking change, so people started complaining (#15463 & #15731) and it was re-introduced in #15595 so I think it has to be fixed behind some flag like strictExcessJsxAttributes as proposed above.

I was wondering, there should be some people that use TypeScript together with JSX, but this bug currently has 8 thumbs up, and a related Stack Overflow question has only 6 upvotes, so I'm a little bit pessimistic tbh...

[eps1lon]

It looks like this is a remnant from contravariance of functions in TypeScript where Component({ excessProperty: 'unhandled' }) is perfectly fine. There are good arguments to not error in this case e.g. when we have function compare(animal: Animal): number and want to call it with compare(Cat) and compare(Dog).

However, anytime you do have components that spread props to other components you most likely don't want to allow excess properties.

It seems to me that we ultimately want to use exact types in these type of components instead of changing the current behavior. If exact types are also implemented for components in JSX then we can resolve this issue without breaking existing code.

[lwolle]

Does anyone know if there is an existing lint rule somewhere that could be used to solve the pain for the time being?

[ErAz7]

@lwolle linters are neither supposed nor able to do such tasks afaik, but if you mean to prevent spreading props, you may use react/jsx-props-no-spreading

But anyways, this makes absolutely no sense to me to see typescript accepting anything in {...props} regardless of component prop types!

[nsaunders]

In case anyone might be interested, I wrote about this here. I wasn't aware of the react/jsx-props-no-spreading rule, but it seems like it would be a good addition to my approach/workarounds.