Skip to content

Update the frontend to support async nodes. #4382

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 20 commits into from
Jul 23, 2025
Merged

Update the frontend to support async nodes. #4382

merged 20 commits into from
Jul 23, 2025

Conversation

guill
Copy link
Member

@guill guill commented Jul 8, 2025 •
edited by sync-by-unito bot
Loading

Also fixes a number of bugs with subgraphs

This should NOT be merged until the relevant PR to core is merged: comfyanonymous/ComfyUI#8830

┆Issue is synchronized with this Notion page by Unito

guill added 6 commits July 2, 2025 12:16
@guill
Update the frontend to support async nodes.
aa5fa81
@guill
Address PR feedback
a70a81c
@guill
Add functions for mapping node locations to an id
c8371d6 
These locations differ from either local ids (like `5`) or execution ids
(like `3:4:5`) in that all nodes which are 'linked' such that changes to
one node will apply changes to other nodes will map to the same id. This
is specifically relevant for multiple instances of the same subgraph.

These IDs will actually get used in a followup commit
@guill
Progress bars work in subgraphs
f6c65d3
@guill
Fix bug with browser tab title
f076a1c
@guill
Add a feature flags message to reduce bandwidth
132a9db
@guill guill mentioned this pull request Jul 8, 2025
Closed
guill
guill commented Jul 8, 2025
View reviewed changes
@guill guill marked this pull request as ready for review July 10, 2025 22:41
@guill guill requested review from Yorha4D, KarryCharon, shinshin86 and a team as code owners July 10, 2025 22:41
webfiltered
webfiltered requested changes Jul 10, 2025
View reviewed changes
Copy link
Contributor

@webfiltered webfiltered left a comment

Choose a reason for hiding this comment

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

Thassalottacode. Looks great so far, but it is a very large PR that could be split up into its distinct features.

Will need to come back to finish up, but have paused and just left the raw notes made as I flicked through.

christian-byrne
christian-byrne reviewed Jul 11, 2025
View reviewed changes
@guill guill mentioned this pull request Jul 13, 2025
Merged
@webfiltered
Copy link
Contributor

webfiltered commented Jul 14, 2025 •
edited
Loading

p.s. you can just revert the Update locales commit *before rebasing - it will regenerate and fix its own merge conflicts. 👍

@github-actions GitHub Actions
Copy link

Fails
🚫

🧪 E2E Test Coverage Missing

All changes should be covered under E2E testing. Please add or update browser tests.
🚫

📹 Visual Documentation Missing

Please add a screen recording or screenshot:

  • GitHub: Drag & drop media to PR description
  • YouTube: Add YouTube link

Generated by 🚫 dangerJS against aae20d9

@guill guill requested a review from webfiltered July 18, 2025 22:28
@github-actions GitHub Actions
Copy link

github-actions bot commented Jul 18, 2025 •
edited
Loading

⚠️ Warnings

⚠️ Warning: E2E Test Coverage Missing

If this PR modifies behavior that can be covered by browser-based E2E tests, those tests are required. PRs lacking applicable test coverage may not be reviewed until added. Please add or update browser tests to ensure code quality and prevent regressions.

⚠️ Warning: Visual Documentation Missing

If this PR changes user-facing behavior, visual proof (screen recording or screenshot) is required. PRs without applicable visual documentation may not be reviewed until provided.
You can add it by:

  • GitHub: Drag & drop media directly into the PR description

  • YouTube: Include a link to a short demo

@guill guill requested a review from christian-byrne July 18, 2025 22:28
webfiltered
webfiltered requested changes Jul 19, 2025
View reviewed changes
Copy link
Contributor

@webfiltered webfiltered left a comment

Choose a reason for hiding this comment

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

This is 90% just removing type assertions. Using 'as' does much more than e.g. specifying a function return type. The latter holds you to the contract - as skips a lot of type checking.

All except one of these should be click-to-commit batchable, for your convenience.

@guill guill requested a review from webfiltered July 20, 2025 02:18
christian-byrne
christian-byrne approved these changes Jul 23, 2025
View reviewed changes
Copy link
Contributor

@christian-byrne christian-byrne left a comment

Choose a reason for hiding this comment

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

LGTM!

@webfiltered webfiltered merged commit 7eb3eb2 into main Jul 23, 2025
12 checks passed
@webfiltered webfiltered deleted the js/async_nodes branch July 23, 2025 03:46
@christian-byrne christian-byrne mentioned this pull request Jul 23, 2025
Open
@webfiltered webfiltered mentioned this pull request Jul 23, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@christian-byrne christian-byrne christian-byrne approved these changes

@webfiltered webfiltered webfiltered approved these changes

@Yorha4D Yorha4D Awaiting requested review from Yorha4D Yorha4D is a code owner

@KarryCharon KarryCharon Awaiting requested review from KarryCharon KarryCharon is a code owner

@shinshin86 shinshin86 Awaiting requested review from shinshin86 shinshin86 is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@guill @webfiltered @christian-byrne