jsx-eslint / eslint-plugin-react

React-specific linting rules for ESLint
MIT License
8.99k stars 2.77k forks source link

'no-unused-prop-types' breaks when using destructuring in jsx attributes #2222

Open ViggoV opened 5 years ago

ViggoV commented 5 years ago

I was scratching my head over why no-unused-prop-types didn't trigger at all for some of my components, and after some experimentation it turns out it happens whenever I use destructuring in jsx attributes:

const DumbComponent = props => {
  const someObj = { value: 619 };
  return (
    <SomeOtherComp {...someObj} />
  );
};

DumbComponent.propTypes = {
  unused_prop: PropTypes.string // doesn't trigger
};

but

const DumbComponent = props => {
  const someObj = { value: 619 };
  return (
    <SomeOtherComp value={someObj.value} />
  );
};

DumbComponent.propTypes = {
  unused_prop: PropTypes.string // triggers
};
ljharb commented 5 years ago

I think you mean object spread - "destructuring" is a different feature.

This is because we can't statically know what you're spreading in - so when you spread, we currently assume they're all used.

However, in this case, the props object isn't what's being spread, so I'd assume it would still trigger.

ViggoV commented 5 years ago

Ah yes, i meant spread. Thanks for the correction.

zdavis commented 5 years ago

Seeing the same thing here.

No error for react/prop-types:

import React, { PureComponent } from "react";
import PropTypes from "prop-types";

export default class TestCase extends PureComponent {
  static propTypes = {};

  render() {
    const foo = { bar: this.props.bar };
    return <div {...foo} />;
  }
}

Error for react/prop-types is triggered in this case:

import React, { PureComponent } from "react";
import PropTypes from "prop-types";

export default class TestCase extends PureComponent {
  static propTypes = {};

  render() {
    const foo = { bar: this.props.bar };
    return <div />;
  }
}
manneredboor commented 3 years ago

I have faced same issue using typescript

Here it works correctly:

type Props = {
  unusedPropType: string; // Triggered as supposed to be
  link?: string;
  onClick?: React.MouseEventHandler<HTMLDivElement | HTMLAnchorElement>;
  children?: React.ReactNode;
};

export function Tag({ link, onClick, children }: Props) {
  if (link) {
    return <Link to={link} onClick={onClick} children={children} />;
  }

  return <div onClick={onClick} children={children} />;
}

Here it is not:


type Props = {
  unusedPropType: string; // Not triggered but supposed to be
  link?: string;
  onClick?: React.MouseEventHandler<HTMLDivElement | HTMLAnchorElement>;
  children?: React.ReactNode;
};

export function Tag({ link, onClick, children }: Props) {
  const props = {
    onClick,
    children,
  };

  if (link) {
    return <Link to={link} {...props} />;
  }

  return <div {...props} />;
}

image image

jwarby commented 1 year ago

Also seeing the same issue when spreading a subset of props onto an element. It makes sense that it wouldn't trigger the rule if it is the props object itself being spread, but I think the rule should be able to figure out what is actually being used in the scenario described by @ViggoV and in my code sample below?

const MyComponent = ({ foo }) => (
  <SomeOtherComponent {...{ foo }} />
);

MyComponent.propTypes = {
  foo: PropTypes.bool.isRequired,
  bar: PropTypes.bool.isRequired
};

From looking at the source, it seems like an ignoreUnusedPropTypesValidation flag will be set on any component that uses spread anywhere within, if I am not mistaken.