⚡ Bolt: Use Promise.all for concurrent file reading in icon export#494
⚡ Bolt: Use Promise.all for concurrent file reading in icon export#494aafre wants to merge 1 commit into
Conversation
Replaced a sequential `await` loop with `Promise.all` in `exportIconsForYAML` inside `useIconRegistry.ts`. 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 optimizes the icon export process in useIconRegistry.ts by refactoring sequential file read operations into concurrent execution using Promise.all. A review comment suggests improving the implementation by avoiding shared state mutation within the concurrent tasks to ensure deterministic key ordering and more idiomatic code.
| const exportData: IconExportData = {}; | ||
|
|
||
| for (const filename of targetFilenames) { | ||
| 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); | ||
| // ⚡ Bolt Performance Optimization: Process FileReader conversions concurrently | ||
| // Replaced sequential await in loop with Promise.all to significantly reduce | ||
| // total export time when processing multiple icon files, while preserving | ||
| // individual error handling. | ||
| 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 using Promise.all correctly introduces concurrency, mutating a shared object (exportData) from within concurrent promises can lead to non-deterministic key ordering in the resulting object. This happens because the order of keys in the final object will depend on the completion order of the asynchronous fileToBase64 calls.
A more idiomatic and robust approach is to have the promises return the data and then assemble the object using Object.fromEntries. This ensures that the order of keys in the exported data remains consistent with the order in targetFilenames (which is deterministic) and avoids side effects inside the concurrent tasks.
References
- Avoid mutating shared state from within concurrent asynchronous operations to prevent race conditions and ensure deterministic behavior.
|
Closing duplicate — the most recent version of this change is PR #535. |
Understood. Acknowledging that this work is a duplicate and stopping work on this task. |
1 participant
💡 What: Replaced a sequential
awaitloop withPromise.allinexportIconsForYAMLinsideuseIconRegistry.ts.🎯 Why: Reading multiple files sequentially blocks execution unnecessarily. Concurrent reads are much faster.
📊 Impact: Significantly reduces total export time when processing multiple icon files by utilizing parallel
FileReaderconversions.🔬 Measurement: Observe faster YAML exports when multiple icons are in the registry.
PR created automatically by Jules for task 15608440110130733370 started by @aafre