1
\$\begingroup\$

I have made a very basic login/registration form in React.js.

React.js login form

The inputted data is send to a Java Spring Boot Rest-API to create an user or to authenticate using JWT. But in this question I want to focus on the React.js code.

import React, { Component } from 'react'
import axios from "axios"
import Cookies from "js-cookie"

import "../styles/Home.sass"

class Home extends Component {
    
    constructor() {
        super()
        this.state = {
            message: {},
            inputs: {},
        }

        this.handleChange = this.handleChange.bind(this)
        this.Form = this.Form.bind(this)
    }

    handleChange = (event) => {
        let inputs = this.state.inputs
        inputs[event.target.name] = event.target.value
        this.setState({inputs})
    }

    render() {
        return (
            <div id="container">
                <div id="text">
                    <div>
                        <h1>Welcome to Finonline</h1>
                        <span id="slogan">Administrate your personal expenses online.</span>
                        <p>
                            This is a demo-project for my job-application showcasing my skills in full-stack webdevelopment, Java, Spring Boot and React.js.
                        </p>
                    </div>
                </div>
                <div id="form-container">
                    <this.Form />
                </div>
            </div>
        )
    }

    Form() {
        const handleSubmit = (event, action) => {
            event.preventDefault()
            
            switch (action) {
                case "register":
                    this.register(this.state.inputs)
                    break
                default:
                    this.login(this.state.inputs)
            }
        }

        return (
            <form onSubmit={(event) => handleSubmit(event, 'login')} >
                <span className="title">Login</span>
                <div className="input-wrapper">
                    <img src="/assets/images/profile.png" alt="" />
                    <input type="text" name="name" placeholder="Username" minLength="3" required="" onChange={this.handleChange} />
                </div>
                <div className="input-wrapper">
                    <img src="/assets/images/password.png" alt="" />
                    <input type="password" name="pass" placeholder="Password" minLength="5" required="" onChange={this.handleChange} />
                </div>
                <div id="form-buttons">
                    <input type="submit" name="login" value="Login" onClick={(event) => handleSubmit(event, 'login')} />
                    <input type="submit" name="register" value="Or register" id="registerbtn" onClick={(event) => handleSubmit(event, 'register')} />
                </div>
                <span id="message" className={this.state.message["type"]}>
                    { this.state.message["text"] }
                </span>
            </form>
        )
    }

    async register(data) {
        let message = this.state.message

        try {
            const response = await axios.post(`http://127.0.0.1:8080/users`,
                    data
                ,
                {
                    insecureHTTPParser: true,
                    headers: {
                        "Content-Type": "application/json",
                    }
                })

            if (response.status == 201) {
                message["type"] = "ok"
                message["text"] = "Successfully registered."
                
                this.setState({ message  })
            } else {
                message["type"] = "error"
                message["text"] = "Error: " + response.data
                
                this.setState({ message  })
            }
        } catch (err) {
            message["type"] = "error"
            message["text"] = "Error: This user already exists."
            
            this.setState({ message  })
        }
    }

    async login(data) {
        let message = this.state.message

        try {
            const response = await axios.post(`http://127.0.0.1:8080/auth`,
                    data
                ,
                {
                    insecureHTTPParser: true,
                    headers: {
                        "Content-Type": "application/json",
                    }
                })
            
            if (response.status == 200) {
                message["type"] = "ok"
                message["text"] = "Login successful."
                
                this.setState({ message  })
            } else {
                message["type"] = "error"
                message["text"] = "Error: Login failed."
                
                this.setState({ message  })
            }
        } catch (err) {
            message["type"] = "error"
            message["text"] = "Error: Login failed."
            
            this.setState({ message  })
        }
    }
}

export default Home

I have made a class extending Component. I have two states: message which holds the response message from the API and the type (ok/error), and inputs which holds the entered input from the user.

  • Is this clean React.js code?
  • Should the functions login and register be present in this class?
  • Is it okay to have a separate function for the form?
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

You can improve you cleanliness by removing some of your redundant patterns and building up robustness. You have a lot of

message["type"] = "ok"
message["text"] = "Login successful."

I'd probably pull that into a function. "type" may benefit from an enumeration with string values.

Your register and login both have very similar structures that can probably be abstracted.

I'd consider using response.ok https://developer.mozilla.org/en-US/docs/Web/API/Response/ok over a manual check of magic status code values. Otherwise, I might pull those 200, 201, etc into their own variables for readability.

handleSubmit has a one case switch. There's a number of ways you can do this, but I might consider an object with the functions since they have the same signature. This could naturally lead into a factory pattern later, e.g. sketching something (didn't check if it runs):

const actions = {
    "register": this.register,
    "login" : this.login
}

const actionToCall = actions[action];

if(actionToCall) {
    actionToCall(this.state.inputs)
} else {
    throw new Error("Bad action")
}

You could also just use an if/else, or if you're really wanting to use a switch, explicitly have register and login cases and the default throw an error.

Having login and register are fine, but you could pull out some classes that have a common interface like "ActionHandler" and use a true factory pattern to build them. For just two that might be overkill.

Form as it's own function is good and I would go further to pull it out to its own component. It's already operating at that leve, but you've got Form tightly coupled with the Home state. I'd have Form communicate to Home by passing state as a prop and a callback function which sets the state (this could just be passing in the setState function`)

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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