Eslint-plugin-react: How can I handle react-router and redux with react/forbid-prop-types?

[casr]

Previously, I just had a propTypes on my root component that would check that that history and store were defined with React.PropTypes.object. I have recently upgraded to the latest AirBnB style guide and in all my efforts looking around I have not come across how to make this code better.

How have others solved this problem?

% npm run lint
./containers/root.dev.jsx
  17:3  error  Prop type `object` is forbidden  react/forbid-prop-types
  18:3  error  Prop type `object` is forbidden  react/forbid-prop-types

./containers/root.jsx:

import React, { PropTypes } from 'react';
import { Provider } from 'react-redux';
import { Router } from 'react-router';
import routes from '../routes';
import DevTools from './dev-tools';

const Root = ({ store, history }) => (
  <Provider store={store}>
    <div>
      <Router history={history} routes={routes} />
      <DevTools />
    </div>
  </Provider>
);

Root.propTypes = {
  history: PropTypes.object,
  store: PropTypes.object,
};

export default Root;

./index.jsx:

import React from 'react';
import { render } from 'react-dom';
import { browserHistory } from 'react-router';
import { syncHistoryWithStore } from 'react-router-redux';

import configureStore from './configure-store';
import Root from './containers/root';

const store = configureStore();
const history = syncHistoryWithStore(browserHistory, store);

render(
  <Root history={history} store={store} />,
  global.document.getElementById('root')
);

[tmcintire]

I'm not 100% sure but from what I have read they want you to actually define the object using the
history: React.PropTypes.shape({ foo: { bar: {} } });
instead of just calling React.PropTypes.object.

While its not wrong, what they want is more description in your proptypes

[casr]

If I use a shape then I should also define the members of the object? But in this instance, as none of the members are used, I get a react/no-unused-prop-types instead. I could just use React.PropTypes.shape without defining the members but then it doesn't seem like I have gained much from it all.

For now I've disabled the rule but I just wondered if there was anything I was missing:

Root.propTypes = {
  history: PropTypes.object, // eslint-disable-line react/forbid-prop-types
  store: PropTypes.object, // eslint-disable-line react/forbid-prop-types
};

[tmcintire]

Ah. In that case you don't even need to define prop types. They are only there as an optional item to describe the props that need to be used in the component. It makes it easier for someone else to look at it and know which props are used in that component and what type of prop it is. But since you aren't using using any you will get the no-unused-proptypes error.

[ljharb]

You should always define propTypes.

[tmcintire]

I agree. However he is not using any props in the components he posted.

[casr]

I am making use of store by the <Provider /> component and history by the <Router /> component. Or did you mean something else?

I thought what I was doing was a common pattern but I'm happy to refactor it if there is a better way of arranging it.

[tmcintire]

Those are passed in variables from other files. Not actual props. Props are props.foo or this.props.foo

[casr]

I am cheekily unwrapping it when I pass in the parameter. Normally, this line:

const Root = ({ store, history }) => (

would look like:

const Root = (props) => (

Sorry if that made it difficult to read!

[ljharb]

@tmcintire destructuring props in an SFC signature is completely fine, are indeed props, and should be covered here.

[nickjanssen]

If I'm using a third-party library with objects like store, history or navigator, how am I supposed to know what that object will look like? Do I really have to dive into the redux code, find out what the store object looks like, and then use a shape object as a propType that I have to maintain myself?

[ljharb]

@nickjanssen there are two answers. the easy/unideal one is to just use eslint ignore comments inline in files that use those props.

The proper solution is to file an issue on the third party library to export a proper propType for each of the props they require (even if that's PropTypes.object) so that your code doesn't need to be concerned with the question of "validating a structure that your code doesn't provide".

[jtiscione]

@nickjanssen I have the same problem- I have 30 different types of objects all shaped differently- do I really have to write a couple hundred lines of shape() calls to make this stupid thing happy?
The only option is disabling this idiotic rule in .eslintrc.

[ljharb]

@jtiscione if you have 30 shapes, then you definitely need 30 shapes if you want them properly documented and checked. Although you’re free to disable it, the rule isn’t idiotic; but having a large quantity of unchecked data structures might be.

[jtiscione]

@ljharb The problem with this react/forbid-prop-types rule is that it's assuming that React is always the appropriate layer to be doing data validation on all objects, verifying shapes and leaf node types.

I don't need to manipulate these things in the React layer at all. These are document objects with unpredictable structures and field names that change every day. They come from another server and I pass them straight through to a separate library that validates the data itself and reports errors properly before passing it along to a REST API.

It seems I would need to replace PropTypes.object with a documentation of the entire third-party REST API- in the form of a single line with several hundred PropTypes calls and thousands of parentheses- which would need to be rewritten with every API revision that comes down the pike. Using PropTypes.string and JSON.parse() would be safer than that.

[ljharb]

It wouldn’t be safer; writing that every time is indeed the safest approach. Your frustration is about convenience - inconvenience which is forced on you by these sloppy third party APIs that are providing unpredictable structures.

React is the right layer to be doing these validations because every layer should be doing them - at no point should any of your code trust its input implicitly.

[jtiscione]

Yeah, but that's just like, your opinion man... :)

Validation at every layer that the data passes through leads to coupling which is unnecessary in layers that don't interact with the data. I don't need my ISP validating data structures (although Ajit Pai would allow it), and sometimes it's inappropriate for React to validate them for similar reasons. Every API change would entail running around to a bunch of React components and convincing them not to throw fits at runtime over data structures that they don't actually manipulate. It's like having a rookie cop holding up airport security every time he encounters a brand of shoes he's never seen before.

And a lot of document structures have unpredictable forms that are completely determined by the end-user. How would I use PropTypes to verify the shape of a tree of SVG nodes that someone threw at me? Do I convert it to a weird object with arrays of magic strings and internal references that conforms to a "DTD" written in the form of PropTypes calls?

I mean, an ESLint rule isn't a big showstopper or anything, but trying to satisfy this one on certain projects... yikes! It would be nice if ESLint actually examined the component's logic for manipulation of fields on opaque props objects, but I guess it would be a pain to implement.

[mcyalcin]

You realize that you can use just shape({}) (essentially same thing as object) if you don't care about the contents of the object. Linter won't report extra fields. You only need to fill in the fields if you are interacting with them (where eslint would catch as 'missing in props validation'), which means they do need to be validated.

[lokori]

Well, apparently I have to do this in every React Component to satisfy the linter if I want to use
history.push to programmatically navigate.

history: PropTypes.shape({ push: PropTypes.func.isRequired }).isRequired

This will satisfy the validator thingy that there is something like _"history"_ which contains a function _"push"_ and this is required to use the component. Well, of course there could be something like "shape of history" somewhere I could use instead of copy-pasting this.

(I might be missing something obvious because this is my first time using React or Redux or these linter rules, but this seems rather annoying.)

[ljharb]

@lokori yes, that's right - and ideally you should have a single module that exports this shape so that you can reuse it. Even more ideally, react-router should be exposing this exact propType - I'd suggest filing an issue with them to request it.

[russellr922]

PropTypes.objectOf(PropTypes.any).isRequired is what I use for the history prop provided by react router to pass linting. I don't see the sense in validating the shape of a prop provided by a library used by 99.99% of React applications.

[ljharb]

@russellr86 ideally react-router would export a propType for the history prop, so you don't have to do that.

Either way, the amount of usage of a given library (which is used in way less than 99% of all react applications, btw) has no correlation to the likelihood that you're going to use it improperly.

[russellr922]

Fair point. They don't at the moment though, I would rather use my solution for now than maintaining it in my project(s) and hoping that new updates to the library don't break it.

[txwkx]

I might be off-topic, however, there is a npm package react-router-prop-types that takes away the hustle of defining the nested shape structures to specify RR-related props.

import ReactRouterPropTypes from 'react-router-prop-types';  // (gzipped: 1.2K)
...
static propTypes = {
    match: ReactRouterPropTypes.match.isRequired,
    history: ReactRouterPropTypes.history.isRequired,
 };