Comfy-Org / ComfyUI_frontend

Official front-end implementation of ComfyUI
https://www.comfy.org/
GNU General Public License v3.0
446 stars 54 forks source link

widgetInput extension erases any other extension's onGraphConfigured overrides #784

Open devingfx opened 6 days ago

devingfx commented 6 days ago

This core extension do not safely override the ComfyNode.protptype.onGraphConfigured by saving a reference to any previously existing method, therefore any other extension safely hijacking this method get no chance to be executed if it get registered before this one... https://github.com/Comfy-Org/ComfyUI_frontend/blob/b8bdba0bcc6112f7ae8f0df0cc355e32de753c91/src/extensions/core/widgetInputs.ts#L751-L753

This should be written as anywhere else a method is hijacked:

const origOnGraphConfigured = nodeType.prototype.onGraphConfigured;
nodeType.prototype.onGraphConfigured = function () {
    const r = origOnGraphConfigured ? origOnGraphConfigured.apply(this, arguments) : undefined;
devingfx commented 6 days ago

BTW you may define a standardised helper for js extension developpers to hijack methods...

One like this:


/**
 * Method inheritance helper, replaces a method on an Object and keep track
 * of the original method to permit it's call.
 * The replacement method signature is 
 * ```js
 * function( _super, ...orginalArguments )
 * {
 *  // Do whatever's needed before
 *  
 *  // Optionally call the old method
 *  //(use `?.` to avoid the call if orginal method did not existed)
 *  const retValue = _super?.call( this, ...originalArguments )
 *  
 *  // Do whatever's needed after (retValue == null if orginal method did not existed)
 *  
 *  return retValue
 * }
 * ```
 * /!\ You need to use a `function(){}` and not an arrow function `()=>{}` for replacement
 * to get `this` pointing to the original Object
 *
 * @param {Object} obj The Object where the method to inherit stands
 * @param {string} methodName The name of the method
 * @param {function} replacement The replacement method, signature: function( _super, ...orginalArguments )
 * 
 * @example
 * inherit( nodeType.prototype, 'onConnectionsChange',
 *  function( _super, ...args )
 *  {
 *      return _super?.apply(this, args)
 *  })
 */
export const inherit = ( obj, methodName, replacement )=> {
    const _super = obj[methodName]
    obj[methodName] = function( ...args )
    {
        return replacement.apply( this, [ _super, ...args ] )
    }
}

This issue's case example usage would be:

inherit( nodeType.prototype, 'onGraphConfigured', 
function onGraphConfigured( _super ) {
      const r = _super?.call( this )
      if (!this.inputs) return r

      for (const input of this.inputs) {
        if (input.widget) {
          if (!input.widget[GET_CONFIG]) {
            input.widget[GET_CONFIG] = () =>
              getConfig.call(this, input.widget.name)
          }

          // Cleanup old widget config
          if (input.widget.config) {
            if (input.widget.config[0] instanceof Array) {
              // If we are an old converted combo then replace the input type and the stored link data
              input.type = 'COMBO'

              const link = app.graph.links[input.link]
              if (link) {
                link.type = input.type
              }
            }
            delete input.widget.config
          }

          const w = this.widgets.find((w) => w.name === input.widget.name)
          if (w) {
            hideWidget(this, w)
          } else {
            convertToWidget(this, input)
          }
        }
      }
    }
)