What I did Wrong with my first React Component

I recently refactored the first React component I wrote. My code was something like this (It wasn’t really a Signup screen, but there’s a similarity in the workflow):

const db = require('./db.js')
const SignupScreen = React.createClass({
    getInitialState: function(){
        return { User: this.props.User, message: '' }
    },
    render: function(){
        const User = this.state.User
        const message = this.state.message
        return (<div>
            Username: ...<br> 
            Password: ...<br>
            <input type="submit" 
                   value="Signup" 
                   onclick={this.handleSignup}
            />
            <p>{message}</p>
        </div>)
    },
    handleSignup: function(event){
        const User = this.state.User
        this.checkUsernameIsUnique(User.getUserName())
            .then(isUnique => {
                if(isUnique){
                    this.saveUser()
                    .then(response => 
                        this.setState({message: 'Account Created'})
                    )
                    .catch(error => 
                        this.setState({message: 'Could not save Account, please try later'})
                    )
                } else {
                    this.setState({message: 'Username taken'})        
                }
            })
    },
    checkUsernameIsUnique: function(){ 
        return db.find('user', this.state.User.toPojo())
                .then(user => user? true : false)
    },
    saveUser: function(){
        return db.save('user', this.state.User.toPojo())
    }
})

So this is pretty bad, but also quite similar to the example code in the React docs, so possibly other people have written stuff like this too.

Things to note:

• Application logic tightly coupled with presentation in a single object - so I had to test my application logic through the clunky medium of React’s TestUtils and a fake DOM.
• The User prop is an object with custom getters, setters, etc, adding complexity to the code, and tedium to unit-test setup. When I changed User's constructor signature a bit, the setup in my tests broke, even though what they were testing didn’t.
• Implicit dependencies from require('./db.js') make it awkward to test. Sure, you can mock them with rewire, but if you have to magically rewrite your code just for the tests, it’s hard to pretend you wrote test first like a proper programmer.
• Application logic is split across subsidiary methods, just so the unit tests can stub them (to return fake results instead of triggering real DB operations). This makes the tests depend on the underlying implementation, and what did they even really test? When I refactored the component internals, the tests broke. Tests shouldn’t need to know how the code works, only what it is supposed to do.

The refactor:

//signup.jsx
const SignupScreen = React.createClass({
    getInitialState: function(){
        return { User: this.props.User, message: '' }
    },
    render: function(){
        const User = this.state.User
        const message = this.state.message
        const {handleSignup} = this.props
        const setMessage = message => this.setState({message})
        return (<div>
            Username: ...<br> //updates User.username
            Password: ...<br> //updates User.password
            <input type="submit" value="Signup" onclick="{(event)=">{
                handleSignup(setMessage, User)
            }}/>
            <p>{message}</p>
        </div>)
    })
//signup-actions.js
export const handleSignup = (findUser, saveUser) => 
    (setMessage, user) => {
    findUser(user.username)
    .then(dbUser => {
        return dbUser? 
            setMessage("Username taken")
            : saveUser(user)
                .then(_ => setMessage("Account created"))

    }).catch(e => setMessage("Couldn't create account "))
}


//signup-page.js
import SignupScreen from './signup.jsx'
import {handleSignup} from './signup-actions.js'
import db from './db.js'
const User = {username: '', password: ''}
const saveUser = user => db.save('users', user)
const findUser = username => db.find('users', {username})
ReactDOM.render((<SignupScreen user={User} handlesignup={handleSignup(findUser, saveuser)}
                , document.getElementById('container'))

There is probably still a lot to improve upon here, but there are at least some improvements:

• Instead of passing a (potentially complex) User object to SignupScreen, we just pass in plain object literal.
• Application logic is opaque to the view (SignupScreen). All it knows to do is create a button that calls a function I’ve passed in.
• The key logic is in handleSignup, which has no implicit dependencies, and simple arguments, so is easy to test.
• SignupScreen has simple arguments, no implicit dependencies (except React), and is easy to instantiate in different environments (eg: tests).

So my conclusions are:

1. Think of the React component as a function that takes data and maybe some event handler functions, and returns a DOM Node. This (to me) is a much simpler mental model than thinking of it as an Object with a mix of custom and inherited methods, maintaining internal state, interacting with parent and children objects, etc. Hate that shit.
2. Keep logic out of your View (the React component). Instead, pass in functions to the component, and have the event-handlers call them. Keeping the view ‘dumb’. Separate the concerns. Everyone will love you and all your wildest dreams will come true.
3. Keep implicit dependencies out of your core application logic. Pass in Effectful operations as parameters instead of putting them in-line amidst the logic. Pass in “environment” and “config” - style data structures instead of importing/requireing them. Tests become clearer, less brittle, and much easier to write (which means they actually get written!).
4. Keep the arguments to functions simple. If a function takes an object with a bunch of methods, you’ll have to instantiate it and manipulate it in your tests’ setup, and the function’ll be brittle to API changes to the object’s class. You’ll be miserable now and later. Instead, simplify the inputs to Numbers, Strings, Lists, key:value Objects, and Functions. If you need a method to be called, wrap up that method call in a function, and pass in that function (see saveUser above).
5. Test the views, even though they’re dumb. There might not seem much to test any more, but I found it was still useful to at least instantiate the view components as part of the automated test suite. It let me know when I broke something in a subsequent refactor.