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

onConnectInput: Uncaught TypeError: this is undefined #94

Open
Codes4Fun opened this issue Jul 27, 2024 · 7 comments
Open

onConnectInput: Uncaught TypeError: this is undefined #94

Codes4Fun opened this issue Jul 27, 2024 · 7 comments

Comments

@Codes4Fun
Copy link

When I try to connect an image output to GetImageSizeAndCount input, I get a javascript error:

jsnodes.js:103 Uncaught TypeError: Cannot read properties of undefined (reading 'outputs')
    at nodeType.onConnectInput (jsnodes.js:103:11)
    at nodeType.onConnectInput (widgetInputs.js:454:30)
    at LGraphNode.connect (litegraph.core.js:4311:30)
    at LGraphCanvas.processMouseUp (litegraph.core.js:6754:54)
    at LGraphCanvas.processMouseUp (undoRedo.js:160:27)

I'm looking at the code involved and it is the latest comfyui and kjnodes.

This looks to me to be an error in comfyui, but your change is more recent

https://github.com/comfyanonymous/ComfyUI/blob/93000580261971971ebb12aff03f6bc3ce30a9f2/web/extensions/core/widgetInputs.js#L454

this.outputs[1]["name"] = "width"

what is happening is that in comfy's onConnectInput, this is passed as an argument to your function, instead of using apply like it does for other function calls.

your change from a months ago though, went from using the first parameter targetSlot to this which is undefined.

83095a5

@kijai
Copy link
Owner

kijai commented Jul 28, 2024

I am on very latest ComfyUI version and I can't reproduce this error.

At one point it for some reason stopped working with targetSlot, which used to return whole node and now only returns the slot index, so I changed it to this which still works fine for me. In what exact situation does it error out?

@Codes4Fun
Copy link
Author

Codes4Fun commented Jul 28, 2024

it errors out when I drag out an image output from a node and drop it onto the image input of a GetImageSizeAndCount node.

for some reason, that I cannot explain this became undefined, there maybe something else going on... even if it doesn't use apply, this should at least point to the window, so maybe something is happening further in the call stack. I'll dig into it more

the issue I found happens both in chrome and firefox

@kijai
Copy link
Owner

kijai commented Jul 28, 2024

it errors out when I drag out an image output from a node and drop it onto the image input of a GetImageSizeAndCount node.

for some reason, that I cannot explain this became undefined, there maybe something else going on... even if it doesn't use apply, this should at least point to the window, so maybe something is happening further in the call stack. I'll dig into it more

the issue I found happens both in chrome and firefox

this should simply point to the node itself. It's possible there's some other custom node interfering.

@Codes4Fun
Copy link
Author

that is the ideal value of this but if you look at the calling function here:

https://github.com/comfyanonymous/ComfyUI/blob/93000580261971971ebb12aff03f6bc3ce30a9f2/web/extensions/core/widgetInputs.js#L454

			const v = onConnectInput?.(this, arguments);

it doesn't use apply, if you look at the other function calls being defined before it in the same code, they use apply, for example:

			const r = origOnInputDblClick ? origOnInputDblClick.apply(this, arguments) : undefined;

since the onConnectInput doesn't use apply, it ends up passing this as an argument.

I can't explain why it doesn't produce an error for you, except maybe your call stack is different?

@Codes4Fun
Copy link
Author

I think I should close this, after doing an update the problem is fixed, the reason is fixed is because your node is defined first which reverses the order.

before I close this though, I just noticed your code has a similar issue, it does not use apply... maybe that is the standard? but then you are also using this, all it takes is for the order to change again or some other custom node to end up in the stack and the same problem can occur

@Codes4Fun
Copy link
Author

so even though the issue is harmless now, this is what happens when kjnodes onConnectInput calls widgetinputs onConnectInput
this_undefined

you can see the Call Stack is kjnodes calling widgetinputs onConnectInput.
under Local, this becomes undefined and targetSlot is a ComfyNode, type is an Arguments, and output originNode originSlot are undefined.

@kijai
Copy link
Owner

kijai commented Jul 28, 2024

Yeah I think you are absolutely correct, somehow I missed that, thanks.

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

No branches or pull requests

2 participants