Skip to content
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

feat: disable no-unused-prop-types rule #59

Merged
merged 1 commit into from May 31, 2019

Conversation

matthargett
Copy link
Contributor

I sifted through the numerous false positive issues, and added a reduction of one of our cases that seemed to match:
jsx-eslint/eslint-plugin-react#1764

Summary

We ran into a few false positives with this rule, and it turns out there are over a dozen issues filed in eslint-plugin-react and a few unmerged PRs. Disabling it for now.

Test plan

I can add an example to the JS files that would fire if the rule is enabled and the false positive isn't fix yet. Let me know if that's necessary to merge and I'll add it.

Will no longer fire on this code (a reduction of a production case):

// @flow
import * as React from 'react';

type Props = {
  maxItems: number,
  itemOpacityOutputRange: Array<number>,
  textOpacityOutputRange: Array<number>,
  itemScaleOutputRange: Array<number>,
  children: Object => React.Node,
};

type State = {
  foo: number,
  bar: number,
};

function computeStateFromProps(props: Props) {
  const { maxItems, itemOpacityOutputRange, textOpacityOutputRange, itemScaleOutputRange } = props;

  if (maxItems && itemOpacityOutputRange && textOpacityOutputRange && itemScaleOutputRange) {
    return { foo: 0, bar: 0 };
  }
  return { foo: 1, bar: 0 };
}

export default class X extends React.PureComponent<Props, State> {
  constructor(props: Props) {
    super(props);
    this.state = computeStateFromProps(props);
  }

  animateItem = (index: ?number) => {
    const { itemScaleOutputRange } = this.props;
    if (itemScaleOutputRange && index) return 0;
    return 1;
  };

  render() {
    const {
      animateItem,
      props: { children },
    } = this;

    return <>{children({ animateItem })}</>;
  }
}

We ran into a few false positives with this rule, and it turns out there are over a dozen issues filed in eslint-plugin-react and a few unmerged PRs. Disabling it for now.

I sifted through the numerous false positive issues, and added a reduction of one of our cases that seemed to match:
jsx-eslint/eslint-plugin-react#1764
Copy link
Member

@thymikee thymikee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think example won't be necessary, we encounter such issues quite frequently as well, thanks for sending that :)

@thymikee thymikee changed the title Disable no-unused-prop-types rule feat: disable no-unused-prop-types rule May 31, 2019
@thymikee thymikee merged commit dfc722f into callstack:master May 31, 2019
@matthargett matthargett deleted the patch-1 branch May 31, 2019 21:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants