Comfy-Org / Comfy-Org/ComfyUI_frontend

fix: capture oldValue before onUploadStart mutates fileComboWidget.value in useImageUploadWidget

Open
#12,242 1 comment 1 reaction 1 assignee Claimed by @jaeone94 View on GitHub
area:widgets
Dominant language
TypeScript
Stars
2k
Forks
699
Avg merge
1d 7h
Merged PRs (30d)
490

Description

## Summary

In `src/renderer/extensions/vueNodes/widgets/composables/useImageUploadWidget.ts`, the `oldValue` passed to `node.onWidgetChanged` is currently captured inside `onUploadComplete`, **after** `onUploadStart` has already mutated `fileComboWidget.value` to the transient upload-start filename (e.g. `"image.png"`). This means `oldValue` reflects the temporary in-progress value rather than the real pre-upload value (e.g. `"missing.png"`).

## Expected Behavior

`oldValue` should be captured at the start of the upload flow (inside or alongside `onUploadStart`, via a closure ref similar to how `rollback` is stored), so that `node.onWidgetChanged` receives the true pre-upload widget value.

## Why It Matters

Currently this is harmless because `clearWidgetRelatedErrors` only consumes `newValue`. However, if any future consumer relies on `oldValue` (e.g. for undo/redo, audit logging, or diffing), it would receive the transient value instead of the real prior value. This also diverges from `uploadAudio.ts` in the same PR, which captures `oldValue` cleanly because it has no `onUploadStart` step.

## Suggested Fix

Store the pre-upload value in a closure ref (alongside `rollback`) inside `onUploadStart`, then use it in `onUploadComplete` when calling `node.onWidgetChanged`.

```ts
let preUploadValue: IComboWidget['value'] | undefined

onUploadStart: (files) => {
if (files.length > 0) {
preUploadValue = fileComboWidget.value // capture real old value here
const prev = fileComboWidget.value
fileComboWidget.value = files[0].name
rollback = () => {
fileComboWidget.value = prev
}
}
},
onUploadComplete: (output) => {
rollback = undefined
// ...
const oldValue = preUploadValue // use real pre-upload value
preUploadValue = undefined
// ...
node.onWidgetChanged?.(fileComboWidget.name, newValue, oldValue, fileComboWidget)
}
```

The unit test in `useImageUploadWidget.test.ts` should also be updated to invoke `onUploadStart` before `onUploadComplete` to better reflect the real upload flow.

## References

- PR: https://github.com/Comfy-Org/ComfyUI_frontend/pull/12212
- Review comment: https://github.com/Comfy-Org/ComfyUI_frontend/pull/12212#discussion_r3237797742
- Requested by: @jaeone94

┆Issue is synchronized with this [Notion page](https://www.notion.so/Issue-12242-fix-capture-oldValue-before-onUploadStart-mutates-fileComboWidget-value-in-useImageU-3606d73d365081828323ccf8ff9e8ae7) by [Unito](https://www.unito.io)

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.