11
\$\begingroup\$

I've been working on learning React, and so like most people do when they get started on something new. I created an arbitrary simple project to get a bit more familiar with the concepts.

It's so simple that it shouldn't really require any explanation. Here's the React code.

var TestInput = React.createClass({
    propTypes: {
        id: React.PropTypes.string.isRequired,
        label: React.PropTypes.string.isRequired,
        placeholder: React.PropTypes.string
    },
    getDefaultProps: function() {
        return {
            type: 'text'
        }
    },
    render: function() {

        var {label, id} = this.props;
        var other = _.omit(this.props, 'label');

        return (
            <div className="form-group">
                <label htmlFor={id}>{label}</label>
                <input className="form-control" {...other}/>
            </div>
        );
    }
});

var TestOutput = React.createClass({
    propTypes: {
        name: React.PropTypes.string.isRequired
    },
    render: function() {
        return (
            <h3>Hello {this.props.name}!</h3>
        );
    }
});

var TestBox = React.createClass({
    getInitialState: function() {
        return {
            firstName: '',
            lastName: ''
        }
    },
    handleInput: function(event) {
        var newState = {};
        newState[event.target.id] = event.target.value;
        this.setState(newState);
    },
    createName: function() {
        var nameParts = [];

        if (this.state.firstName.length == 0 && this.state.lastName.length != 0) {
            nameParts.push('Mr.');
        }
        if (this.state.firstName.length != 0) {
            nameParts.push(this.state.firstName);
        }
        if (this.state.lastName.length != 0) {
            nameParts.push(this.state.lastName);
        }
        if (nameParts.length == 0) {
            nameParts.push('Anonymous');
        }

        return nameParts.join(' ');
    },
    render: function() {
        return (
            <div className="test-box">
                <TestInput id="firstName" label="First Name" placeholder="John" onKeyUp={this.handleInput}/>
                <TestInput id="lastName" label="Last Name" placeholder="Smith" onKeyUp={this.handleInput}/>
                <TestOutput name={this.createName()} />
            </div>
        );
    }
});

React.render(
    <TestBox/>,
    document.getElementById('container')
);

It works, but it seems like I had to do way too much to get it to work. Not that it was difficult to write, but it just seems like a lot of data transfer and boilerplate to do very little. Maybe this is just because my example is simple and contrived? I'm just thinking that with jQuery I could write a single event handler which works on both input's keyUp and update the H3's text based on the current values.

What am I doing wrong? Or am I doing it right?

\$\endgroup\$
2
\$\begingroup\$

React has a small amount of boilerplate you need to repeat in order to keep your components well documented, thus the usage of propTypes which is one of great things to validate data.

There is nothing wrong with how your components are structured, as react "ways" were followed.

Only thing I can observe and which can be improved is to use strict comparisons, eg: === instead of == and so on. Why?

The identity operator returns true if the operands are strictly equal (see above) with no type conversion.

More information at MDN

As a bonus if your project uses Babel then you can use const and let for defining variables.

Also you could transform:

var {label, id} = this.props;
var other = _.omit(this.props, 'label');

To:

const {label, id, ...other} = this.props;

You can notice that we have eliminated the use of lodash/underscore.

You can find more about Babel at their homepage.

\$\endgroup\$
2
  • \$\begingroup\$ Welcome to Code Review, and nice answer! While the link about comparison operators is useful, it would be preferable to include the essential parts from it here, and provide the link for reference. The link may change or break, but if you quote the explanation here it will be preserved with your answer. \$\endgroup\$ Sep 17 '15 at 11:06
  • 1
    \$\begingroup\$ @SuperBiasedMan - thanks for the welcome and the tips, applied! \$\endgroup\$
    – gor181
    Sep 17 '15 at 11:51
2
\$\begingroup\$

2019 Answer - React 16+ compatible

TL;DR : Fully fonctional code :

class TestBox extends React.Component {
    constructor(props) {
        super(props)

        this.state = {
            firstName: '',
            lastName: ''
        }
    }

    handleInput = field => ev => {
        this.setState({ [field]: ev.target.value })
    }

    render(){
        const { firstName, lastName } = this.state

        return (
            <div className="test-box">
                {[
                    ['firstName', 'First Name : ', 'John'],
                    ['lastName', 'Last Name : ', 'Smith']
                ].map(([id, label, place]) => <TestInput id={id} label={label} placeholder={place} onChange={this.handleInput(id)} key={id} />)}
                <TestOutput first={firstName} last={lastName} />
            </div>
        )
    }
}

const TestInput = ({ id, label, onChange, placeholder }) => (
    <div className="form-group">
        <label htmlFor={id}>{label}</label>
        <input className="form-control" onChange={onChange} placeholder={placeholder} />
    </div>
)

const TestOutput = ({ first, last }) => <h3>Hello {[last ? first || 'Mr.' : first, last && ' ' + last].join('') || 'Anonymous'}</h3>
    
ReactDOM.render(<TestBox/>, document.getElementById('container'));
<script src="https://cdnjs.cloudflare.com/ajax/libs/react/16.6.0/umd/react.production.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/react-dom/16.6.0/umd/react-dom.production.min.js"></script>
<div id='container'>


React has evolved quite a lot since 2015, so here is my answer.
I will be using class (statefull) and function (stateless) components for this answer.

Let's take a look at your ouput component first.

It should print out Hello followed by the first and last name, which can be done with a single line component and an array containing the name props that we will join :

const TestOutput = ({ first, last }) => <h3>Hello {[first, last].join('')}</h3>

If both names are empty, the result should be Anonymous, for that we will use the || operator :

{[last: first].join('') || 'Anonymous'}

And now, display Mr. if there is a last name, but not the first :

[last ? first || 'Mr.' : first, last && ' ' + last]

Why isn't the space char always added to the last name ?

If the user does not put anything in the last name input, a space would be generated anyway, and Anonymous would never be shown since the generated string could not be empty.

Final output :

const TestOutput = ({ first, last }) => <h3>Hello {[last ? first || 'Mr.' : first, last && ' ' + last].join('') || 'Anonymous'}</h3>

The TestInput components will be quite similar to the one made in the question, but with an up-to-date syntaxing using a stateless component :

const TestInput = ({ id, label, onChange, placeholder }) => (
    <div className="form-group">
        <label htmlFor={id}>{label}</label>
        <input className="form-control" onChange={onChange} placeholder={placeholder} />
    </div>
)

And the last part, the TestBox. This component will still have the last and first name in its state and has to be a class (statefull) component :

class TestBox extends React.Component {
    constructor(props) {
        super(props)

        this.state = {
            firstName: '',
            lastName: ''
        }
    }

A single function will be responsible for getting both input fields change, and receive which field sent an update in its first value :

handleInput = field => ev => {
    this.setState({ [field]: ev.target.value })
}

The value in the state that will get updated depends on the first parameter set, before being fired by an event.

Now, the rendering.

Since both your inputs are fairly similar, I preferred mapping them over an array containing their differences, but this is up to your preferences, as it may decrease readability :

{[
    ['firstName', 'First Name', 'John'],
    ['lastName', 'Last Name', 'Smith']
].map(([id, label, place]) => <TestInput id={id} label={label} placeholder={place} onChange={this.handleInput(id)} key={id} />)}

Each input will have the given id, placeholder, label and preconfigured handleInput function.

The remaining rendering function will now look like the following :

render(){
    const { firstName, lastName } = this.state

    return (
        <div className="test-box">
            {[
                ['firstName', 'First Name', 'John'],
                ['lastName', 'Last Name', 'Smith']
            ].map(([id, label, place]) => <TestInput id={id} label={label} placeholder={place} onChange={this.handleInput(id)} key={id} />)}
            <TestOutput first={firstName} last={lastName} />
        </div>
    )
}

I dod not include it in my snippet but the new propType syntax would look like the following for TestInput :

TestInput.propTypes = {
    id: PropTypes.string.isRequired,
    label: PropTypes.string.isRequired,
    placeholder: PropTypes.string
}

More information about new propTypes

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.