Implement ES2017 Object.getOwnPropertyDescriptors #934
Labels
feature
Issues considered a new feature
good first issue
Great place to start if you're looking to start an open source "resume"
Milestone
Comments
|
I think this is also there. |
|
Don't think so. We've got the method to get a single descriptor (Object.getOwnPropertyDescriptor), but not the method to get them all in one go (Object.getOwnPropertyDescriptors) |
p-bakker
added
the
good first issue
tuchida
added a commit
to tuchida/rhino
that referenced
this issue
Mar 9, 2022
tuchida
added a commit
to tuchida/rhino
that referenced
this issue
Mar 12, 2022
tuchida
added a commit
to tuchida/rhino
that referenced
this issue
Mar 12, 2022
…tors Remove TODO comment
tuchida
added a commit
to tuchida/rhino
that referenced
this issue
Mar 14, 2022
gbrail
pushed a commit
that referenced
this issue
Mar 18, 2022
|
This change seems causing a regression as I commented here: HtmlUnit@c696e34 |
|
@RuralHunter would you happen to have some sample code that reproduces this issue, cause all tests passed, so it must be somewhat of an edge case I think |
|
Have started to dig into this |
|
@p-bakker Not yet. I just raised this question to see if anyone has some quick insight. I will try to find the minimal test case but maybe need some time since the original js is quite complex and I don't have much knowledge on rhino. |
|
Ok, was able to make a simple case that reproduces this problem... will report this alter if i understand what is going on here. |
|
Sorry the sample was wrong, i saw a ghost from bug hunting. |
|
Another theory - buildDataDescriptor is called with a broken scope and because of this the prototype is set to null. |
|
a scope without prototype: doesn't that happen when using shared/sealed/dynamic scopes? |
|
Can you show me the stack trace of JavaScript part as well as Java? |
|
|
What i found so far
From my point of view i expect the scope (topLevelScope) has to be the window (like it was in many other buildDataDescriptor calls forces by the same script before. But in this case it seems to be a (Native) object created by the script itself. Digging more into the script requires more setup on my side - i have to use my favorite Charles proxy :-) |
|
ok, I got the test code: var rr=function(t) {
var desc={
configurable: true,
enumerable: false,
key: "wfunc",
value: function(){},
writable: true
};
Object.defineProperty(t.prototype,"wfunc",desc);
};
var Fl = function() {
function e() {}
rr(e);
return e;
}();
var l=Fl.prototype;
var props1=Object.getOwnPropertyNames(l);
console.log('getOwnPropertyNames='+props1);
var props2=Object.getOwnPropertySymbols(l);
console.log('getOwnPropertySymbols='+props2);
var u=Object.getOwnPropertyDescriptors(l);
console.log('u=');
console.log(u);
var props=props1.concat(props2);
console.log('props='+props);
props.forEach(function(t) {
var a=u[t];
console.log(t+": "+a.hasOwnProperty("value"));
}); |
|
I didn't know why I was using --- a/src/org/mozilla/javascript/NativeObject.java
+++ b/src/org/mozilla/javascript/NativeObject.java
@@ -510,7 +510,7 @@ public class NativeObject extends IdScriptableObject implements Map {
Scriptable s = getCompatibleObject(cx, scope, arg);
ScriptableObject obj = ensureScriptableObject(s);
Object nameArg = args.length < 2 ? Undefined.instance : args[1];
- Scriptable desc = obj.getOwnPropertyDescriptor(cx, nameArg);
+ Scriptable desc = obj.getOwnPropertyDescriptor(cx, scope, nameArg);
return desc == null ? Undefined.instance : desc;
}
case ConstructorId_getOwnPropertyDescriptors:
@@ -521,7 +521,7 @@ public class NativeObject extends IdScriptableObject implements Map {
ScriptableObject descs = (ScriptableObject) cx.newObject(scope);
for (Object key : obj.getIds(true, true)) {
- Scriptable desc = obj.getOwnPropertyDescriptor(cx, key);
+ Scriptable desc = obj.getOwnPropertyDescriptor(cx, scope, key);
if (desc == null) {
continue;
} else if (key instanceof Symbol) {
--- a/src/org/mozilla/javascript/ScriptableObject.java
+++ b/src/org/mozilla/javascript/ScriptableObject.java
@@ -2648,11 +2652,10 @@ public abstract class ScriptableObject
}
}
- protected ScriptableObject getOwnPropertyDescriptor(Context cx, Object id) {
+ protected ScriptableObject getOwnPropertyDescriptor(Context cx, Scriptable scope, Object id) {
Slot slot = querySlot(cx, id);
if (slot == null) return null;
- Scriptable scope = getParentScope();
- return slot.getPropertyDescriptor(cx, (scope == null ? this : scope));
+ return slot.getPropertyDescriptor(cx, scope);
}
protected Slot querySlot(Context cx, Object id) {var l = (function(){}).prototype;
console.log('l', l);
var desc = Object.getOwnPropertyDescriptor(l, 'constructor');
console.log('desc', desc.hasOwnProperty('value')); // true |
|
a more simplified version |
|
OK -- I belatedly realized that this whole issue is re-opened from the original issue that introduced the future. For future reference, let's not do this -- if we fix an issue, but there's a bug in the fix, then please open a new issue rather than re-opening an old one. |
gbrail
pushed a commit
that referenced
this issue
May 27, 2022
…ons at some places Fix handling of the parent scope in a number of places. Fixes #934, which was re-opened due to a bug in the original implementation.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Proposal: https://github.com/tc39/proposal-object-getownpropertydescriptors
MDN: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/getOwnPropertyDescriptors
The text was updated successfully, but these errors were encountered: