Skip to content
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.

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

LGraphNode ES6 class conversion #68

Merged
merged 1 commit into from
Aug 11, 2024
Merged

LGraphNode ES6 class conversion #68

merged 1 commit into from
Aug 11, 2024

Conversation

huchenlei
Copy link
Member

No description provided.

@huchenlei
Copy link
Member Author

huchenlei commented Aug 9, 2024

@pythongosssss Can you take a look why some group tests fails?

To reproduce, checkout #68 and npm link it to be used for ComfyUI_frontend with Comfy-Org/ComfyUI_frontend#334.

For following expectation, it seems like widgets remains to be an array that does not hold 'value' key.

    // This will use a primitive widget named 'value'
    expect(group.widgets.length).toBe(1)
    expect(group.widgets['value'].value).toBe('positive')

I am not very familar with the trick here in groupNode.

@pythongosssss
Copy link
Member

pythongosssss commented Aug 10, 2024

@huchenlei
Looks like this is due to nodes that dont have titles now being called "Unnamed" instead of using their type name
This is due to classes extending the base class not calling the super ctor correctly

e.g. add a primitive

Before:
image

With that PR:
image

@pythongosssss
Copy link
Member

The nodes aren't passing the ctor arguments on the call to super() on the UI PR

@huchenlei huchenlei merged commit fcca975 into master Aug 11, 2024
4 checks passed
@huchenlei huchenlei deleted the LGraphNode branch August 11, 2024 14:06
huchenlei added a commit that referenced this pull request Aug 16, 2024
huchenlei added a commit that referenced this pull request Aug 16, 2024
huchenlei added a commit that referenced this pull request Aug 28, 2024
huchenlei added a commit that referenced this pull request Aug 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants