Server code to upload and save images to cloud storage

Question

Is there a way to beautify my code? It works but there repeated blocks and I am not sure that I'm using some functions in the right way. I'm not new to JavaScript but I want to improve it and get rid of Bad Coding Habits.

Below is my code snippet that I'm using on the server to upload product images to cloud storage using multer and sharp.

const EasyYandexS3 = require('easy-yandex-s3')
const multer = require('multer')
const sharp = require('sharp')
const slug = require('slug')

const s3 = new EasyYandexS3({
  auth: {
    accessKeyId: process.env.KEY_ID,
    secretAccessKey: process.env.SECRET_KEY,
  },
  Bucket: process.env.BACKET, // Название бакета
  debug: false, // Дебаг в консоли
})

const storage = multer.memoryStorage()
const fileFilter = (req, file, cb) => {
  if (
    file.mimetype === 'image/jpeg' ||
    file.mimetype === 'image/jpg' ||
    file.mimetype === 'image/png'
  ) {
    cb(null, true)
  } else {
    cb(null, false)
  }
}

const upload = multer({
  storage,
  fileFilter,
  limits: {
    fileSize: 1024 * 1024 * 5, // ограничение до 5 мб
  },
})

const uploadFields = upload.fields([
  { name: 'cover', maxCount: 1 },
  { name: 'media', maxCount: 4 },
])

const uploadImages = (req, res, next) => {
  uploadFields(req, res, (err) => {
    if (err instanceof multer.MulterError) {
      if (err.code === 'LIMIT_UNEXPECTED_FILE') {
        return res.send('Превышено количество файлов.')
      }
    } else if (err) {
      return res.send(err)
    }
    next()
  })
}
const resizeImages = async (req, res, next) => {
  // Функция загрузки фотографий в бакет
  async function transform(size, file, filename, type) {
    const folder = slug(req.body.title)
    const resizedImgFilename = `${filename}-${size}`
    const resizedImgBuffer = await sharp(file.buffer)
      .resize(size)
      .toFormat('jpeg')
      .jpeg({ quality: 90 })
      .toBuffer()
    const upload = await s3.Upload(
      {
        buffer: resizedImgBuffer,
        name: resizedImgFilename,
      },
      `/products/${folder}/`
    )
    if (size === 800) {
      type.push(upload.Location.slice(0, -4))
    }
  }

  // Проверяем какие файл загружены
  if (req.files.media === undefined && req.files.cover === undefined) {
    return next()
  } else if (req.files.media === undefined) {
    await Promise.all(
      req.files.cover.map(async (file) => {
        const filename = Date.now() + Math.round(Math.random() * 1e2) + 'c'
        const type = req.body.cover
        await transform(800, file, filename, type)
        await transform(400, file, filename, type)
        await transform(250, file, filename, type)
      })
    )
  } else if (req.files.cover === undefined) {
    await Promise.all(
      req.files.media.map(async (file) => {
        const filename = Date.now() + Math.round(Math.random() * 1e2) + 'm'
        const type = req.body.media
        await transform(800, file, filename, type)
        await transform(400, file, filename, type)
        await transform(250, file, filename, type)
      })
    )
  } else {
    await Promise.all(
      req.files.media.map(async (file) => {
        const filename = Date.now() + Math.round(Math.random() * 1e2) + 'm'
        const type = req.body.media
        await transform(800, file, filename, type)
        await transform(400, file, filename, type)
        await transform(250, file, filename, type)
      })
    )
    await Promise.all(
      req.files.cover.map(async (file) => {
        const filename = Date.now() + Math.round(Math.random() * 1e2) + 'c'
        const type = req.body.cover
        await transform(800, file, filename, type)
        await transform(400, file, filename, type)
        await transform(250, file, filename, type)
      })
    )
  }
  next()
}

module.exports = {
  uploadImages,
  resizeImages,
}

Answer 1

Within resizeImages, the if (req.files.media)... seems to be highly redundant. You seem to be unnecessarily checking for existence of media and cover instead of just processing them if they are defined.

const async transformItems(items, fileType, type) {
  return Promise.all((items ?? []).map((file) => {
    const filename = Date.now() + Math.round(Math.random() * 1e2) + fileType
    await transform(800, file, filename, type)
    await transform(400, file, filename, type)
    await transform(250, file, filename, type)
  })
}

await transformItems(req.files.media, 'm', req.body.media);
await transformItems(req.files.cover, 'c', req.body.cover);

next()

There isn't really any need to guard whether media or cover exist or are populated, that can easily be handled within the helper and the promise will just return.

The rest of it doesn't appear to show much if any repetition.

As a matter of style, if prefer to use the classic form function ... declaration rather than assigning an arrow function to a const symbol, because it's clearer that it's a local function. Arrow function definition and calls can be somewhat easily misread I find.

You also have a function scope symbol called upload that will mask the file (outer) scope symbol of the same name, which adds to confusion.