Dynamically overriding class methods will trigger recursive endless loop

[antanas.arvasevicius]

Hi, Isomorphic,

due a fact that SmartClient do not allow to override methods in a consistent way ( impossible to call parent method if any method exists on class), we are using this technique:

var superMethod = isc.ListGrid.getInstanceProperty('xxx');
isc.ListGrid.addMethods({
    xxx: function () {
        // do own stuff..        
        superMethod.apply(this, arguments);
    }
}

It worked pretty well till overriden method is called first in class graph (e.g. var a = isc.ListGrid.create({}); a.doSomething(); )
but if we have a class Menu extends ListGrid; and variable var b = isc.Menu.create({}); b.doSomething(); it will trigger endless recursion.

Found a problem in: ISC_Core.js: invokeSuper(...) method:

if (nativeArguments && nativeArguments.callee != null &&
    nativeArguments.callee != methodCallingSuper)
{
    //this.logWarn("recursion detected: to continue current super chain caller" +
    //             " should be: " + this.echoLeaf(methodCallingSuper) +
    //             " but caller is: " + this.echoLeaf(nativeArguments.callee));
    methodCallingSuper = isc.Class._getOriginalMethod(methodName, this);
    nextProto = staticSuper ? this : this.getPrototype();
}

nativeArguments.callee != methodCallingSuper

This condition is always true, because callee do not match to original method in ListGrid, and it causes endless recursion because nextProto will direct to the same class.

Could you please tell me how to avoid this recursion? Or how to do overriding in proper way (I need to override certain method in SmartClient's default class and call back it's parent implementation too)?

A solution would be to use property '_originalMethod' which will point to original method as `methodCallingSuper = isc.Class._getOriginalMethod(methodName, this);` will do resolving to real, but I do not think it's proper solution.

[Isomorphic]

Properly and efficiently implementing Super() is extremely complicated, when you consider the tough cases like inter-recursion (two or more nested super calls ongoing in the same instance). You are correct that your solution doesn't cover all cases.

You should either:

1. just create a subclass and use it everywhere, rather than trying to change the base implementation

2. do what you're trying to do in the style of a patch - completely replace the original method with a new method that includes the behavior of the old, plus whatever new behavior you wanted

[antanas.arvasevicius]

Thanks for your fast response.
It's easy to say use your subclass everythere then 100k+ lines of code now exists with native classes :)

Second approach is not viable because some methods use internal properties and methods of class (which is obfuscated in prod.)

As a feature suggestion would be nice to have some kind of isc.ListGrid.overrideMethods({ xxx: function (super, a, b, c, d) { do whatever; super.apply(this, [a,b,c,d]); } });

I've got a work around: "isc.Class._getOriginalMethod(methodName, this);" is doing resolving and checks instance method's property "_originalMethod" and I could do like this:

var superMethod = isc.ListGrid.getInstanceProperty('xxx');

isc.ListGrid.addMethods({
    xxx: function () {
        // do own stuff..        
        superMethod.apply(this, arguments);
    }
}

isc.ListGrid.getInstanceProperty('xxx')._originalMethod = isc.ListGrid.getInstanceProperty('xxx');

Then condition: nativeArguments.callee != methodCallingSuper will be false. As it's truly a hack, but also "callee" is deprecated long time ago too.. :) I hope this solution would work for me, but could you clarify me on issues what this hack could have? What come first in mind that will be issues if method is in use with Observers. Thanks.

[Isomorphic]

To migrate a large body of code, use search and replace.

To patch a method with obfuscated variables, start from the obfuscated code of the function (just call toString()) and add your code.

Sorry, the forums are not a good place for a in-depth explanation of how complex it is to implement Super and all the considerations that went into the current implementation (which would further include browser bugs, and quirks of JS engines that make certain things slower than would be expected..).

The code is there for you to read and analyze if you wish. But we would strongly, strongly recommend you take one of the approaches we've covered here rather than trying to make Super (and all related systems, such as observation) compatible with your approach of pseudo-patching the framework.

There's also of course a third approach of contributing whatever you're building, if it's generally useful.