⚡ Bolt: [performance improvement] Parallelize icon base64 conversions#444
⚡ Bolt: [performance improvement] Parallelize icon base64 conversions#444aafre wants to merge 1 commit into
Conversation
Replaced a sequential `for...of` loop with `Promise.all` in `exportIconsForYAML` to concurrently process `FileReader` conversions, reducing I/O blocking time when exporting multiple resume icons. Co-authored-by: aafre <8656674+aafre@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request parallelizes icon exports in the useIconRegistry hook by replacing a sequential for...of loop with Promise.all to improve performance. Feedback suggests refactoring the implementation to avoid side-effects within the map function by using Object.fromEntries and enhancing type safety through property preservation and the use of the unknown type.
| await Promise.all( | ||
| targetFilenames.map(async (filename) => { | ||
| const entry = registry[filename]; | ||
| if (entry) { | ||
| try { | ||
| const base64Data = await fileToBase64(entry.file); | ||
| exportData[filename] = { | ||
| data: base64Data, | ||
| type: entry.file.type, | ||
| size: entry.file.size, | ||
| uploadedAt: entry.uploadedAt.toISOString(), | ||
| }; | ||
| } catch (error) { | ||
| console.warn(`Failed to export icon ${filename}:`, error); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| }) | ||
| ); |
There was a problem hiding this comment.
While the current implementation correctly parallelizes the asynchronous fileToBase64 operations, it relies on side-effects (mutating the exportData object) inside the map function. A more idiomatic approach is to return key-value pairs and use Object.fromEntries. Additionally, to align with repository standards, ensure all original properties are preserved during transformation to prevent data loss, and use unknown instead of any for better type safety.
const exportResults = await Promise.all(
targetFilenames.map(async (filename) => {
const entry = registry[filename];
if (!entry) return null;
try {
const base64Data = await fileToBase64(entry.file);
return [
filename,
{
...entry,
data: base64Data,
type: entry.file.type,
size: entry.file.size,
uploadedAt: entry.uploadedAt.toISOString(),
},
];
} catch (error) {
console.warn("Failed to export icon " + filename + ":", error);
return null;
}
})
);
const exportData: IconExportData = Object.fromEntries(
exportResults.filter((result): result is [string, unknown] => result !== null)
);References
- When transforming data objects, preserve all original properties by default to prevent accidental data loss, rather than hardcoding for an expected schema.
- When handling data from external sources, prefer using the unknown type over any to enforce type-checking and improve type safety.
1 participant
💡 What: Refactored the
exportIconsForYAMLfunction inuseIconRegistry.tsto map over filenames and usePromise.allinstead of a sequentialfor...ofawait loop.🎯 Why: Executing independent asynchronous file conversion tasks sequentially artificially limits throughput and blocks the main execution flow linearly.
📊 Impact: Total export time is now constrained by the longest single conversion rather than the sum of all conversions, significantly speeding up the save/export process for users with many custom icons.
🔬 Measurement: Verified by ensuring the linter passes for the refactored code and that existing tests still run successfully (accounting for pre-existing unrelated test failures).
PR created automatically by Jules for task 11453954068509134295 started by @aafre