Add isInjected to highlight the fact that some props are injected by decorators

[ignatiusreza]

PropTypes are good in defining what kind of props a component expect to receive, but it doesn't help in communicating what props need to be given when consuming a component, especially when working with a decorated or connected component.

The following example shows how the modifier is meant to be used in es6 class. The modifier should also be usable anywhere PropTypes can be used.

class MyComponent extends Component {
  static propTypes = {
    foo: PropTypes.string,
    intl: intlShape.isInjected,
    location: PropTypes.shape.isInjected,
  }
}

export default withRouter(injectIntl(TodoList))

In the example given above, if we don't mark intl and location as isInjected, consumer of the component might think that they will be able to pass in these two props, even though they actually are meant to be injected by react-intl and react-router respectively. This confusion grows with the number of injected props, as in the case when e.g. using connect from react-redux to inject some state and dispatch..

Arguably, we can solve this just by not listing injected props in the propTypes.. Unfortunately though, it won't go well if we also have eslint for react/prop-types, since linter will complain if we're using some props that are not defined in propTypes..

Alternatively, we can also designate a different place to define those injected props, e.g. by having static injectedPropTypes.. but, this approach will potentially require changes on react sides..

[ljharb]

It's the job of injectIntl to hoist .propTypes, and remove the props that it's provided - the consumer should be able to look at the export's .propTypes and see that intl is missing - in no way should the consumer be looking at the original .propTypes in the code and relying on that.

[ignatiusreza]

That's correct if we're talking about runtime check done by code.. but, the purpose of this new modifier is to help human in understanding what can (should) be passed in to any components..

If we rely on decorators having to properly hoist and cleanup the propTypes, then for human to understand it, we need to first run the code, and then inspect the component using something like the developer console.. it doesn't sounds like a good use of time from the PoV of developer productivity..

[ljharb]

in that case, I’d suggest a comment - if it doesn’t need to be runtime, then it doesn’t need to be a runtime marker in prop-types either.

[ignatiusreza]

yes, I thought about that also, and it is indeed the simpler solution.. but, then I thought that having it as built-in support in prop-types could help in developing a standard in the community so that we can potentially rely on it not just in our own code base, but also when looking into packages or when helping in open source projects..

it also open up the possibility for static analyzer, e.g. to parse and generate list of components inside a project with its props shape.. while it can also be achieved using comments, having it in code comes with a free guard for typo, since if the props are not defined correctly, an error will be raised at runtime..

[ljharb]

I think the standard the community follows is runtime detection. If you want to know what a component takes as props, open up a repl, require it, and check its .propTypes. Nothing’s more accurate than that.

[ignatiusreza]

hmmm... i'm not so sure about that.. as evidently found in:

• connect should hoist wrapped component's propTypes if they exist reduxjs/react-redux#672
• The component passed to injectIntl should return a component with the same propTypes formatjs/formatjs#509
• withRouter HOC does not provide propTypes from wrapped component remix-run/react-router#4868

where the idea for hoisting propTypes by decorator are rejected vs

• expose .propTypes and .defaultProps on the wrapper airbnb/react-with-styles#38

where it is accepted.. so there are still no consensus in the community.. and if decorators writer refuse to hoist propTypes, the responsibility to write it correctly and informatively falls once again back to the component being wrapped..

[ljharb]

There’s nothing stopping you from manually hoisting the propTypes if the HOC author refuses to do so.

[ljharb]

I think an .isInjected feature in prop-types itself wouldn't actually solve the problem - it'd have to be in react itself, to avoid warning on missing required injected props, and as such, it's out of scope to solve it only here.

The real solution here, as indicated above, is to make the HOC properly hoist propTypes, and subtract out the props it injects.

Thanks for the PR!