Comfy-Org / Comfy-Org/ComfyUI_frontend
fix: capture oldValue before onUploadStart mutates fileComboWidget.value in useImageUploadWidget
- 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
Assessment
This issue has not been assessed yet.