ReactJS Application - resiliency VS failing fast

Question

I'm in the middle of the development for a React application and this is the approach I used for my components: I validate the props that I expect to be received using the PropTypes validation but I still assign default values in order to avoid that it breaks if something goes wrong with the received data.

Recently I've been told that we should not do that, that the props are what we expect from the parent and if the contract is not respected to let the component break.

Which approach is correct and what are the pros and cons?

Some of my considerations as food for thought..

Following my initial approach, in the tests I explicitly test default values passing to the component under test some invalid data and expecting a valid snapshot to be still printed out. The tests don't fail due to some bad data but I print out the PropTypes validation warnings (that could though be transformed in errors if wanted - I think - or be silenced mocking them out in the test).

These warnings both in the tests and in the real application are more concise and clear than just seeing an error saying "cannot read 'someProp' from undefined" or similar (and letting React render cycle break). The propType validations directly and clearly tell you what you did wrong (you passed in the wrong type as prop, the prop was missing completely, etc).

Using the second approach instead the tests fail because the app breaks. I think that this is a good approach only if the test coverage is really good (90/100%) otherwise it's a risk - it could go live and break on edge cases ruining the product reputation. Refactoring or requirements changes happen quite often and some edge cases could end up with undesired data that break the application and was not captured in automated or manual tests.

This means that when the application is live the code could break in a parent component due to some bad data and the whole application stop working, where instead in the first case the app is resilient and simply displays some empty fields in a controlled way.

Thoughts?

Follows a simplified example:

React component

import React from 'react';
import PropTypes from 'prop-types';
import styles from './styles.css';

export const App = ({ person : { name, surname, address, subscription } = {} }) => (
  <div style={styles.person}>
    <p> {person.name} </p>
    <p> {person.surname} </p>
    <p> {person.address} </p>
    <div>
    {
      person.subscription &&
       <Subscription details={person.subscription} />
    }
    </div>
  </div>
);

// PS. this is incorrect in this example (as pointed out in an answer). Real code used inline initialization.
// App.defaultProps = { 
//  person: { subscription: undefined },
// };

App.propTypes = {
  person: PropTypes.shape({
    name: PropTypes.string.isRequired,
    surname: PropTypes.string.isRequired,
    address: PropTypes.string,
    subscription: PropTypes.object,
  }).isRequired,
};

Test

import React from 'react';
import { shallow } from 'enzyme';
import { mockOut } from 'testUtils/mockOut';

import { App } from '../index.js';

describe('<App>', () => {
  mockout(App, 'Subscription');

  it('renders correctly', () => {
    const testData = {
      name: 'a name',
      surname: 'a surname',
      address: '1232 Boulevard Street, NY',
      subscription: { some: 'data' },
    }
    const tree = shallow(<App person={testData} />);
    expect(tree.html()).toMatchSnapshot();
  });

  it('is resilient in case of bad data - still generates PropTypes validation logs', () => {
    const tree = shallow(<App person={undefined} />);
    expect(tree.html()).toMatchSnapshot();
  });
});

UPDATE:

Main focus of the question is on whether it is correct or not to assign default values to props that are marked with isRequired (instead of letting their absence break the component)

Answer 1

Recently I've been told that we should not do that, that the props are what we expect from the parent and if the contract is not respected to let the component break.

Exactly, if a props in component is optional, component(who renders the actual view) should handle that, not the parent component.

However, you can have a situation where the parent should break if any of the child components contract is breaking. I can think of two possible way to handle this situation-

1. Passing error notifier to child components, where if anything goes wrong child can report error to parent component. But this is not clean solution because if there are N child and if more than one will break(or report error) to parent, you will have no clue and will be hard to manage.[This is not effective at all but wrote here because I used to follow this when I was learning React :P]

2. Using try/catch in parent component and not trust any child component blindly and show error messages when something goes wrong. When you are using try/catch in your all component, you can safely throw error from the components when any contract is not fulfilled.

Which approach is correct and what are the pros and cons?

IMO, the second approach(try/catch in components and throwing error when requirements are not fulfilled)is valid and will solve all the issues. While writing tests for component when props are not passed you can expect an error when loading the component.

Update

If you are using React > 16, here is the way to handle errors.

Answer 2

In my opinion, I will not let one or two missing attribute to break my application. React acts as presentation layer in my app and I think it is way to far that showing "Oops! There is something wrong" when I can not find a key in one object. It seems like a message from a badly-broken server with 500 status, but we know it is definitely not that wrong.

For me, I do build some rules to handle communication between render function and defaultProps:

Let's say we have an user object passed from parent:

defaultProps: {
  user: {
    avatar: {
      small: ''
    }
  }
}

in the render function

render() {
  const { user } = this.props;
  // if user.avatar is not defined or user.avatar.small is empty string or undefined then we render another component we have prepared for this situation.
  if (!user.avatar || !user.avatar.small) {
    return (
      // components
      ...
    );
  }
  // normal situation
  return (
    // components
    ...
  );
}

The example above is for string and we need different implement for other data types.

Good luck.

Answer 3

It is not correct to assign default values to .isRequred props via component defaultProps. According to the official docs:

The defaultProps will be used to ensure that this.props.name will have a value if it was not specified by the parent component. The propTypes typechecking happens after defaultProps are resolved, so typechecking will also apply to the defaultProps.

If you set default property value in Component.defaultProps, then you will never get a warning if this prop isn't provided by the parent component.