Skip to content

Multi-parameter React function components should be errors #33104

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
danvk opened this issue Aug 27, 2019 · 3 comments
Closed

Multi-parameter React function components should be errors #33104

danvk opened this issue Aug 27, 2019 · 3 comments
Assignees
Labels
Experience Enhancement Noncontroversial enhancements Fix Available A PR has been opened for this issue

Comments

@danvk
Copy link
Contributor

danvk commented Aug 27, 2019

React function components take a single props parameter. If you mistakenly think they take positional parameters, the error message you get can be quite confusing because it says nothing about the number of parameters. Instead, it's usually something to do with assignability.

Even if this is WAI, maybe this helps someone else with this error!

TypeScript Version: 3.5.2

Search Terms:

  • react function component error message

Code

import * as React from 'react';

function FC(a: any, b: number, c: number) {
  return <div>{a} {b} {c}</div>;
}

function MainComponent() {
  return <FC a="A" b="B" c="C" />
}

Expected behavior:

It should be an error to use a function which takes multiple parameters as a component in JSX. This is almost certainly a mistake. It's similar to calling a two-parameter function with one parameter, which is an error:

FC({a: 'A', b: 'B', c: 'C'});
// Expected 3 arguments, but got 1. ts(2554)

Actual behavior:

No error. b and c are undefined at runtime.

Playground Link: link

Related Issues:


Real-world example of the confusing way in which this came up:

import * as React from 'react';

interface RowType {
  [columnName: string]: string | number;
}

function CardComponent(props: {row: RowType; column: string; tooltip: string}): JSX.Element {
  return <div title={props.tooltip}>{props.row[props.column] || ''}</div>;
}

export class MyComponent extends React.Component<{row: RowType}, {}> {
  render() {
    const createCard = (col: string) => (
      <CardComponent
        row={this.props.row}
        key={col}
        column={col}
        tooltip={`This is column ${col}`}
      />
    );

    const els = ['A', 'B', 'C'].map((v, i) => createCard(v));

    return <div>{els}</div>;
  }
}

Refactor createCard to be a function component:

const FunctionCard = (row: RowType, column: string) => (
  <CardComponent row={row} column={column} tooltip={`This is column ${column}`} />
);

class MyComponent2 extends React.Component<{row: RowType}, {}> {
  render() {
    const createCard = (col: string) => (
      <FunctionCard row={this.props.row} key={col} column={col} />
    );

    const els = ['A', 'B', 'C'].map((v, i) => createCard(v));

    return <div>{els}</div>;
  }
}

You get the following error message on the row=:

Type 'RowType' is not assignable to type 'string | number'.
  Type 'RowType' is not assignable to type 'string'.ts(2322)
Toy.tsx(6, 3): The expected type comes from this index signature.

It's not clear at first glance why the type checker is trying to assign RowType to string | number. If you squint hard enough, you eventually realize that FunctionCard is getting called with a single argument:

FunctionCard({
  row: this.props.row,
  column: col
})

TypeScript tries to assign this {row, column} object to the first parameter of FunctionCard, namely row. Since this is RowType, which has an index signature, it tries to assign this.props.row (whose type is RowType) to the value type of the index signature, namely string | number. Hence the error.

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Aug 28, 2019
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 3.8.0 milestone Aug 28, 2019
@fatcerberus
Copy link

If I'm reading this right...

Does --strictFunctionTypes help here? It seems like the problem might be that function types are bivariant by default so that a function which requires { a: string, b: string } is assignable to one that only requires { a: string } (i.e. covariance, which is unsound for function parameters). The error you want requires strict contravariance, which --strictFunctionTypes gives you for function types (but not method types!)

@weswigham
Copy link
Member

IIRC you are actually allowed to have a two-parameter component, but the second parameter is a context that react passes thru for you. I think that pattern is deprecated in place of hooks, though.

@danvk
Copy link
Contributor Author

danvk commented Aug 28, 2019

@fatcerberus I have strict set, so I don't think strictFunctionTypes is relevant here.

@weswigham the react typings do in fact allow a second context param:

    interface FunctionComponent<P = {}> {
        (props: PropsWithChildren<P>, context?: any): ReactElement | null;
        propTypes?: WeakValidationMap<P>;
        contextTypes?: ValidationMap<any>;
        defaultProps?: Partial<P>;
        displayName?: string;
    }

When I try to declare FC using that type, I do get an error:

const FC: React.FunctionComponent = (a: any, b: number, c: number) => {
  return <div>{a} {b} {c}</div>;
}

In any case, a function with three parameters definitely shouldn't be usable in JSX!

@weswigham weswigham added Experience Enhancement Noncontroversial enhancements Fix Available A PR has been opened for this issue and removed Needs Investigation This issue needs a team member to investigate its status. labels Feb 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Experience Enhancement Noncontroversial enhancements Fix Available A PR has been opened for this issue
Projects
None yet
Development

No branches or pull requests

4 participants