From fe853b505bcb5ac25d3bf3097a607257fddb1aeb Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Thu, 16 Dec 2010 23:18:30 -0500 Subject: [PATCH] [*] Properties are no longer shared between class instances - Properties were previously shared on the prototype level, acting as though they were static class properties, which can cause some nasty bugs --- lib/class.js | 28 ++++++++++++++++++++++++---- test/test-class-extend.js | 31 +++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 4 deletions(-) diff --git a/lib/class.js b/lib/class.js index 72e2968..fd37b17 100644 --- a/lib/class.js +++ b/lib/class.js @@ -93,13 +93,23 @@ var extend = ( function( extending ) var result_data = { abstractMethods: ( base.abstractMethods || [] ).slice() }; - util.propCopy( props, prototype, result_data ); + + var properties = {}; + util.propCopy( props, prototype, result_data, { + property: function( name, value ) + { + properties[ name ] = value; + + // also add to prototype + prototype[ name ] = value; + }, + } ); // reference to the parent prototype (for more experienced users) prototype.parent = base.prototype; // set up the new class - var new_class = create_ctor( result_data, extending ); + var new_class = create_ctor( result_data, properties ); setup_props( new_class, result_data ); new_class.prototype = prototype; @@ -126,16 +136,26 @@ var extend = ( function( extending ) * * @return {Function} constructor */ - function create_ctor( result_data ) + function create_ctor( result_data, properties ) { // concrete class if ( result_data.abstractMethods.length === 0 ) { return function() { + // initialize each of the properties for this instance to + // ensure we're not sharing prototype values + var value; + for ( prop in properties ) + { + // initialize the value with a clone to ensure that they do + // not share references (and therefore, data) + this[ prop ] = util.clone( properties[ prop ] ); + } + + // call the constructor, if one was provided if ( this.__construct instanceof Function ) { - // call the constructor this.__construct.apply( this, arguments ); } }; diff --git a/test/test-class-extend.js b/test/test-class-extend.js index 49e99f0..4d61083 100644 --- a/test/test-class-extend.js +++ b/test/test-class-extend.js @@ -135,3 +135,34 @@ assert.throws( function() }); }, TypeError, "Cannot override property with a method" ); + +var AnotherFoo = Class.extend( +{ + arr: [], + obj: {}, +}); + +var Obj1 = new AnotherFoo(), + Obj2 = new AnotherFoo(); + +Obj1.arr.push( 'one' ); +Obj2.arr.push( 'two' ); + +Obj1.obj.a = true; +Obj2.obj.b = true; + +// to ensure we're not getting/setting values of the prototype (=== can also be +// used to test for references, but this test demonstrates the functionality +// that we're looking to ensure) +assert.ok( + ( ( Obj1.arr[ 0 ] === 'one' ) && ( Obj2.arr[ 0 ] === 'two' ) ), + "Multiple instances of the same class do not share array references" +); + +assert.ok( + ( ( ( Obj1.obj.a === true ) && ( Obj1.obj.b === undefined ) ) + && ( ( Obj2.obj.a === undefined ) && ( Obj2.obj.b === true ) ) + ), + "Multiple instances of the same class do not share object references" +); +