Prototype supertype property proxy fix
This was a nasty bug that I discovered when working on a project at work probably over a year ago. I had worked around it, but ease.js was largely stalled at the time; with it revitalized by GNU, it's finally getting fixed. See test case comments for more information.newmaster
parent
62414a4294
commit
c2e7fc5071
|
@ -360,6 +360,14 @@ exports.prototype.build = function extend( _, __ )
|
|||
// increment class identifier
|
||||
this._classId++;
|
||||
|
||||
// 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 ) )
|
||||
{
|
||||
this._discoverProtoProps( prototype, prop_init );
|
||||
}
|
||||
|
||||
// build the various class components (XXX: this is temporary; needs
|
||||
// refactoring)
|
||||
try
|
||||
|
@ -465,6 +473,55 @@ exports.prototype._getBase = function( base )
|
|||
};
|
||||
|
||||
|
||||
/**
|
||||
* Discovers public properties on the given object and create an associated
|
||||
* property
|
||||
*
|
||||
* This allows inheriting from a prototype that uses properties by ensuring
|
||||
* that we properly proxy to that property. Otherwise, assigning the value
|
||||
* on the private visibilit object would mask the underlying value rather
|
||||
* than modifying it, leading to an inconsistent and incorrect state.
|
||||
*
|
||||
* This assumes that the object has already been initialized with all the
|
||||
* properties. This may not be the case if the prototype constructor does
|
||||
* not do so, in which case there is nothing we can do.
|
||||
*
|
||||
* This does not recurse on the prototype chian.
|
||||
*
|
||||
* For a more detailed description of this issue, see the interoperability
|
||||
* test case for classes.
|
||||
*
|
||||
* @param {Object} obj object from which to gather properties
|
||||
* @param {Object} prop_init destination property object
|
||||
*
|
||||
* @return {undefined}
|
||||
*/
|
||||
exports.prototype._discoverProtoProps = function( obj, prop_init )
|
||||
{
|
||||
var hasOwn = Object.hasOwnProperty,
|
||||
pub = prop_init[ 'public' ];
|
||||
|
||||
for ( var field in obj )
|
||||
{
|
||||
var value = obj[ field ];
|
||||
|
||||
// we are not interested in the objtype chain, nor are we
|
||||
// interested in functions (which are methods and need not be
|
||||
// proxied)
|
||||
if ( !( hasOwn.call( obj, field ) )
|
||||
|| typeof value === 'function'
|
||||
)
|
||||
{
|
||||
continue;
|
||||
}
|
||||
|
||||
this._memberBuilder.buildProp(
|
||||
prop_init, null, field, value, {}
|
||||
);
|
||||
}
|
||||
};
|
||||
|
||||
|
||||
exports.prototype.buildMembers = function buildMembers(
|
||||
props, class_id, base, prop_init, memberdest, staticInstLookup
|
||||
)
|
||||
|
|
|
@ -98,5 +98,185 @@ require( 'common' ).testCase(
|
|||
} );
|
||||
} );
|
||||
},
|
||||
|
||||
|
||||
/**
|
||||
* This was a subtle bug that creeped up in a class that was derived
|
||||
* from a prototype: the prototype was setting its property values
|
||||
* (which are of course public), which the class was also manipulating.
|
||||
* Unfortunately, the class was manipulating a property of a same name
|
||||
* on the private visibility object, whereas the prototype instance was
|
||||
* manipulating it on the public. Therefore, the value of the property
|
||||
* varied depending on whether you asked the class instance or the
|
||||
* prototype instance that it inherited. Yikes.
|
||||
*
|
||||
* The root issue of this was even more subtle: the parent method (that
|
||||
* does the manipulation) was invoked, meaning that it was executed
|
||||
* within the context of the private visibility object, which is what
|
||||
* caused the issue. However, this issue is still valid regardless of
|
||||
* whether a parent method is called.
|
||||
*
|
||||
* Mitigating this is difficult, so we settle for a combination of good
|
||||
* guessing and user education. We assume that all non-function fields
|
||||
* set on the object (its own fields---not the prototype chain) by the
|
||||
* constructor are public and therefore need to be proxied, and so
|
||||
* implicitly declare them as such. Any remaining properties that are
|
||||
* set on the object (e.g. set by methods but not initialized in the
|
||||
* ctor) will need to be manually handled by declaring them as public in
|
||||
* the class. We test the first case here.
|
||||
*/
|
||||
'Recognizes and proxies prototype properties as public': function()
|
||||
{
|
||||
var expected = 'baz',
|
||||
expected2 = 'buzz';
|
||||
|
||||
// ctor initializes a single property, which is clearly public (as
|
||||
// all fields on an object are)
|
||||
var P = function()
|
||||
{
|
||||
this.foo = 'bar';
|
||||
|
||||
this.updateFoo = function( val )
|
||||
{
|
||||
this.foo = val;
|
||||
};
|
||||
};
|
||||
|
||||
var inst = this.Class.extend( P,
|
||||
{
|
||||
// since updateField is invoked within the context of the
|
||||
// instance's private visibility object (unless falling back),
|
||||
// we need to ensure that the set of foo is properly proxied
|
||||
// back to the public property
|
||||
'override updateFoo': function( val )
|
||||
{
|
||||
// consider that we're now invoking the parent updateFoo
|
||||
// within the context of the private visibility object,
|
||||
// *not* the public visibility object that it is accustomed
|
||||
// to
|
||||
this.__super( val );
|
||||
return this;
|
||||
},
|
||||
|
||||
ownUpdateFoo: function( val )
|
||||
{
|
||||
this.foo = val;
|
||||
return this;
|
||||
}
|
||||
} )();
|
||||
|
||||
// if detection failed, then the value of foo will still be "bar"
|
||||
this.assertEqual( inst.ownUpdateFoo( expected ).foo, expected );
|
||||
|
||||
// another interesting case; they should be mutual, but it's still
|
||||
// worth demonstrating (see docblock comments)
|
||||
this.assertEqual( inst.updateFoo( expected2 ).foo, expected2 );
|
||||
},
|
||||
|
||||
|
||||
/**
|
||||
* This demonstrates what happens if ease.js is not aware of a
|
||||
* particular property. This test ensures that the result is as
|
||||
* expected.
|
||||
*/
|
||||
'Does not recognize non-ctor-initialized properties as public':
|
||||
function()
|
||||
{
|
||||
var expected = 'bar';
|
||||
|
||||
var P = function()
|
||||
{
|
||||
this.init = function( val )
|
||||
{
|
||||
// this was not initialized in the ctor
|
||||
this.foo = val;
|
||||
return this;
|
||||
};
|
||||
};
|
||||
|
||||
var inst = this.Class.extend( P,
|
||||
{
|
||||
rmfoo: function()
|
||||
{
|
||||
// this is not proxied
|
||||
this.foo = undefined;
|
||||
return this;
|
||||
},
|
||||
|
||||
getFoo: function()
|
||||
{
|
||||
return this.foo;
|
||||
}
|
||||
} )();
|
||||
|
||||
// the public foo and the foo visible inside the class are two
|
||||
// different references, so rmfoo() will have had no effect on the
|
||||
// public API
|
||||
this.assertEqual(
|
||||
inst.init( expected ).rmfoo().foo,
|
||||
expected
|
||||
);
|
||||
|
||||
// but it will be visible internally
|
||||
this.assertEqual( inst.getFoo(), undefined );
|
||||
},
|
||||
|
||||
|
||||
/**
|
||||
* In the case where ease.js is unable to do so automatically, we should
|
||||
* be able to correct the proxy situation ourselves. This is where the
|
||||
* aforementioned "education" part comes in; it will be documented in
|
||||
* the manual.
|
||||
*/
|
||||
'Declaring non-ctor-initialized properties as public resolves proxy':
|
||||
function()
|
||||
{
|
||||
var expected = 'bar';
|
||||
|
||||
var P = function()
|
||||
{
|
||||
this.init = function()
|
||||
{
|
||||
// this was not initialized in the ctor
|
||||
this.foo = null;
|
||||
return this;
|
||||
};
|
||||
};
|
||||
|
||||
var inst = this.Class.extend( P,
|
||||
{
|
||||
// the magic
|
||||
'public foo': null,
|
||||
|
||||
setFoo: function( val )
|
||||
{
|
||||
this.foo = val;
|
||||
return this;
|
||||
}
|
||||
} )();
|
||||
|
||||
this.assertEqual( inst.init().setFoo( expected ).foo, expected );
|
||||
},
|
||||
|
||||
|
||||
/**
|
||||
* While this should follow as a conseuqence of the above, let's be
|
||||
* certain, since it would re-introduce the problems that we are trying
|
||||
* to avoid (not to mention it'd be inconsistent with OOP conventions).
|
||||
*/
|
||||
'Cannot de-escalate visibility of prototype properties': function()
|
||||
{
|
||||
var P = function() { this.foo = 'bar'; };
|
||||
|
||||
var Class = this.Class;
|
||||
this.assertThrows( function()
|
||||
{
|
||||
Class.extend( P,
|
||||
{
|
||||
// de-escalate from public to protected
|
||||
'protected foo': '',
|
||||
} );
|
||||
} );
|
||||
},
|
||||
} );
|
||||
|
||||
|
|
Loading…
Reference in New Issue