From daae0c6843f3c53686b7fa905b33d53858c7a264 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Sat, 20 Apr 2013 21:54:29 -0400 Subject: [PATCH] Corrected bug whereby multiple override calls would clear __super too early Before this change, __super was set to undefined. However, consider that we have two method overrides---foo and bar---and the code for bar is: this.foo(); this.__super(); foo() would set __super to undefined and so bar cannot invoke its super method unless it stores a reference to __super before invoking foo(). This patch fixes this issue. --- lib/MethodWrappers.js | 13 +++++++------ test/test-class-parent.js | 22 +++++++++++++++++++++- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/lib/MethodWrappers.js b/lib/MethodWrappers.js index 5e8466e..28b761c 100644 --- a/lib/MethodWrappers.js +++ b/lib/MethodWrappers.js @@ -34,17 +34,18 @@ exports.standard = { retval = undefined ; - // the _super property will contain the parent method (we don't - // store the previous value for performance reasons and because, - // during conventional use, it's completely unnecessary) + // the _super property will contain the parent method (store the + // previous value to ensure that calls to multiple overrides will + // be supported) + var psup = context.__super; context.__super = super_method; retval = method.apply( context, arguments ); // prevent sneaky bastards from breaking encapsulation by stealing - // method references (we set to undefined rather than deleting it - // because deletion causes performance degradation within V8) - context.__super = undefined; + // method references and ensure that __super is properly restored + // for nested/multiple override invocations + context.__super = psup; // if the value returned from the method was the context that we // passed in, return the actual instance (to ensure we do not break diff --git a/test/test-class-parent.js b/test/test-class-parent.js index 15619b5..b9bd73b 100644 --- a/test/test-class-parent.js +++ b/test/test-class-parent.js @@ -29,6 +29,7 @@ var common = require( './common' ), // get in the way of our assertions hitMethod = false, hitMethod2 = false, + hitDouble = false, method2Arg = null, Foo = Class.extend( @@ -46,6 +47,11 @@ var common = require( './common' ), return this; }, + + 'virtual double': function() + { + hitDouble = true; + } }), SubFoo = Foo.extend( @@ -59,6 +65,14 @@ var common = require( './common' ), { return this.__super( arg ); }, + + 'override double': function() + { + this.myMethod(); + this.__super(); + + return this; + } }), foo = new Foo(), @@ -81,7 +95,7 @@ assert.equal( hitMethod = hitMethod2 = false; var arg = 'foobar'; -sub_foo.myMethod().myMethod2( arg ); +sub_foo.myMethod().myMethod2( arg ).double(); // myMethod overrides without calling parent assert.equal( @@ -112,3 +126,9 @@ assert['throws']( function() }); }, TypeError, "Methods must be overridden with a Function" ); +// ensure that __super is not cleared after a call to an override +assert.equal( + hitDouble, + true, + "__super is maintained in a stack-like manner" +);