Skip to content

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

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

Open
realityking opened this issue Jan 28, 2018 · 5 comments

Comments

@realityking
Copy link

realityking commented Jan 28, 2018

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;
@realityking realityking changed the title react/prop-types doesn react/prop-types doesn't work when mixing createReactClass with ES6 style prop types Jan 28, 2018
@manjula-dube
Copy link

Hey anyone working on the issue so far?

@ljharb
Copy link
Member

ljharb commented Jan 29, 2018

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
Copy link

@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
Copy link
Member

ljharb commented Jan 31, 2018

@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
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

4 participants