[feat] Add ZIP download support for multiple assets #6997
Conversation
## Summary When downloading multiple selected assets, they are now bundled into a single ZIP file instead of downloading each file individually. ## Changes - Use `client-zip` library to generate ZIP files in the browser - Use `Promise.allSettled` to include remaining files in ZIP even if some fail - Automatically add numbers to duplicate filenames - Notify users with toast messages based on download status - Auto-generate filename with timestamp to prevent filename collisions ### Modified Files - `src/platform/assets/composables/useMediaAssetActions.ts`: Implement ZIP download logic - `src/locales/en/main.json`: Add new i18n strings - `package.json`: Add `client-zip` dependency 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
viva-jinyi
requested review from
a team,
KarryCharon,
Yorha4D and
shinshin86
as code owners
dosubot
bot
added
the
size:L
📝 WalkthroughWalkthroughThis change adds ZIP-based bulk download functionality for multiple assets. It introduces the Changes
Sequence DiagramsequenceDiagram
participant UI as User Interface
participant Comp as useMediaAssetActions
participant Fetch as Parallel Fetches
participant Zip as client-zip
participant DL as downloadBlob
participant Toast as Toast Notifications
UI->>Comp: downloadMultipleAssets(assets[])
activate Comp
Comp->>Toast: Show "Preparing ZIP" toast
activate Toast
Toast-->>Comp:
deactivate Toast
Comp->>Fetch: Fetch all assets in parallel
activate Fetch
Fetch-->>Fetch: Promise.allSettled()
Fetch-->>Comp: {name, input: Response} | failure
deactivate Fetch
Comp->>Comp: Deduplicate filenames<br/>Filter successful fetches
Comp->>Zip: Collect files for ZIP
activate Zip
Zip->>Zip: Generate ZIP blob
Zip-->>Comp: Zip blob
deactivate Zip
Comp->>DL: downloadBlob(zipBlob, filename)
activate DL
DL-->>Comp:
deactivate DL
alt All successful
Comp->>Toast: Success notification
else Partial success
Comp->>Toast: Partial success with counts
else All failed
Comp->>Toast: Error notification
end
deactivate Comp
Possibly related PRs
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 11/27/2025, 02:03:01 PM UTC 🔗 Links🎉 Your Storybook is ready for review! |
🎭 Playwright Test Results⏰ Completed at: 11/27/2025, 02:11:32 PM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 3.18 MB (baseline 3.18 MB) • 🔴 +2.84 kBMain entry bundles and manifests
Status: 3 added / 3 removed Graph Workspace — 948 kB (baseline 948 kB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 6.54 kB (baseline 6.54 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 1 added / 1 removed Panels & Settings — 298 kB (baseline 298 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 6 added / 6 removed UI Components — 139 kB (baseline 139 kB) • ⚪ 0 BReusable component library chunks
Status: 9 added / 9 removed Data & Services — 12.5 kB (baseline 12.5 kB) • ⚪ 0 BStores, services, APIs, and repositories
Status: 3 added / 3 removed Utilities & Hooks — 2.94 kB (baseline 2.94 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 1 added / 1 removed Vendor & Third-Party — 8.58 MB (baseline 8.56 MB) • 🔴 +20.5 kBExternal libraries and shared vendor chunks
Status: 5 added / 5 removed Other — 3.84 MB (baseline 3.84 MB) • ⚪ 0 BBundles that do not match a named category
Status: 23 added / 23 removed |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/platform/assets/composables/useMediaAssetActions.ts (2)
140-151: Duplicate filename numbering starts at 2.The logic correctly handles duplicates, but the numbering starts at
_2for the first duplicate (skipping_1). This is a minor UX inconsistency - users might expectfile.txt,file_1.txt,file_2.txtinstead offile.txt,file_2.txt,file_3.txt.If you prefer 0-indexed naming:
- if (nameCount.has(filename)) { - const count = nameCount.get(filename)! + 1 - nameCount.set(filename, count) + const count = nameCount.get(filename) ?? 0 + nameCount.set(filename, count + 1) + if (count > 0) { const parts = filename.split('.') const ext = parts.length > 1 ? parts.pop() : '' filename = ext ? `${parts.join('.')}_${count}.${ext}` : `${filename}_${count}` - } else { - nameCount.set(filename, 1) }
176-177: Consider memory usage for large batches.Using
.blob()loads the entire ZIP into memory. For very large batches or high-resolution assets, this could cause memory pressure. The current implementation is acceptable for typical use cases, but consider adding a file count or size limit warning if users frequently download large batches.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
package.json(1 hunks)pnpm-workspace.yaml(1 hunks)src/locales/en/main.json(1 hunks)src/platform/assets/composables/useMediaAssetActions.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (15)
**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{vue,ts,tsx}: Leverage VueUse functions for performance-enhancing utilities
Use vue-i18n in Composition API for any string literals and place new translation entries in src/locales/en/main.json
Files:
src/platform/assets/composables/useMediaAssetActions.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursorrules)
Use es-toolkit for utility functions
Files:
src/platform/assets/composables/useMediaAssetActions.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Use TypeScript for type safety
**/*.{ts,tsx}: Never useanytype - use proper TypeScript types
Never useas anytype assertions - fix the underlying type issue
Files:
src/platform/assets/composables/useMediaAssetActions.ts
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (.cursorrules)
Implement proper error handling in components and services
**/*.{ts,tsx,js,vue}: Use 2-space indentation, single quotes, no semicolons, and maintain 80-character line width as configured in.prettierrc
Organize imports by sorting and grouping by plugin, and runpnpm formatbefore committing
Files:
src/platform/assets/composables/useMediaAssetActions.ts
src/**/*.{vue,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{vue,ts}: Leverage VueUse functions for performance-enhancing styles
Implement proper error handling
Use vue-i18n in composition API for any string literals. Place new translation entries in src/locales/en/main.json
Files:
src/platform/assets/composables/useMediaAssetActions.ts
src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.ts: Use es-toolkit for utility functions
Use TypeScript for type safety
Files:
src/platform/assets/composables/useMediaAssetActions.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use camelCase for variable and setting names in TypeScript/Vue files
Files:
src/platform/assets/composables/useMediaAssetActions.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,vue}: Useconst settingStore = useSettingStore()andsettingStore.get('Comfy.SomeSetting')to retrieve settings in TypeScript/Vue files
Useawait settingStore.set('Comfy.SomeSetting', newValue)to update settings in TypeScript/Vue files
Check server capabilities usingapi.serverSupportsFeature('feature_name')before using enhanced features
Useapi.getServerFeature('config_name', defaultValue)to retrieve server feature configurationEnforce ESLint rules for Vue + TypeScript including: no floating promises, no unused imports, and i18n raw text restrictions in templates
Files:
src/platform/assets/composables/useMediaAssetActions.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Define dynamic setting defaults using runtime context with functions in settings configuration
UsedefaultsByInstallVersionproperty for gradual feature rollout based on version in settings configuration
Files:
src/platform/assets/composables/useMediaAssetActions.ts
src/**/{services,composables}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/{services,composables}/**/*.{ts,tsx}: Useapi.apiURL()for backend endpoints instead of constructing URLs directly
Useapi.fileURL()for static file access instead of constructing URLs directly
Files:
src/platform/assets/composables/useMediaAssetActions.ts
src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/*.{ts,tsx,vue}: Sanitize HTML with DOMPurify to prevent XSS attacks
Avoid using @ts-expect-error; use proper TypeScript types instead
Use es-toolkit for utility functions instead of other utility libraries
Implement proper TypeScript types throughout the codebase
Files:
src/platform/assets/composables/useMediaAssetActions.ts
src/**/{composables,components}/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Clean up subscriptions in state management to prevent memory leaks
Files:
src/platform/assets/composables/useMediaAssetActions.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/platform/assets/composables/useMediaAssetActions.ts
src/**/{components,composables}/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Use vue-i18n for ALL user-facing strings by adding them to
src/locales/en/main.json
Files:
src/platform/assets/composables/useMediaAssetActions.ts
**/composables/use*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Name composables in the format
useXyz.ts
Files:
src/platform/assets/composables/useMediaAssetActions.ts
🧠 Learnings (4)
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Use provide/inject for dependency injection in Vue components
Applied to files:
src/platform/assets/composables/useMediaAssetActions.ts
📚 Learning: 2025-11-24T19:47:02.860Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T19:47:02.860Z
Learning: Applies to src/**/*.vue : Utilize provide/inject for dependency injection
Applied to files:
src/platform/assets/composables/useMediaAssetActions.ts
📚 Learning: 2025-11-24T19:46:52.279Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-24T19:46:52.279Z
Learning: Applies to **/*.vue : Utilize provide/inject for dependency injection in Vue 3
Applied to files:
src/platform/assets/composables/useMediaAssetActions.ts
📚 Learning: 2025-11-24T19:47:34.324Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:34.324Z
Learning: Applies to src/**/*.{ts,tsx,vue} : Use es-toolkit for utility functions instead of other utility libraries
Applied to files:
package.json
🧬 Code graph analysis (1)
src/platform/assets/composables/useMediaAssetActions.ts (3)
src/platform/assets/schemas/assetSchema.ts (1)
AssetItem(72-72)src/platform/assets/utils/assetUrlUtil.ts (1)
getAssetUrl(21-29)scripts/size-collect.js (1)
files(42-42)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: collect
- GitHub Check: lint-and-format
- GitHub Check: test
- GitHub Check: setup
🔇 Additional comments (6)
pnpm-workspace.yaml (1)
49-49: LGTM!The
client-ziplibrary is a good choice for browser-based ZIP creation - it's lightweight, supports streaming, and handles the use case of bundling multiple assets well.package.json (1)
162-162: LGTM!Dependency correctly added to
dependencies(notdevDependencies) sinceclient-zipis used at runtime for ZIP creation in the browser.src/locales/en/main.json (1)
2187-2190: LGTM!The new i18n strings are well-structured with proper interpolation placeholders (
{count},{succeeded},{failed}) and provide clear feedback for all ZIP download states (preparing, success, partial success, failure).src/platform/assets/composables/useMediaAssetActions.ts (3)
118-162: Good use ofPromise.allSettledfor resilient parallel fetching.The implementation correctly uses
Promise.allSettledto handle partial failures, allowing the ZIP to include successfully fetched files even when some fail. The inner try-catch and warning logs provide good observability for debugging.
112-112: Good use of dynamic import for code splitting.Dynamically importing
client-ziponly when needed reduces the initial bundle size for users who don't use the multi-download feature.
179-184: LGTM!The timestamp format is filesystem-safe (avoids colons which aren't allowed on Windows) and provides sufficient granularity to prevent collisions.
| // Show loading toast | ||
| const loadingToast = { | ||
| severity: 'info' as const, | ||
| summary: t('g.loading'), | ||
| detail: t('mediaAsset.selection.preparingZip', { count: assets.length }), | ||
| life: 0 // Keep until manually removed | ||
| } | ||
| toast.add(loadingToast) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
PrimeVue useToast remove method documentation
💡 Result:
Short answer: useToast returns the Toast service (PrimeVueToast) which exposes add(...), removeGroup(group) and removeAllGroups() — there is no documented single-item remove(id/message) on the current PrimeVue Toast service API. [1][2][3]
Sources:
- PrimeVue Toast docs (useToast / ToastService). [1] (primevue.org)
- ToastService type definitions showing available methods (add, removeGroup, removeAllGroups). [2] (app.unpkg.com)
- useToast implementation (injects PrimeVueToast symbol). [3] (app.unpkg.com)
If you want an API that removes a specific toast, tell me which PrimeVue version you use and I’ll check that version’s source/commit history.
Citations:
- 1: https://primevue.org/toast?utm_source=openai
- 2: https://app.unpkg.com/primevue%403.29.0/files/toastservice/ToastService.d.ts?utm_source=openai
- 3: https://app.unpkg.com/primevue%404.0.0-beta.4/files/usetoast/usetoast.mjs?utm_source=openai
🏁 Script executed:
# Find and read the file to see the actual implementation
cat -n src/platform/assets/composables/useMediaAssetActions.ts | sed -n '95,120p'Repository: Comfy-Org/ComfyUI_frontend
Length of output: 1103
🏁 Script executed:
# Search for other toast.remove() usage in the codebase to see patterns
rg -n 'toast\.remove' --type ts --type tsx -B 2 -A 2Repository: Comfy-Org/ComfyUI_frontend
Length of output: 97
🏁 Script executed:
# Search for toast.remove() calls using correct type filtering
rg -n 'toast\.remove' --type ts -B 2 -A 2Repository: Comfy-Org/ComfyUI_frontend
Length of output: 955
🏁 Script executed:
# Read more of the file to see where the toast is removed
cat -n src/platform/assets/composables/useMediaAssetActions.ts | sed -n '100,180p'Repository: Comfy-Org/ComfyUI_frontend
Length of output: 3323
Toast removal mechanism will fail — toast.remove() doesn't accept message objects.
PrimeVue's toast.remove() method does not exist in the ToastService API. The only available removal methods are removeGroup(group) and removeAllGroups(). The current code at lines 190 and 216 attempts to call toast.remove(loadingToast), which will not work and the loading toast will persist indefinitely.
Use a message group to reliably remove the loading toast:
- // Show loading toast
- const loadingToast = {
- severity: 'info' as const,
- summary: t('g.loading'),
- detail: t('mediaAsset.selection.preparingZip', { count: assets.length }),
- life: 0 // Keep until manually removed
- }
- toast.add(loadingToast)
+ // Show loading toast with group for removal
+ const loadingGroup = 'zip-download-loading'
+ toast.add({
+ severity: 'info',
+ summary: t('g.loading'),
+ detail: t('mediaAsset.selection.preparingZip', { count: assets.length }),
+ life: 0, // Keep until manually removed
+ group: loadingGroup
+ })Then update removal calls (lines 190 and 216):
- toast.remove(loadingToast)
+ toast.removeGroup(loadingGroup)and
- toast.remove(loadingToast)
+ toast.removeGroup(loadingGroup)📝 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.
| // Show loading toast | |
| const loadingToast = { | |
| severity: 'info' as const, | |
| summary: t('g.loading'), | |
| detail: t('mediaAsset.selection.preparingZip', { count: assets.length }), | |
| life: 0 // Keep until manually removed | |
| } | |
| toast.add(loadingToast) | |
| // Show loading toast with group for removal | |
| const loadingGroup = 'zip-download-loading' | |
| toast.add({ | |
| severity: 'info', | |
| summary: t('g.loading'), | |
| detail: t('mediaAsset.selection.preparingZip', { count: assets.length }), | |
| life: 0, // Keep until manually removed | |
| group: loadingGroup | |
| }) |
🤖 Prompt for AI Agents
In src/platform/assets/composables/useMediaAssetActions.ts around lines 102-109
(and update calls at ~190 and ~216), the loading toast is created as a message
object but later removed with toast.remove(message) which is invalid; change the
loading toast to include a message group (e.g., group: 'loadingZip') when adding
it, then replace the failing removal calls with toast.removeGroup('loadingZip')
so the loading toast can be reliably cleared; keep life: 0 if you still want it
persistent until removal.
christian-byrne
left a comment
christian-byrne
left a comment
There was a problem hiding this comment.
This is a good idea but what are pros/cons of doing this on the client vs. doing this on the server?
Summary
When downloading multiple selected assets, they are now bundled into a single ZIP file instead of downloading each file individually.
Changes
client-ziplibrary to generate ZIP files in the browserPromise.allSettledto include remaining files in ZIP even if some failModified Files
src/platform/assets/composables/useMediaAssetActions.ts: Implement ZIP download logicsrc/locales/en/main.json: Add new i18n stringspackage.json: Addclient-zipdependencyscreen-capture.webm
🤖 Generated with Claude Code
Co-Authored-By: Claude [email protected]
┆Issue is synchronized with this Notion page by Unito