Rule proposal: disallow referencing another component's propTypes

[lencioni]

Some people might want to use babel-plugin-transform-react-remove-prop-types to remove propTypes from their components in production builds, as an optimization. This transform changes code like:

const Baz = () => (
  <div />
);

Baz.propTypes = {
  foo: React.PropTypes.string
};

into:

const Baz = () => (
  <div />
);

This works, but might end up breaking things in production if you import a component and then reference that component's propTypes. Instead, the propTypes should be separately exported in this case and referenced directly from the export.

Bad:

import { pick } from 'lodash';

import SomeComponent from './SomeComponent';

export default function AnotherComponent(props) {
  const passedProps = pick(props, Object.keys(SomeComponent.propTypes));
  return (
    <SomeComponent {...passedProps} />
  );
};

Good:

import { pick } from 'lodash';

import { propTypes: someComponentPropTypes }, SomeComponent from './SomeComponent';

export default function AnotherComponent(props) {
  const passedProps = pick(props, Object.keys(someComponentPropTypes));
  return (
    <SomeComponent {...passedProps} />
  );
};

This rule would make the aforementioned Babel plugin safe to use.

[Jessidhia]

I think there should be a way to allow it in case you use the PropTypes of a given component as part of the PropTypes definition in a containing component.

This is a contrived example that doesn't really gain much of anything, but I think that in such constructs, it's better to validate the proptypes if you know what they're supposed to be than taking in PropTypes.any:

import React, { PropTypes } from 'react'
import Child from './Child'

export const Parent = ({ childProps }) => (
  <Child {...childProps}/>
)
Parent.propTypes = {
  childProps: PropTypes.shape(Child.propTypes).isRequired
}

[lencioni]

I think it is fine to allow this as long as we can reasonably detect this type of usage.

[lencioni]

@oliviertassinari @nkt any thoughts on this proposal or interest in picking it up? :)

[oliviertassinari]

@lencioni I have missed that proposal. That's a good idea!

You are definitely not the only one wondering. That's the biggest risk I'm aware of when using this babel transformation plugin.
For instance, @necolas opened an issue closely related to that topic two months ago: oliviertassinari/babel-plugin-transform-react-remove-prop-types#68

Regarding the implementation, I don't think that eslint is able to resolve imports.
I'm assuming we are going to need a heuristic. Do we have enough information to build a good one?

[lencioni]

I think referencing the propTypes property on any object would be good enough.

[lencioni]

So, it would error out on things like these:

Foo.propTypes;
const { propTypes } = Foo;
Foo['propTypes'];

There may be other patterns to look for. @ljharb can probably think of some.

[ljharb]

@oliviertassinari eslint-plugin-import is certainly able to resolve imports, but I don't think that's required here - just preventing using .propTypes and similar.

[hzoo]

If this is specific to babel-plugin-transform-react-remove-prop-types, we could just error from the plugin itself instead of the eslint rule (just depends on whether we think its better to make it independent of not)

[ljharb]

The goal is to be able to strip propTypes but ensure that doing so won't break any code.

I don't have any intuition about where the best place for the error is, but having it in eslint might let it be caught earlier.