From 8b8a08b7dc0cc0e8fe3233a9de0954bb9cd34afd Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Tue, 29 Apr 2014 10:47:12 -0400 Subject: [PATCH] Subtypes of prototype subtypes now work correctly --- lib/ClassBuilder.js | 3 ++- lib/class.js | 27 +++++++++++++++++++++++---- test/Class/InteropTest.js | 36 ++++++++++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 5 deletions(-) diff --git a/lib/ClassBuilder.js b/lib/ClassBuilder.js index 69c426b..59d9a10 100644 --- a/lib/ClassBuilder.js +++ b/lib/ClassBuilder.js @@ -365,7 +365,8 @@ exports.prototype.build = function extend( _, __ ) // if we are inheriting from a prototype, we must make sure that all // properties initialized by the ctor are implicitly public; otherwise, // proxying will fail to take place - if ( !( prototype instanceof exports.ClassBase ) ) + // TODO: see Class.isA TODO + if ( prototype.___$$vis$$ === undefined ) { this._discoverProtoProps( prototype, prop_init ); } diff --git a/lib/class.js b/lib/class.js index b97fd5c..9410dfb 100644 --- a/lib/class.js +++ b/lib/class.js @@ -148,6 +148,9 @@ module.exports.use = function( traits ) }; +var _dummyclass = { prototype: {} }; +var _dummyinst = { constructor: { prototype: {} } }; + /** * Determines whether the provided object is a class created through ease.js * @@ -157,9 +160,18 @@ module.exports.use = function( traits ) */ module.exports.isClass = function( obj ) { - obj = obj || {}; + obj = obj || _dummyclass; - return ( obj.prototype instanceof ClassBuilder.ClassBase ) + if ( !obj.prototype ) + { + return false; + } + + // TODO: this just checks one of many internal fields; we need something + // more formal (cannot use a strict ClassBase check because it will fail + // when extending prototypes) + return ( ( obj.prototype.___$$vis$$ !== undefined ) + || ( obj.prototype instanceof ClassBuilder.ClassBase ) ) ? true : false ; @@ -177,9 +189,16 @@ module.exports.isClass = function( obj ) */ module.exports.isClassInstance = function( obj ) { - obj = obj || {}; + obj = obj || _dummyinst; - return ( obj instanceof ClassBuilder.ClassBase ) + if ( !obj.constructor || !obj.constructor.prototype ) + { + return false; + } + + // TODO: see isClass TODO + return ( ( obj.constructor.prototype.___$$vis$$ !== undefined ) + || ( obj instanceof ClassBuilder.ClassBase ) ) ? true : false; }; diff --git a/test/Class/InteropTest.js b/test/Class/InteropTest.js index 1bf75a0..c099f78 100644 --- a/test/Class/InteropTest.js +++ b/test/Class/InteropTest.js @@ -319,6 +319,42 @@ require( 'common' ).testCase( }, + /** + * This is a regression test for an interesting (and particularily + * nasty) bug for a situation that is probably reasonably rare. The + * original check for a non-class supertype checked whether the + * supertype was an instance of the internal base class. While this + * works, it unforunately causes problems for subtypes of the class that + * extended the prototype---the check will fail, since there is no + * ClassBase in the prototype chain. + * + * This resulted in it processing the class fields, which ended up + * overwriting ___$$vis$$, which clobbered all the methods. Doh. + */ + 'Subtypes of prototype subtypes yield stable classes': function() + { + function P() {}; + + // sub-subtype of P + var expected = {}; + var C = this.Class.extend( P, {} ).extend( + { + foo: function() { return expected; } + } ); + + var inst = C(); + + // this should be recognized as a class (prior to the fix, it was + // not), and inst should be an instance of a class + this.assertOk( this.Class.isClass( C ) ); + this.assertOk( this.Class.isClassInstance( inst ) ); + this.assertOk( this.Class.isA( C, inst ) ); + + // before the fix, foo is undefined since ___$$vis$$ was clobbered + this.assertStrictEqual( inst.foo(), expected ); + }, + + /** * When prototypally extending a class, it is not wise to invoke the * constructor (just like ease.js does not invoke the constructor of