Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: add error handling and temp file cleanup in PNG optimization #256

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: add error handling and temp file cleanup in PNG optimization #256

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 34 additions & 24 deletions _api/src/optimizeImages.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import * as fs from "fs"
import * as path from "path"
import chalk from "chalk"
import { v4 as uuidv4 } from "uuid"
import sharp from "sharp"
import { optimize } from "svgo"
import { readFileSync, writeFileSync, unlinkSync, renameSync } from 'node:fs'
import { extname, relative } from 'node:path'
import chalk from 'chalk'
import { v4 as uuidv4 } from 'uuid'
import sharp from 'sharp'
import { optimize } from 'svgo'
import { getFilePathsInDirectory, getFileSize, isDirectory } from "./utils"

// Constants for image size limits
Expand All @@ -12,10 +12,10 @@ const PNG_SIZE_LIMIT = 50 * 1024 // 50KB
const PNG_MAX_HEIGHT = 256 // 256px

// Image file extensions to optimize
const imageExtensions = [".png", ".svg"]
const imageExtensions: readonly string[] = [".png", ".svg"] as const

// Optimize images in the directory
export async function optimizeImages(dir: string) {
export async function optimizeImages(dir: string): Promise<void> {
const files = getFilePathsInDirectory(dir)

for (const file of files) {
Expand All @@ -24,7 +24,7 @@ export async function optimizeImages(dir: string) {
continue
}

const ext = path.extname(file).toLowerCase()
const ext = extname(file).toLowerCase()
if (!imageExtensions.includes(ext)) {
continue
}
Expand All @@ -42,43 +42,53 @@ export async function optimizeImages(dir: string) {
}

const newSize = getFileSize(file)
const relativePath = path.relative(dir, file)
const relativePath = relative(dir, file)
console.log(`Optimized ${relativePath}: ${originalSize} bytes -> ${newSize} bytes`)

if (ext === ".svg" && newSize > SVG_SIZE_LIMIT) {
console.warn(chalk.yellow(`Deleting oversized SVG: ${relativePath}`))
fs.unlinkSync(file)
unlinkSync(file)
}

if (ext === ".png" && newSize > PNG_SIZE_LIMIT) {
console.warn(chalk.yellow(`Deleting oversized PNG: ${relativePath}`))
fs.unlinkSync(file)
unlinkSync(file)
}
}
}

// Optimize PNG file
async function optimizePng(filePath: string) {
async function optimizePng(filePath: string): Promise<void> {
const image = sharp(filePath)
const metadata = await image.metadata()

// Use a temporary file for the output
const tempFilePath = `${filePath}.${uuidv4()}.tmp`

if (metadata.height && metadata.height > PNG_MAX_HEIGHT) {
await image.resize({ height: PNG_MAX_HEIGHT }).toFile(tempFilePath)
} else {
const data = await image.toBuffer()
await sharp(data).toFile(tempFilePath)
}
try {
if (metadata.height && metadata.height > PNG_MAX_HEIGHT) {
await image.resize({ height: PNG_MAX_HEIGHT }).toFile(tempFilePath)
} else {
const data = await image.toBuffer()
await sharp(data).toFile(tempFilePath)
}

// Replace the original file with the optimized file
fs.renameSync(tempFilePath, filePath)
// Replace the original file with the optimized file
renameSync(tempFilePath, filePath)
} catch (error) {
// Clean up temporary file if it exists
try {
unlinkSync(tempFilePath)
} catch {
// Ignore error if temp file doesn't exist
}
throw error // Re-throw the original error
}
}

// Optimize SVG file
function optimizeSvg(filePath: string) {
const data = fs.readFileSync(filePath, "utf8")
function optimizeSvg(filePath: string): void {
const data = readFileSync(filePath, "utf8")
const result = optimize(data, { path: filePath })
fs.writeFileSync(filePath, result.data)
writeFileSync(filePath, result.data)
Comment on lines +90 to +93
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add error handling to SVG optimization.

The SVG optimization lacks error handling for file operations, unlike the PNG optimization. Consider wrapping the operations in a try-catch block.

-function optimizeSvg(filePath: string): void {
-  const data = readFileSync(filePath, "utf8")
-  const result = optimize(data, { path: filePath })
-  writeFileSync(filePath, result.data)
+function optimizeSvg(filePath: string): void {
+  try {
+    const data = readFileSync(filePath, "utf8")
+    const result = optimize(data, { path: filePath })
+    writeFileSync(filePath, result.data)
+  } catch (error) {
+    console.error(`Failed to optimize SVG file: ${filePath}`, error)
+    throw error
+  }
}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function optimizeSvg(filePath: string): void {
const data = readFileSync(filePath, "utf8")
const result = optimize(data, { path: filePath })
fs.writeFileSync(filePath, result.data)
writeFileSync(filePath, result.data)
function optimizeSvg(filePath: string): void {
try {
const data = readFileSync(filePath, "utf8")
const result = optimize(data, { path: filePath })
writeFileSync(filePath, result.data)
} catch (error) {
console.error(`Failed to optimize SVG file: ${filePath}`, error)
throw error
}
}

}