1
0
Fork 0

Fix setting public properties

- This currently only works with ES5 engines
  - Fallback to follow so this will work with pre-ES5 engines
- As such, I do not recommend using this commit outside of ES5-compliant engines that work properly with getters/setters
  - This does NOT include IE8, as IE8 only works with getters/setters on DOM elements
closure/master
Mike Gerwitz 2011-03-02 23:21:10 -05:00
parent 74c2fc57c1
commit bc9e2bb7b2
2 changed files with 93 additions and 0 deletions

View File

@ -537,6 +537,25 @@ function attachPropInit( prototype, properties, members )
// initialize the value with a clone to ensure that they do // initialize the value with a clone to ensure that they do
// not share references (and therefore, data) // not share references (and therefore, data)
this[ prop ] = util.clone( prop_pub[ prop ] ); this[ prop ] = util.clone( prop_pub[ prop ] );
( function( prop )
{
var inst = this;
// public properties, when set internally, must forward to the
// actual variable
inst_props.__defineSetter__( prop, function( val )
{
inst[ prop ] = val;
} );
// since we're defining a setter, we'll need to define a getter
// to return the value, or we'll simply return undefined
inst_props.__defineGetter__( prop, function()
{
return inst[ prop ];
} );
} ).call( this, prop );
} }
var methods_protected = members[ 'protected' ], var methods_protected = members[ 'protected' ],

View File

@ -50,9 +50,22 @@ var common = require( './common' ),
// protected/private properties (for testing purposes) // protected/private properties (for testing purposes)
return this[ name ]; return this[ name ];
}, },
/**
* Allows us to set a value from within the class
*/
'public setValue': function( name, value )
{
this[ name ] = value;
},
})(); })();
/**
* Public members are the only members added to the instance's prototype to be
* accessible externally
*/
( function testPublicMembersAreAccessbileExternally() ( function testPublicMembersAreAccessbileExternally()
{ {
assert.equal( assert.equal(
@ -69,6 +82,67 @@ var common = require( './common' ),
} )(); } )();
/**
* For reasons that are discussed in the next test (writing to public
* properties), we need to make sure public members are available internally.
* Actually, we don't need to test public methods, really, but it's in there for
* good measure. Who knows what bugs may be introduced in the future.
*
* This ensures that the getter is properly proxying the value to us.
*/
( function testPublicMembersAreAccessibleInternally()
{
assert.equal(
foo.getProp( 'pub' ),
pub,
"Public properties are accessible internally"
);
assert.equal(
foo.getProp( 'pubf' )(),
pub,
"Public methods are accessible internally"
);
} )();
/**
* This may sound like an odd test, but it's actually very important. Due to how
* private/protected members are implemented, it compromises public members. In
* fact, public members would not work internally without what is essentially a
* proxy via setters.
*
* This test is to ensure that the setter is properly forwarding writes to the
* object within the prototype chain containing the public values. Otherwise,
* setting the value would simply mask it in the prototype chain. The value
* would appear to have changed internally, but when accessed externally, the
* value would still be the same. That would obviously be a problem ;)
*/
( function testPublicPropertiesAreWritableInternally()
{
var val = 'moomookittypoo';
// start by setting the value
foo.setValue( 'pub', val );
// we should see that change internally...
assert.equal(
foo.getProp( 'pub' ),
val,
"Setting the value of a public property internally should be " +
"observable /internally/"
);
// ...as well as externally
assert.equal(
foo.pub,
val,
"Setting the value of a public property internally should be " +
"observable /externally/"
);
} )();
( function testProtectedAndPrivateMembersAreNotAccessibleExternally() ( function testProtectedAndPrivateMembersAreNotAccessibleExternally()
{ {
assert.equal( assert.equal(