Skip to content

Conversation

AustinMroz
Copy link
Collaborator

@AustinMroz AustinMroz commented Oct 13, 2025

Under some infrequent circumstances, audioWidget.value is not a string. Presumably if a workflow is loaded with a saved file choice that does not exist and the value is set to undefined instead.

Instead of a cast, a proper type guard is used and the widget is not updated if the new value is not a string.

┆Issue is synchronized with this Notion page by Unito

@dosubot dosubot bot added the size:XS This PR changes 0-9 lines, ignoring generated files. label Oct 13, 2025
Copy link

github-actions bot commented Oct 13, 2025

🎨 Storybook Build Status

Build completed successfully!

⏰ Completed at: 10/13/2025, 05:39:13 PM UTC

🔗 Links


🎉 Your Storybook is ready for review!

Copy link

github-actions bot commented Oct 13, 2025

🎭 Playwright Test Results

⚠️ Tests passed with flaky tests

⏰ Completed at: 10/13/2025, 05:53:43 PM UTC

📈 Summary

  • Total Tests: 498
  • Passed: 466 ✅
  • Failed: 0
  • Flaky: 2 ⚠️
  • Skipped: 30 ⏭️

📊 Test Reports by Browser

  • chromium: View Report • ✅ 457 / ❌ 0 / ⚠️ 2 / ⏭️ 30
  • chromium-2x: View Report • ✅ 2 / ❌ 0 / ⚠️ 0 / ⏭️ 0
  • chromium-0.5x: View Report • ✅ 1 / ❌ 0 / ⚠️ 0 / ⏭️ 0
  • mobile-chrome: View Report • ✅ 6 / ❌ 0 / ⚠️ 0 / ⏭️ 0

🎉 Click on the links above to view detailed test results for each browser configuration.

if (typeof audioWidget.value !== 'string') return
audioUIWidget.element.src = api.apiURL(
getResourceURL(...splitFilePath(audioWidget.value as string))
getResourceURL(...splitFilePath(audioWidget.value))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How resilient is the splitFilePath function? Can i handle empty strings

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It won't throw an error, but it'll still result in a request being made for an empty filename.

Directly un-setting audioUIWidget.element.src could be beneficial, but I'm reasonably confident this invalid typing can only occur on workflow load where such a change doesn't make a difference.

Copy link

@luke-mino-altherr luke-mino-altherr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@christian-byrne christian-byrne added needs-backport Fix/change that needs to be cherry-picked to the current feature freeze branch 1.28 labels Oct 13, 2025
@christian-byrne christian-byrne merged commit e59d2dd into main Oct 13, 2025
38 checks passed
@christian-byrne christian-byrne deleted the austin/fix-uploadAudio-cast branch October 13, 2025 18:52
github-actions bot pushed a commit that referenced this pull request Oct 13, 2025
Under some infrequent circumstances, `audioWidget.value` is not a
string. Presumably if a workflow is loaded with a saved file choice that
does not exist and the value is set to undefined instead.

Instead of a cast, a proper type guard is used and the widget is not
updated if the new value is not a string.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-6041-Use-type-check-instead-of-cast-28b6d73d365081249353f4f905769f89)
by [Unito](https://www.unito.io)
@comfy-pr-bot
Copy link
Member

@AustinMroz Successfully backported to #6042

@arjansingh arjansingh mentioned this pull request Oct 14, 2025
arjansingh added a commit that referenced this pull request Oct 14, 2025
## What's Changed

### 🚀 Features
- Add MediaAssetCard presentation components (#5878)
- Make Vue nodes' outputs/previews responsively sized and work with node
resizing (#5970)
- Allow connection to subgraphIOs in vue mode (#6016)
- Add distribution detection pattern (#6028)
- Make nodeData.widgets reactive (#6019)

### 🐛 Bug Fixes
- Fix FLOAT widget incrementing broken & disabled state styles on widget
number input (Vue) (#6036)
- Fix Vue node border styles in different states (executing, error,
selected) (#6018)
- Fix Vue node opacity conditions (user node opacity, bypass state,
muted state) (#6022)
- Fix: emit layout change for batch node bounds (#5939)
- Safer restoration of widgets_values on subgraph nodes (#6015)
- Fix(execution): reset progress state after runs to unfreeze tab
title/favicon (main) (#6026)
- Use type check instead of cast (#6041)

### 🎨 Style & Design
- [style] match widget border/outline styles with designs (#6021)
- [style] make Vue widget/slot/label width and spacing align with
designs (#6023)

### ♿ Accessibility
- Add aria labels on vue node widgets (#6032)

### 🔧 Maintenance
- [refactor] adjust Vue node fixtures to not be coupled to Litegraph
(#6033)
- [refactor] reorganize devtools test nodes into modules (#6020)

### 🧪 Testing
- [test] add browser test for control+a selection of Vue nodes (#6031)

### 🔄 CI/CD
- [ci] fix update locales workflow (#6017)

**Full Changelog**:
v1.29.1...v1.29.2

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-6045-1-29-2-28c6d73d3650817a8c36fba944ce69a8)
by [Unito](https://www.unito.io)

---------

Co-authored-by: arjansingh <[email protected]>
Co-authored-by: github-actions <[email protected]>
christian-byrne pushed a commit that referenced this pull request Oct 14, 2025
Backport of #6041 to `core/1.28`

Automatically created by backport workflow.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-6042-backport-1-28-Use-type-check-instead-of-cast-28b6d73d365081c890abe16efbff7c2b)
by [Unito](https://www.unito.io)

Co-authored-by: AustinMroz <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1.28 needs-backport Fix/change that needs to be cherry-picked to the current feature freeze branch size:XS This PR changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants