jeudi 5 octobre 2017

Resilience of React application Vs letting it break

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 will break 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..

In my case, in the tests, I explicitly test default values passing down 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). These warnings both in the test and in the real application are more concise and clear than just seeing an error saying "cannot read 'someProp' from undefined" or similar. 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).

In the second case instead the test fails because the app breaks. I think that this is a good approach only if the test coverage is good (70/90%) otherwise it's a risk. Refactoring or requirements changes can happen 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 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 }) => (
  <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>
);

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 data - still generates PropTypes validation logs', () => {
    const tree = shallow(<App person={undefined} />);
    expect(tree.html()).toMatchSnapshot();
  });
});

Aucun commentaire:

Enregistrer un commentaire