Add Proptypes for wrapped and wrapper components

[saulshanabrook]

It would be nice to be able to add propTypes to both the inner and outer components returned by the decorator.

[christianalfoni]

Hm... do not think it is common to allow that, any particular reason? :-)

[saulshanabrook]

For example, I have a component called Systems that other components should pass a systemsPath prop when creating. They are passing it to the cerebral wrapper, so Systems Container. Then the actual Systems component gets a couple of props passed in that cerebral feeds it, but doesn't see the systemsPath prop.

@Cerebral(props => ({
  systems: props.systemsPath,
  newSystem: getNewSystemPath(props.systemsPath)
}))
class Systems extends Component {
  [...]
}

// here i want to define the propTypes on the wrapped system
Systems.propTypes = {
  systemsPath: React.PropTypes.array.isRequired
}
// but i would also like to set the propTypes on the inner System to this:
// {
//  systems: React.PropTypes.arrayOf(React.PropTypes.object).isRequired,
//  newSystem: React.PropTypes.object
//}

export default Systems

I want to make sure that:

1. the component is being called by other components with the right prop types.
2. cerebral is getting the right types from the state to pass into the

Does that makes sense?

[christianalfoni]

Hm hm, first thanks for explaining it so thoroughly :-)

I do not quite understand why it is not good enough to do:

Systems.propTypes = {
  systems: React.PropTypes.arrayOf(React.PropTypes.object).isRequired,
  newSystem: React.PropTypes.object
}

Since it is just passing the props straight through I can not quite see it is relevant to "double check"?

[saulshanabrook]

But then how do I set the prop types for the wrapped component? Like where do I set these prop types

{
  systemsPath: React.PropTypes.array.isRequired
}

[christianalfoni]

Ah, now I understand why you mean, sorry.

Are these propTypes checked at runtime? If so Cerebral-React should throw an error when an invalid path is used. So this would be handled automatically. Would that make sense? It feels a bit overkill to make an API change to handle this, as you can only use arrays there.

[saulshanabrook]

Yeah propTypes are checked at runtime. Hm, what do you mean by "invalid path"? The props don't have to be path's, that is just an example, they could be any type/value.

For example...

@Cerebral(props => ({
  systems: ['systems', props.systemID],
}))
class System extends Component {
  [...]
}

// here i want to define the propTypes on the component that I call create elsewhere in my code
System.propTypes = {
  systemID: React.PropTypes.number.isRequired
}
// but i would also like to set the propTypes on the inner component, that cerebral initializes, to this:
// {
//  systems: React.PropTypes.arrayOf(React.PropTypes.object).isRequired,
//}

export default System

Does that makes sense as an example?

As a solution, maybe we could pass the propTypes in to the decorator?

@Cerebral(props => ({
  systems: ['systems', props.systemID],
}), {
  systems: React.PropTypes.arrayOf(React.PropTypes.object).isRequired,
})
class System extends Component {
  [...]
}

System.propTypes = {
  systemID: React.PropTypes.number.isRequired
}

export default System

[tnrich]

@saulshanabrook, I think that your proposal makes sense. Would it be hard to implement that strategy @christianalfoni ?

[tnrich]

@saulshanabrook This seems like it would be a more general issue with using decorators with React. Maybe others in the community have a solution for it?

[tnrich]

hey @saulshanabrook and @christianalfoni , tested it out and it looks like using:
https://github.com/popkirby/react-props-decorators
will work just fine!

An example is:

@Cerebral({
    sequenceLength: ['sequenceLength'],
    bpsPerRow: ['bpsPerRow'],
})
@propTypes({
    sequenceLength: PropTypes.number.isRequired,
    bpsPerRow: PropTypes.number.isRequired,
})
class SequenceEditor extends React.Component {
...

This seems like a good general solution that doesn't require cerebral to add anything specific. It might be nice for others to know about it though if you wanted to put something in the docs about it.

Also I forked it and updated the dependencies and made a pull request so hopefully that'll get updated soon.

[saulshanabrook]

@tnrich Wow that looks great! I think there should be a section in the docs about how to do this with react. @christianalfoni I would be happy to have a go at it, if you think that makes sense.

[christianalfoni]

@saulshanabrook Yeah, please add it to the docs, that would be great :-)