[backport rh-test] refactor: centralize all download utils across app and apply special cloud-specific behavior (#6230)

Backport of #6188 to `rh-test`

Automatically created by backport workflow.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-6230-backport-rh-test-refactor-centralize-all-download-utils-across-app-and-apply-special-c-2956d73d3650810d980fc30dfea02cc5)
by [Unito](https://www.unito.io)

Co-authored-by: Christian Byrne <cbyrne@comfy.org>
This commit is contained in:
Comfy Org PR Bot
2025-10-24 04:23:36 +09:00
committed by GitHub
parent 797b1c5bae
commit 63c91a62fd
9 changed files with 160 additions and 89 deletions

View File

@@ -1,29 +1,64 @@
/**
* Utility functions for downloading files
*/
import { isCloud } from '@/platform/distribution/types'
// Constants
const DEFAULT_DOWNLOAD_FILENAME = 'download.png'
/**
* Trigger a download by creating a temporary anchor element
* @param href - The URL or blob URL to download
* @param filename - The filename to suggest to the browser
*/
function triggerLinkDownload(href: string, filename: string): void {
const link = document.createElement('a')
link.href = href
link.download = filename
link.style.display = 'none'
document.body.appendChild(link)
link.click()
document.body.removeChild(link)
}
/**
* Download a file from a URL by creating a temporary anchor element
* @param url - The URL of the file to download (must be a valid URL string)
* @param filename - Optional filename override (will use URL filename or default if not provided)
* @throws {Error} If the URL is invalid or empty
*/
export const downloadFile = (url: string, filename?: string): void => {
export function downloadFile(url: string, filename?: string): void {
if (!url || typeof url !== 'string' || url.trim().length === 0) {
throw new Error('Invalid URL provided for download')
}
const link = document.createElement('a')
link.href = url
link.download =
const inferredFilename =
filename || extractFilenameFromUrl(url) || DEFAULT_DOWNLOAD_FILENAME
// Trigger download
document.body.appendChild(link)
link.click()
document.body.removeChild(link)
if (isCloud) {
// Assets from cross-origin (e.g., GCS) cannot be downloaded this way
void downloadViaBlobFetch(url, inferredFilename).catch((error) => {
console.error('Failed to download file', error)
})
return
}
triggerLinkDownload(url, inferredFilename)
}
/**
* Download a Blob by creating a temporary object URL and anchor element
* @param filename - The filename to suggest to the browser
* @param blob - The Blob to download
*/
export function downloadBlob(filename: string, blob: Blob): void {
const url = URL.createObjectURL(blob)
triggerLinkDownload(url, filename)
// Revoke on the next microtask to give the browser time to start the download
queueMicrotask(() => URL.revokeObjectURL(url))
}
/**
@@ -39,3 +74,15 @@ const extractFilenameFromUrl = (url: string): string | null => {
return null
}
}
const downloadViaBlobFetch = async (
href: string,
filename: string
): Promise<void> => {
const response = await fetch(href)
if (!response.ok) {
throw new Error(`Failed to fetch ${href}: ${response.status}`)
}
const blob = await response.blob()
downloadBlob(filename, blob)
}

View File

@@ -3,6 +3,7 @@ import { GLTFExporter } from 'three/examples/jsm/exporters/GLTFExporter'
import { OBJExporter } from 'three/examples/jsm/exporters/OBJExporter'
import { STLExporter } from 'three/examples/jsm/exporters/STLExporter'
import { downloadBlob } from '@/base/common/downloadUtil'
import { t } from '@/i18n'
import { useToastStore } from '@/platform/updates/common/toastStore'
@@ -38,13 +39,7 @@ export class ModelExporter {
try {
const response = await fetch(url)
const blob = await response.blob()
const link = document.createElement('a')
link.href = URL.createObjectURL(blob)
link.download = desiredFilename
link.click()
URL.revokeObjectURL(link.href)
downloadBlob(desiredFilename, blob)
} catch (error) {
console.error('Error downloading from URL:', error)
useToastStore().addAlert(t('toastMessages.failedToDownloadFile'))
@@ -152,19 +147,11 @@ export class ModelExporter {
private static saveArrayBuffer(buffer: ArrayBuffer, filename: string): void {
const blob = new Blob([buffer], { type: 'application/octet-stream' })
const link = document.createElement('a')
link.href = URL.createObjectURL(blob)
link.download = filename
link.click()
URL.revokeObjectURL(link.href)
downloadBlob(filename, blob)
}
private static saveString(text: string, filename: string): void {
const blob = new Blob([text], { type: 'text/plain' })
const link = document.createElement('a')
link.href = URL.createObjectURL(blob)
link.download = filename
link.click()
URL.revokeObjectURL(link.href)
downloadBlob(filename, blob)
}
}

View File

@@ -1,5 +1,7 @@
import * as THREE from 'three'
import { downloadBlob } from '@/base/common/downloadUtil'
import { type EventManagerInterface } from './interfaces'
export class RecordingManager {
@@ -149,17 +151,7 @@ export class RecordingManager {
try {
const blob = new Blob(this.recordedChunks, { type: 'video/webm' })
const url = URL.createObjectURL(blob)
const a = document.createElement('a')
document.body.appendChild(a)
a.style.display = 'none'
a.href = url
a.download = filename
a.click()
window.URL.revokeObjectURL(url)
document.body.removeChild(a)
downloadBlob(filename, blob)
this.eventManager.emitEvent('recordingExported', null)
} catch (error) {

View File

@@ -1,3 +1,4 @@
import { downloadBlob } from '@/base/common/downloadUtil'
import { t } from '@/i18n'
import { LGraphCanvas } from '@/lib/litegraph/src/litegraph'
import { useToastStore } from '@/platform/updates/common/toastStore'
@@ -145,18 +146,7 @@ class ManageTemplates extends ComfyDialog {
const json = JSON.stringify({ templates: this.templates }, null, 2) // convert the data to a JSON string
const blob = new Blob([json], { type: 'application/json' })
const url = URL.createObjectURL(blob)
const a = $el('a', {
href: url,
download: 'node_templates.json',
style: { display: 'none' },
parent: document.body
})
a.click()
setTimeout(function () {
a.remove()
window.URL.revokeObjectURL(url)
}, 0)
downloadBlob('node_templates.json', blob)
}
override show() {
@@ -298,19 +288,9 @@ class ManageTemplates extends ComfyDialog {
const blob = new Blob([json], {
type: 'application/json'
})
const url = URL.createObjectURL(blob)
const a = $el('a', {
href: url,
// @ts-expect-error fixme ts strict error
download: (nameInput.value || t.name) + '.json',
style: { display: 'none' },
parent: document.body
})
a.click()
setTimeout(function () {
a.remove()
window.URL.revokeObjectURL(url)
}, 0)
// @ts-expect-error fixme ts strict error
const name = (nameInput.value || t.name) + '.json'
downloadBlob(name, blob)
}
}),
$el('button', {

View File

@@ -1,5 +1,6 @@
import { toRaw } from 'vue'
import { downloadBlob } from '@/base/common/downloadUtil'
import { t } from '@/i18n'
import { LGraph, LGraphCanvas } from '@/lib/litegraph/src/litegraph'
import type { Point, SerialisableGraph } from '@/lib/litegraph/src/litegraph'
@@ -14,7 +15,6 @@ import { useWorkflowThumbnail } from '@/renderer/core/thumbnail/useWorkflowThumb
import { api } from '@/scripts/api'
import { app } from '@/scripts/app'
import { blankGraph, defaultGraph } from '@/scripts/defaultGraph'
import { downloadBlob } from '@/scripts/utils'
import { useDialogService } from '@/services/dialogService'
import { useExtensionService } from '@/services/extensionService'
import { useDomWidgetStore } from '@/stores/domWidgetStore'

View File

@@ -51,20 +51,8 @@ export async function addStylesheet(
})
}
export function downloadBlob(filename: string, blob: Blob) {
const url = URL.createObjectURL(blob)
const a = $el('a', {
href: url,
download: filename,
style: { display: 'none' },
parent: document.body
})
a.click()
setTimeout(function () {
a.remove()
window.URL.revokeObjectURL(url)
}, 0)
}
/** @knipIgnoreUnusedButUsedByCustomNodes */
export { downloadBlob } from '@/base/common/downloadUtil'
export function uploadFile(accept: string) {
return new Promise<File>((resolve, reject) => {

View File

@@ -1,13 +1,14 @@
import { toRaw } from 'vue'
import { fromZodError } from 'zod-validation-error'
import { downloadBlob } from '@/base/common/downloadUtil'
import { useErrorHandling } from '@/composables/useErrorHandling'
import { LGraphCanvas, LiteGraph } from '@/lib/litegraph/src/litegraph'
import { useSettingStore } from '@/platform/settings/settingStore'
import { paletteSchema } from '@/schemas/colorPaletteSchema'
import type { Colors, Palette } from '@/schemas/colorPaletteSchema'
import { app } from '@/scripts/app'
import { downloadBlob, uploadFile } from '@/scripts/utils'
import { uploadFile } from '@/scripts/utils'
import { useNodeDefStore } from '@/stores/nodeDefStore'
import { useColorPaletteStore } from '@/stores/workspace/colorPaletteStore'

View File

@@ -1,5 +1,6 @@
import _ from 'es-toolkit/compat'
import { downloadFile } from '@/base/common/downloadUtil'
import { useSelectedLiteGraphItems } from '@/composables/canvas/useSelectedLiteGraphItems'
import { useNodeAnimatedImage } from '@/composables/node/useNodeAnimatedImage'
import { useNodeCanvasImagePreview } from '@/composables/node/useNodeCanvasImagePreview'
@@ -770,18 +771,10 @@ export const useLitegraphService = () => {
{
content: 'Save Image',
callback: () => {
const a = document.createElement('a')
const url = new URL(img.src)
url.searchParams.delete('preview')
a.href = url.toString()
a.setAttribute(
'download',
// @ts-expect-error fixme ts strict error
new URLSearchParams(url.search).get('filename')
)
document.body.append(a)
a.click()
requestAnimationFrame(() => a.remove())
const filename = new URLSearchParams(url.search).get('filename')
downloadFile(url.toString(), filename ?? undefined)
}
}
)

View File

@@ -2,15 +2,38 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { downloadFile } from '@/base/common/downloadUtil'
let mockIsCloud = false
vi.mock('@/platform/distribution/types', () => ({
get isCloud() {
return mockIsCloud
}
}))
// Global stubs
const createObjectURLSpy = vi
.spyOn(URL, 'createObjectURL')
.mockReturnValue('blob:mock-url')
const revokeObjectURLSpy = vi
.spyOn(URL, 'revokeObjectURL')
.mockImplementation(() => {})
describe('downloadUtil', () => {
let mockLink: HTMLAnchorElement
let fetchMock: ReturnType<typeof vi.fn>
beforeEach(() => {
mockIsCloud = false
fetchMock = vi.fn()
vi.stubGlobal('fetch', fetchMock)
createObjectURLSpy.mockClear().mockReturnValue('blob:mock-url')
revokeObjectURLSpy.mockClear().mockImplementation(() => {})
// Create a mock anchor element
mockLink = {
href: '',
download: '',
click: vi.fn()
click: vi.fn(),
style: { display: '' }
} as unknown as HTMLAnchorElement
// Spy on DOM methods
@@ -20,7 +43,7 @@ describe('downloadUtil', () => {
})
afterEach(() => {
vi.restoreAllMocks()
vi.unstubAllGlobals()
})
describe('downloadFile', () => {
@@ -35,6 +58,8 @@ describe('downloadUtil', () => {
expect(document.body.appendChild).toHaveBeenCalledWith(mockLink)
expect(mockLink.click).toHaveBeenCalled()
expect(document.body.removeChild).toHaveBeenCalledWith(mockLink)
expect(fetchMock).not.toHaveBeenCalled()
expect(createObjectURLSpy).not.toHaveBeenCalled()
})
it('should use custom filename when provided', () => {
@@ -45,6 +70,8 @@ describe('downloadUtil', () => {
expect(mockLink.href).toBe(testUrl)
expect(mockLink.download).toBe(customFilename)
expect(fetchMock).not.toHaveBeenCalled()
expect(createObjectURLSpy).not.toHaveBeenCalled()
})
it('should extract filename from URL query parameters', () => {
@@ -55,6 +82,7 @@ describe('downloadUtil', () => {
expect(mockLink.href).toBe(testUrl)
expect(mockLink.download).toBe('extracted-image.jpg')
expect(createObjectURLSpy).not.toHaveBeenCalled()
})
it('should use default filename when URL has no filename parameter', () => {
@@ -64,6 +92,7 @@ describe('downloadUtil', () => {
expect(mockLink.href).toBe(testUrl)
expect(mockLink.download).toBe('download.png')
expect(createObjectURLSpy).not.toHaveBeenCalled()
})
it('should handle invalid URLs gracefully', () => {
@@ -74,6 +103,8 @@ describe('downloadUtil', () => {
expect(mockLink.href).toBe(invalidUrl)
expect(mockLink.download).toBe('download.png')
expect(mockLink.click).toHaveBeenCalled()
expect(fetchMock).not.toHaveBeenCalled()
expect(createObjectURLSpy).not.toHaveBeenCalled()
})
it('should prefer custom filename over extracted filename', () => {
@@ -84,6 +115,7 @@ describe('downloadUtil', () => {
downloadFile(testUrl, customFilename)
expect(mockLink.download).toBe(customFilename)
expect(createObjectURLSpy).not.toHaveBeenCalled()
})
it('should handle URLs with empty filename parameter', () => {
@@ -92,6 +124,7 @@ describe('downloadUtil', () => {
downloadFile(testUrl)
expect(mockLink.download).toBe('download.png')
expect(createObjectURLSpy).not.toHaveBeenCalled()
})
it('should handle relative URLs by using window.location.origin', () => {
@@ -101,6 +134,8 @@ describe('downloadUtil', () => {
expect(mockLink.href).toBe(relativeUrl)
expect(mockLink.download).toBe('relative-image.png')
expect(fetchMock).not.toHaveBeenCalled()
expect(createObjectURLSpy).not.toHaveBeenCalled()
})
it('should clean up DOM elements after download', () => {
@@ -111,6 +146,54 @@ describe('downloadUtil', () => {
// Verify the element was added and then removed
expect(document.body.appendChild).toHaveBeenCalledWith(mockLink)
expect(document.body.removeChild).toHaveBeenCalledWith(mockLink)
expect(fetchMock).not.toHaveBeenCalled()
expect(createObjectURLSpy).not.toHaveBeenCalled()
})
it('streams downloads via blob when running in cloud', async () => {
mockIsCloud = true
const testUrl = 'https://storage.googleapis.com/bucket/file.bin'
const blob = new Blob(['test'])
const blobFn = vi.fn().mockResolvedValue(blob)
fetchMock.mockResolvedValue({
ok: true,
status: 200,
blob: blobFn
} as unknown as Response)
downloadFile(testUrl)
expect(fetchMock).toHaveBeenCalledWith(testUrl)
const fetchPromise = fetchMock.mock.results[0].value as Promise<Response>
await fetchPromise
const blobPromise = blobFn.mock.results[0].value as Promise<Blob>
await blobPromise
await Promise.resolve()
expect(blobFn).toHaveBeenCalled()
expect(createObjectURLSpy).toHaveBeenCalledWith(blob)
expect(revokeObjectURLSpy).toHaveBeenCalledWith('blob:mock-url')
expect(mockLink.click).toHaveBeenCalled()
})
it('logs an error when cloud fetch fails', async () => {
mockIsCloud = true
const testUrl = 'https://storage.googleapis.com/bucket/missing.bin'
const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
fetchMock.mockResolvedValue({
ok: false,
status: 404,
blob: vi.fn()
} as unknown as Response)
downloadFile(testUrl)
expect(fetchMock).toHaveBeenCalledWith(testUrl)
const fetchPromise = fetchMock.mock.results[0].value as Promise<Response>
await fetchPromise
await Promise.resolve()
expect(consoleSpy).toHaveBeenCalled()
expect(createObjectURLSpy).not.toHaveBeenCalled()
consoleSpy.mockRestore()
})
})
})