Hi Raphael,
Thanks for getting back about this. And great work on implementing all these
ES5 features in Rhino!
> Perhaps I'm misunderstanding you, but I can't replicate the behaviour
> you describe with:
> "if configurable is false for a property that was defined by a data
> descriptor, I can still convert it to a getter / setter once "
>
> I get:
> [rhino(master)]$ java -jar build/rhino1_7R3pre/js.jar -version 180
> Rhino 1.7 release 3 PRERELEASE 2010 01 06
> js> var obj = {a:1}
> js> Object.defineProperty(obj, "b", {configurable:false,
> enumerable:true, value:5})
> [object Object]
> js> Object.defineProperty(obj, "b", {configurable:false,
> enumerable:true, get: function(){print("gotcha"); return 6}})
> js: "<stdin>", line 4: uncaught JavaScript runtime exception:
> TypeError: Cannot change "b" from a data property to an accessor
> property because configurable is false.
> at <stdin>:4
>
> js>
>
> Could you write a failing test for this behaviour?
You are right, with normal objects this works as expected. The situation where
I encountered it was with Function.prototype, when trying out various things
with Object.defineProperty(). Now of course the following should not be done
anyway, but nevertheless they way it acts seems wrong:
(// marks my comments)
lehni$ java -jar build/rhino1_7R3pre/js.jar -version 180
Rhino 1.7 release 3 PRERELEASE 2010 04 14
// Describe the name property in Function.prototype to make sure it is not
configurable
js> JSON.stringify(Object.getOwnPropertyDescriptor(Function.prototype, 'name'));
{"value":"","writable":false,"enumerable":false,"configurable":false}
// Try configuring it anyway, by defining a new getter for it
js> Object.defineProperty(Function.prototype, 'name', { get: function() {
return 'Another Name' } });
function () {
[native code, arity=0]
}
// This returned the Function.prorotype object, so it appeared to have worked,
as it did not throw an error.
// Let's check:
js> JSON.stringify(Object.getOwnPropertyDescriptor(Function.prototype, 'name'));
{"enumerable":false,"configurable":false}
// Indeed, value is gone from the descriptor, and there should be a get
function, which was filtered out by JSON.stringify, as it is a function.
// Just make sure by checking Object.keys:
js> Object.keys(Object.getOwnPropertyDescriptor(Function.prototype, 'name'));
enumerable,configurable,get
// So let's try and see if the getter actually is called
js> function test() {}
js> test.name;
test
// It doesn't seem to be called.
// But what happens if we redefine anyway?
js> Object.defineProperty(Function.prototype, 'name', { get: function() {
return 'New Name' } });
// Now we finally get the expected exception:
js: "<stdin>", line 7: uncaught JavaScript runtime exception: TypeError: Cannot
change the get attribute of "name" because configurable is false.
at <stdin>:7
> I agree that the PropertyDescriptor class sounds like a better
> solution. However when I initially started working on defineProperty,
> the feedback was that the this abstraction was not a good idea, so
> best to confirm with Norris et al about that one.
>
> See https://bugzilla.mozilla.org/show_bug.cgi?id=489329#c3
I was not aware of your previous work on such a class. My understanding of
Norris' point is that he did not like the PropertyDescriptor object being
returned directly from Object.getOwnPropertyDescriptor, as it did not behave
like a proper NativeObject. The way I propose to implement this is to use a
PropertyDescriptor class simply as an internal cache for all these values, to
avoid continuous lookup on a Scriptable object which tends to be slow.
PropertyDescriptor would convert itself to and from Scriptable objects, and to
the outside, nothing would change at all.
I switched an inheritance framework library from simply setting properties on
the prototype object to using Object.defineProperty, and the performance
decreased notably. So I would like to find a way to get some of that speed back
and still use the new features.
Also, I would like to propose to make your two versions of defineProperty into
one:
private void defineOwnProperty(Context cx, Object id, ScriptableObject desc,
boolean checkValid)
private void defineOwnProperty(Context cx, Slot slot, ScriptableObject desc,
int attributes)
As the second is only ever called from the first, and through the merge,
further optimisations can take place.
I would then also propose to make this function public, so classes inheriting
from ScriptableObject can then override defineOwnProperty, for example when
they want to take note of such a property change. This is a requirement I have
for one of my projects where I use Rhino as the JS engine.
I am happy to do all this work myself but am curios to hear the thoughts of
others first, especially also from Norris.
Best,
Jürg
_______________________________________________
dev-tech-js-engine-rhino mailing list
[email protected]
https://lists.mozilla.org/listinfo/dev-tech-js-engine-rhino
