0
\$\begingroup\$

I use this code to populate a DB with a batch of pokemon entries from the POKEAPI.

Since I'm using TypeScript, I added some custom parsing to use the data's type in the code. I've used a mix of A instanceof Object and 'field' in A, but what would be a better way?

On top of this, I'm converting data to the Result<T> type. This is just to avoid using null or undefined for data that couldn't be parsed.

Other question: at the bottom I put await prisma.pokemon.createMany({ data: allPokemon }) (with await) because the createMany didn't go through without await. Did this happen because the DB would disconnect too early or something?

// ===================
// RESULT
// ===================

type Result<T> = Ok<T> | Error

type Ok<T> = { type: 'ok', data: T }
type Error = { type: 'error', error: string }

const ok = <T>(data: T): Ok<T> => ({ type: 'ok', data })
const error = (error: string): Error => ({ type: 'error', error })

const isOk = <T>(res: Result<T>): res is Ok<T> => res.type === 'ok'

/**
 * Returns data if `res` is Ok, or `def` otherwise
 *
 * @returns T
 */
const orDefault = <T>(res: Result<T>, def: T) => res.type === 'ok' ? res.data : def

/**
 * Collects data from any Ok value in an array of Results.
 * Also acts as a filter for Error values.
 *
 * @returns {T[]}
 */
const collectData = <T>(resAr: Result<T>[]) => resAr.filter(isOk).map(cur => cur.data)

// ===================
// POKEMON PARSING
// ===================

type Pokemon = { id: number, name: string }

type PokemonBatch = {
  count: number
  results: { name: string }[]
}

const isPokemonName = (input: unknown): input is Pokemon => input instanceof Object && 'name' in input && typeof input.name === 'string'

const parsePokemonName = (input: unknown): Result<{ name: string }> =>
  isPokemonName(input) ? ok(input) : error('Failed to parse pokemon name')

const isPokemonBatchLike = (input: unknown): input is PokemonBatch =>
  input instanceof Object && 'count' in input && typeof input.count === 'number' && 'results' in input && Array.isArray(input.results)

const parsePokemonBatch = (input: unknown): Result<PokemonBatch> => {
  if (isPokemonBatchLike(input)) {
    // filter out fields we couldn't parse
    const results = collectData(input.results.map(parsePokemonName))

    return ok({ count: input.count, results: results })
  }

  return error('Failed to parse pokemon batch.')
}

const getPokemonCount = () =>
  fetch(`https://pokeapi.co/api/v2/pokemon`)
    .then(res => res.json())
    .then(data => orDefault(parsePokemonBatch(data), { count: 0, results: [] }).count)

const getAllPokemon = async (): Promise<Pokemon[]> => {
  const count = await getPokemonCount()

  const pokemon = await fetch(`https://pokeapi.co/api/v2/pokemon?limit=${count}`)
    .then(res => res.json())
    .then(data => parsePokemonBatch(data))

  if (pokemon.type === 'ok')
    // using index for id field, for now doesn't matter if the order is correct
    return pokemon.data.results.map(({ name }, i) => ({ id: i, name }))
  else
    return []
}

// ===================
// PRISMA
// ===================

const prisma = new PrismaClient()

async function main() {
  const count = await prisma.pokemon.count()

  // populate DB if first time / empty
  if (count === 0) {
    const allPokemon = await getAllPokemon()

    await prisma.pokemon.createMany({ data: allPokemon })
  }
}

main()
  .then(async () => {
    await prisma.$disconnect()
  })
  .catch(async (e) => {
    console.error(e)
    await prisma.$disconnect()
    process.exit(1)
  })
```
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$
  • All in all, it was fairly easy to read and understand the code. Some minor improvements suggested below
  • Can't speak to the prisma usage, as I'm not familiar with it. However, it looks like you are correct that the function without await would immediately return and run the disconnect code. I'm a bit surprised this didn't cause an error though
  • The same endpoint is called twice, first to get the count, then to use that count as limit. Since you intend to get everything in one go anyway, you can just use a really high value for limit instead. If the API has a max value for the limit, you could use the "next" token returned in the response to iterate over the results
  • The code abstracts over the API with names like 'getPokemonCount' and 'getAllPokemon', and it does some specific processing for the application. This path leads you to repeat the code for each endpoint for any concern you have in your application. Already you are parsing the same type of response in two places in your code. I suggest creating a really thin layer over the API instead. E.g. just 'getPokemons' (or whatever the API endpoint is called), and add support for passing the various query parameters and whatnot supported in that function. This way you make the API client a single concern in your code.
  • Using the name 'Error' as a type is a bit annoying since it clashes with the global Error object. Maybe use 'Failure' instead?
  • 'isPokemonName' is not the best name. The input is not a pokemon name, it's an object that contains a pokemon name. In addition, the guard clause sets the type to "Pokemon". A better name would be 'isPokemon', or 'isPokemonLike' to adhere to the standard set by 'isPokemonBatchLike'
  • I think you can reduce the amount of code by a lot if you use nulls instead of ok/error. 'orDefault' would be replaced by ??. collectData would be replaced by a simple filter. The reason why I think this is that you don't handle the errors anyway other than defaulting to empty values, so there's nothing to gain.
  • About parsing json into types: If you want to do this the safest possible way, I recommend looking into something like typebox + ajv, where you can define your json schema and types in one go, and parse them with proper error messages and the whole shebang. If you trust the API, this may be a bit overkill, and you could just cast the response type instead.
  • If it's important to always clean something up after a promise is done (like disconnecting in both the then and catch block), you can use 'finally'. Probably not a big deal here though.
\$\endgroup\$
2
  • \$\begingroup\$ Thanks a lot for your help! Small question about your point about using 'finally', specifically if used for the DB disconnection part: is there a way to deal with the process.exit part, which should only happen in the catch block after the DB disconnected? \$\endgroup\$
    – eloy
    Commented Dec 27, 2023 at 11:15
  • \$\begingroup\$ Yes, you can put the finally part before then and catch. It will then run before the catch block, but still pass the error transparently. One thing to look out for is that the disconnect can also throw, so the original error may be masked by this if it happens. \$\endgroup\$ Commented Dec 30, 2023 at 8:06

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.