2
\$\begingroup\$

I'm looking for a review of a recent project I finished for school. It is an app that functions like Twitter. It includes an Express server and Routes. Instead of using databases, it uses JSON files. Users can create, edit, and delete 'chirps' (tweets) with fetch requests. Specific instructions for the project are included on the README.

Please leave any feedback from small mistakes/improvements to big picture stuff like structure and organization. Thank you

My app: https://github.com/ChristyCakes/CovalenceReactBoilerPlate

app.jsx

import React, { Component, Fragment } from 'react';
import { BrowserRouter as Router, Route, Switch } from 'react-router-dom';
import { Home, Chirp, Edit, Delete } from './components';
import './styles.css';

class App extends Component {

    render() {
        return (
            <Router>
                <Fragment>
                    <Switch>
                        <Route exact path="/" component={Home} />
                        <Route path="/:id/edit" component={Edit} />
                        <Route path="/:id/delete" component={Delete} />
                        <Route path="/:id" component={Chirp} />
                    </Switch>
                </Fragment>
            </Router>
        )
    }
}

export default App;

home.jsx

import React, { Component } from 'react';
import 'isomorphic-fetch';
import Current from './current'
import logo from './logo.png'
import NewChirp from './newChirp'

class Home extends Component {
    constructor() {
        super();
        this.state = { chirps: {} }
    }

    componentDidMount() {
        fetch('http://127.0.0.1:3000/api/chirps')
            .then(response => response.json())
            .then(data => {
                this.setState({ chirps: data })
            })
            .catch(err => console.log(err))
    }

    render() {
        return (
            <div>
                <div className="panel">
                    <img src={logo} alt="logo" height="300px" />
                    <h1>Chirper</h1>
                </div>
                <NewChirp />
                <Current chirps={this.state.chirps} />
            </div>
        );
    }
}

export { Home };

chirp.jsx

import React, { Component, Fragment } from 'react';
import { BrowserRouter as Router, Link } from 'react-router-dom';
import 'isomorphic-fetch';

class Chirp extends Component {
    constructor() {
        super();
        this.state = {
            user: "",
            text: "",
        }
    }

    componentDidMount() {
        fetch(`http://127.0.0.1:3000/api/chirps/${this.props.match.params.id}`)
            .then(response => response.json())
            .then(data => {
                this.setState({
                    user: data.user,
                    text: data.text
                })
            })
            .catch(err => console.log(err))
    }

    render() {
        return (
            <div>
                <Fragment >
                    <Link to="/" className="homelink" style={{ textDecoration: "none" }}>Home</Link>
                </Fragment>
                <div className="current">
                    <div className="flex-column">
                        <div className='chirps'>
                            <p>{this.state.user}: {this.state.text}</p>
                            <Fragment >
                                <Link to={{
                                    pathname: `/${this.props.match.params.id}/edit`,
                                    state: {
                                        user: this.state.user,
                                        text: this.state.text
                                    }
                                }}>
                                    <button onClick={this.editClick}>Edit</button>
                                </Link>
                                <Link to={`/${this.props.match.params.id}/delete`}><button className="delete">x</button></Link>
                            </Fragment>
                        </div>
                    </div>
                </div>
            </div>
        )
    }
}

export { Chirp };

edit.jsx

import React, { Component, Fragment } from 'react';
import { BrowserRouter as Router, Link } from 'react-router-dom';
import 'isomorphic-fetch';

class Edit extends Component {
    constructor() {
        super();
        this.state = {
            user: "",
            text: ""
        }
        this.editChirp = this.editChirp.bind(this);
        this.inputHandler = this.inputHandler.bind(this)
    }

    inputHandler(e) {
        this.setState({ [e.target.name]: e.target.value })
    }

    editChirp(e) {
        e.preventDefault();
        fetch(`http://127.0.0.1:3000/api/chirps/${this.props.match.params.id}`, {
            method: "PUT",
            headers: {
                "Content-Type": "application/json"
            },
            body: JSON.stringify({
                user: this.state.user,
                text: this.state.text
            })
        })
            .then(() => this.props.history.push('/'))
            .catch(err => console.log(err))
        }

    render() {
        const user = this.props.location.state.user;
        const text = this.props.location.state.text;
        return (
            <div>
                <Fragment>
                    <Link to="/" className="homelink" style={{ textDecoration: "none" }}>Home</Link>
                </Fragment>
                <h2>Edit Your Chirp</h2>
                <div className="input">
                    <form action="">
                        <input
                            type="text"
                            placeholder={user}
                            size="10"
                            id="user"
                            name="user"
                            onChange={this.inputHandler}
                            value={this.state.user}
                        />
                        <input
                            type="text"
                            placeholder={text}
                            size="60"
                            id="text"
                            name="text"
                            onChange={this.inputHandler}
                            value={this.state.text}
                        />
                        <button
                            onClick={this.editChirp}
                            id="submit">
                            Update
                    </button>
                    </form>
                </div>

            </div>
        )
    }
}

export { Edit };
\$\endgroup\$
1
\$\begingroup\$

Superfluous Fragments

You don't need to wrap anything in a <Fragment>. Fragments are just used for when you want to return multiple elements without adding anything extra to the DOM.

Exports

In your components, you export an Object (like export { Edit }) when I think it would be best to just use the default export. So the last line of edit.jsx should become export default Edit and then change app.jsx to import Edit from './components/Edit'.

"Smart and Dumb" or "Presentational and Container" components

You mix your business logic and data access in with your view code, which works, but is bad for reusability and general code readability.

Instead, you should have a component that is only concerned with rendering a chirp (i.e. it takes in the chirp as a prop and doesn't fetch it), and another that fetches the data from the API and renders a <Chirp> with that.

It might look something like this:

// presentational/Chirp.jsx
export default function Chirp(props) {
    return (
        <div>
            <Link to="/" className="homelink" style={{ textDecoration: "none" }}>Home</Link>
            <div className="current">
                <div className="flex-column">
                    <div className="chirps">
                        <p>{props.user}: {props.text}</p>
                        <Link to={{
                            pathname: `/${this.props.match.params.id}/edit`,
                            state: {
                                user: this.state.user,
                                text: this.state.text
                            }
                        }}>Edit</Link>
                        <Link to={`/${this.props.match.params.id}/delete`}>x</Link>
                    </div>
                </div>
            </div>
        </div>
    )
}

// container/ChirpContainer.jsx
import Chirp from '../presentational/Chirp'

class ChirpContainer extends Component {
    constructor () {
        super()
        this.state = {
            user: '',
            text: ''
        }
    }

    componentDidMount () {
        fetch(`http://127.0.0.1:3000/api/chirps/${this.props.match.params.id}`)
            .then(response => response.json())
            .then(data => {
                this.setState({
                    user: data.user,
                    text: data.text
                })
            })
            .catch(err => console.log(err))
    }

    render () {
        return <Chirp user={this.state.user} text={this.state.text} />
    }
}

export default ChirpContainer

Showing an error message

When the call to get data from your API fails, you just print the message out to the console. This is fine for testing and development, but maybe you want to look into showing a message to the user instead, as most of them don't bother to look through the console.

\$\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.