jsx-eslint / eslint-plugin-react

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

react/prop-types doesn't work when mixing createReactClass with ES6 style prop types #1663

Open realityking opened 6 years ago

realityking commented 6 years ago

To facilitate moving to ES6 classes we're mixing createReactClass with ES6 style prop types.

react/prop-types doesn't seem to recognize prop types in this case and reports all used props as "missing in props validation".

File to reproduce:

'use strict';

const createReactClass = require('create-react-class');
const React = require('react');
const PropTypes = require('prop-types');

const Component = createReactClass({
  render: function () {
    return (<div>{this.props.prop}</div>);
  }
});

Component.propTypes = {
  prop: PropTypes.string
};

module.exports = Component;
manjula-dube commented 6 years ago

Hey anyone working on the issue so far?

ljharb commented 6 years ago

I’m confused; does React even work with this?

At any rate, React themselves provides a codemod that can convert all your non-mixin-using createClasses to class-based components; is there a reason not to use that?

abenitesDC commented 6 years ago

@ljharb I believe what you are pointing is a complete separate discussion from the actual problem reported. There are many teams that for various reasons are still using React without ES6 support. Unfortunately, for big projects, migrating code is not always as straight-forward as running a codemod.

And to answer your first question, this indeed works in React. (check: https://reactjs.org/docs/react-without-es6.html)

ljharb commented 6 years ago

@abenitesDC fair. Those linked docs don't mention PropTypes, but I verified manually that indeed, it works in either form.

We should indeed modify the prop-types rule to detect static PropTypes with createClass components as well.

realityking commented 6 years ago

Sorry for not getting back to this earlier. React indeed supports mixing these two.

We're unlikely to switch to ES6 classes until Node.js natively supports class fields. I'd however like to eliminate the amount of changes needed when we do, thus this usage.