TypeScript, React and Node app to get 'games' data from a database and display them on a frontend

Question

This is only one of the first apps I've built with Node, React and TypeScript. I was wondering what could be made better. This is at this point the best I've been able to come up with, but I'm not sure if there's code here that could be difficult for another developer to read, could be improved, or anything else really. Or even stuff that could be added to this app.

I'd appreciate any kind of feedback on the entirety of my code.

Backend

app.ts

import { mySqlConnect } from "./utils/mySqlConnect"

const NUM_GAMES = 50

const app = express()
app.use(cors())

const db = mysql.createConnection(mySqlConnect)

db.connect(function (err) {
  if (err) {
    console.error("failed connection:" + err.stack)
    return
  }
  console.log("Connected.")
})

app.get("/api/games", (req, res) => {
  const teamId = req.query.team_id
  const offset = req.query.offset

  let sql = ` SELECT * FROM games
    WHERE team = '${teamId}' 
    ORDER BY date DESC 
    LIMIT ${NUM_GAMES} OFFSET ${offset}
  `

  db.query(sql, (err, results) => {
    if (err) throw err
    const response = results.map((game: { payload: string }) => JSON.parse(game.payload))
    res.send(response)
  })
})

mySqlConnect.ts

import * as dotenv from "dotenv"
dotenv.config()

const config = {
  host: process.env.HOSTNAME,
  user: process.env.USERNAME,
  password: process.env.PASSWORD,
  port: process.envPORT,
  database: process.env.DB_NAME,
}

type RemoveUndefinedFields<T> = {
  [P in keyof T]: Exclude<T[P], undefined>
}

export const mysqlConnect = config as RemoveUndefinedFields<typeof config>

Frontend

App.js

const App = () => {
  return (
    <div>
      <GlobalStyle />
      <NavBar />
      <Games />
    </div>
  )
}

Games.tsx

const OFFSET_GAMES = 10
const TEAM_ID = "team_id_here"

const Games = () => {
  const [games, setGames] = useState<Game[]>([])
  const [numFetches, setNumFetches] = useState(OFFSET_GAMES)
  const [isLoading, setIsLoading] = useState(false)

  useEffect(() => {
    setIsLoading(prev => !prev)
    fetch(`https://localhost:3402/api/games/?team_id=${TEAM_ID}&offset=${numFetches}`)
      .then((res) => res.json())
      .then((res) => {
        setGames((prevGames) => [...prevGames, ...res])
        setIsLoading(prev => !prev)
      })
  }, [numFetches])

  const fetchGames = () => setNumFetches((prev) => prev + OFFSET_GAMES)

  const displayGames = () => {
    const datesShown: string[] = []

    return games.map(game => {
      const date = parseDate(game.date)
      let showDate = true

      if (datesShown.includes(date)) {
        showDate = false
      } else {
        datesShown.push(date)
      }

      return (
        <>
          {showDate && <Date date={date}/>}
          <Game
            id={game.id}
            game_type={game.game_type}
            date={game.date}
            observations={game.observations}
          />
        </>
      )
    })
  }

  return (
    <div>
      {games.length ? (
        <GamesContainer>
          {displayGames()}
          <Button
            text="Load more games"
            onClick={fetchGames}
            isLoading={isLoading}
          />
        </GamesContainer>
      ) : null}
      {(!games.length || isLoading) && <Loader text={"Loading games..."} />}
    </div>
  )
}

GameCard.tsx

export type Game = {
  id: string
  game_type: string
  date: string
  observations?: string[] | undefined
}

const GameCard: React.FC<Game> = ({
  id,
  game_type,
  date,
  observations,
}) => {

  const gameMap = new Map(Object.entries(gameDictionary))

  return (
    <GameCardContainer key={id}>
      <h3>{parseDate(date)}</h3>
      <h4>{gameMap.get(game_type)}</h4>
      {observations?.map((obs) => (
        <h5>{obs}</h5>
      ))}
    </GameCardContainer>
  )
}

gameDictionary.ts

export const gameDictionary = {
  football: "Football match, the derby!",
  basketball: "Basketball game. Looking for MJ?",
  tennis: "Nadal X Federer? Tennis match!",
}

Answer 1

general

a

You want to use typescript this is a good practice to use type for your function or const or state.

Example :

function something (a: string, b: boolean): void {
  // ....
}

const something: void = (a: string, b: boolean) => {
  // ....
}

const a: number = 0;

const [a, setA] = useState<boolean>(false);

b

This is a good practice too to have some comment on your code. A simply way to add it (in my opinion) is the jsdoc.

c

I don't know if you have this kind of stuff but if you want to have a quality standard on your code read about eslint (now eslint handle the tslint).

api

a

console.log

My opinion

• use a logger instead of the console.log command like pino, the advantage of that is to have a production ready logger.

For example :

• when the NODE_ENV is set to production the log is writen on a single line
• this kind of stuff provide multiple level of log, this is useful because you have the ability to set different log for different environment, in dev you set the level to debug, in prod to error and so on.

b

if (err) throw err

My opinion :

this is a bad practice to throw the db error from the api, if some user want to use your api for bad reason this log possibly tell us some useful information about your database or data model

• throw custom error if the teamId doesn't exist (like http 404)
• if db error throw custom error too (maybe http 500)

c

port: process.envPORT

replace by port: process.env.PORT

d

This is purely personal ^^ but I don't see the value of putting /api into your url. In general we put this on the url via a sub domain name (for example app is todo.io and api api.todo.io)

app

a

GlobalStyle

What contain this file ?

b

https://localhost:3402

put this kind of information into a .env file is more powerful if you want to have more than one environment (dev, staging, prod for example)

and this will save you a lot of time if the url or the port change (this is a pain to refactor 100 fetch not to refactor one .env file :) )

c

setIsLoading(prev => !prev)

My opinion :

In general you know when you want the loading is true or false. I think this is more strong to have directly the right value into the set function (example: setIsLoading(false)). In some case, when the code failed or do not what you want to do, the !boolean can put the wrong value

d

<></>

My opinion : Personally I prefer to use <Fragment></Fragment>, this do the same thing but I find it more readable

but in the most of the case a div handle this kind of stuff too.

More info here.

e

{games.length ?

I think you don't need to have a ternary here, a games.length && is sufficient.

f

{observations?

Personally I prefer to use

{(observations && observations.length) && 
  observations.map((obs) => (
    <h5>{obs}</h5>
  ))
}