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

prop-types Validating external propTypes (eg. decorators) #322

Open
danielkcz opened this issue Nov 19, 2015 · 13 comments
Open

prop-types Validating external propTypes (eg. decorators) #322

danielkcz opened this issue Nov 19, 2015 · 13 comments

Comments

@danielkcz
Copy link

This is something that comes from loggur/react-redux-provide#1 as propTypes are used to actually define data that will be provided to the component like this:

@provide({
  list: PropTypes.arrayOf(PropTypes.object).isRequired,
  pushItem: PropTypes.func.isRequired
})
export default class GoodTimes extends Component {
}

That doesn't coop well with this rule unfortunately. I am wondering if there is a way how to analyze this and read propTypes from higher-order component / decorator also.

I have noticed some undocumented customValidators, but I am not sure how does that works. If I understand correctly it accepts list of props that will be ignored?

@IanVS
Copy link

IanVS commented Nov 20, 2015

I am also wondering the same thing, if there's a good way to avoid adding them all manually as exceptions. In my case, I'm using a formsy-react decorator, and don't want to get linting errors on the methods it provides on this.props.

@wuct
Copy link

wuct commented Dec 17, 2015

I also run into this issue when I am using acdlite/recompose. It turns out that moving propTypes up to a HOC / decorator is not only a sugar but also necessary. For example, when a component is wrapped in a HOC, one can not get its propTypes from outside unless its propTypes is declared in the outermost HOC.

// set propTypes in the component
@pure
class Foo extends Component {
    static propTypes = {}
}
expect(Foo.propTypes).to.be.an('undefined');

// set propTypes in the outermost HOC
@setPropTypes({})
@pure
class Bar extends Component {}
expect(Bar.propTypes).to.be.an('object');

@yannickcr
Copy link
Member

I'll be happy to fix the rule for this but I've never used react-redux-provide, formsy-react or recompose.

Is there any common pattern on which we could rely?

@raveclassic
Copy link

@yannickcr
Here's basic example for using bem modifiers:

//bem.js
export function BEM(blockName) {
    if (typeof blockName !== 'string') {
        throw new Error(ERROR_DECORATOR_BLOCKNAME_VALIDATION);
    }

    return function(target) {
        if (typeof target !== 'function') {
            throw new Error(ERROR_DECORATOR_TARGET_VALIDATION);
        }

        if (target.prototype instanceof React.Component) {
            if (!target.propTypes || target.propTypes && !target.propTypes.modifiers) {
                target.propTypes = Object.assign({}, target.propTypes, {
                    modifiers: React.PropTypes.arrayOf(
                        React.PropTypes.string
                    )
                });
            }
            if (!target.defaultProps || target.defaultProps && !target.defaultProps.modifiers) {
                target.defaultProps = Object.assign({}, target.defaultProps, {
                    modifiers: []
                });
            }
        }

        target.prototype.bem = bem.bind(null, blockName);
        return target;
    };
}
//table.jsx
const CN_TABLE = 'table';

@BEM(CN_TABLE)
export default class Table extends React.Component {
    render() {
        return (
            <table className={this.bem(this.props.modifiers)}>
                <TableCol/>
                <TableHead/>
            </table>
        );
    }
}

@jochienabuurs
Copy link

any progress on this topic?

@arronmabrey
Copy link

@yannickcr and all I added a simple test case that exposes an issue I came across while working with https://github.com/acdlite/recompose

#762

Hopefully this can help bring attention to this issue.

@monsieurnebo
Copy link

We need this !

@yvele
Copy link

yvele commented Feb 14, 2017

PS: I'm right now using the ignore option of react/prop-types as a workarround for my decorated proptypes

@giuseppepaul
Copy link

@yvele I'm trying to do this but struggling to get it working, could you show me what your .eslintrc looks like?

@yvele
Copy link

yvele commented Feb 17, 2017

@giuseppepaul Sure!

"rules": {
   "react/prop-types" : ["error", { "ignore": ["foo", "bar] }]
}

@SampsonCrowley
Copy link

Just to leave another option, I export the proptypes for a decorator then destructure them into the connected class.

Decorator

...
...

export const authPropTypes = {
  authorize: PropTypes.func,
  isLoggedIn: PropTypes.bool,
  login: PropTypes.func,
  logout: PropTypes.func,
  userProfile: PropTypes.object
};

export default function withAuth(ComposedComponent) {
  ...
}

Decorated:

...
import withAuth, {authPropTypes} from 'decorators/with-auth';

@withAuth
class Header extends Component {
  static propTypes = {
    checked: PropTypes.bool,
    toggleMenu: PropTypes.func,
    ...authPropTypes
  }
...
}

@damianobarbati
Copy link

Any progress on this? It's so annoying to duplicate all the component proptypes around the codebase or importing/spreading them all over. 😔

@pbassut
Copy link

pbassut commented Apr 23, 2018

Would be great to have this.

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

No branches or pull requests