Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Question: ScriptableObject implements the Map interface, but the Map.get impl. doesn't take into account the ScriptableObject prototype #903

Closed
p-bakker opened this issue May 21, 2021 · 18 comments

Comments

@p-bakker
Copy link
Collaborator

So, if I have a JavaScript Object created like:

let p = {name: 'test'}
let c = Object.create(p);

And object c gets send to Java and the Java code tries to access the name property through the Map interface, it won't find it.

Is that the desired behavior?

Reason I'm asking is not so much that I need this behavior one way or another, but that I'm looking whether I can introduce an EcmaObject interface that follows the Object Internal Methods specification and the only method in ScriptableObject (which would implement the EcmaObject interface) that collides is Object ScriptableObject.get(Object id), which is part of the java.util.Map interface implementation.

The underlying reason of trying to get to this EcmaObject interface is easier implementation of new EcmaScript features (Template Literals, Proxy/Reflect etc.).

If there are any thoughts on this EcmaObject interface approach, besides the specific question about ScriptableObject.get & prototype, I'd appreciate it.

@tuchida
Copy link
Contributor

tuchida commented May 21, 2021

ScriptableObject.getProperty traces the prototype.

public static Object getProperty(Scriptable obj, String name)

@p-bakker
Copy link
Collaborator Author

Yeah, I know, but getProperty is not the part of the Map interface, so if in Java code you send this c value into some Java code that deals with Maps, it wont call getProperty, but get.

And yeah, the EcmaObject interface could instead define a getProperty instead of get, but then the naming is not inline with the spec (although the spec doesn't explicitly specify the name of the internal Object methods), which is just confusing imho.

@p-bakker
Copy link
Collaborator Author

Didn't explicitly state this before but the Object Internal [[Get]] method as defined by EcmaScript is to walk to the prototype chain (contrary to the current ScriptableObject.get(Object) or ScriptableObject.get(int/String/Symbol, Scriptable) implementations)

@tonygermano
Copy link
Contributor

I don't think it should walk the prototype chain, since a Map is for holding data, and not representing objects. JSON.stringify also only looks at own properties.

I often use a javascript object to initialize a Map like:

var map = new java.util.HashMap({name: 'test'})

I would not want some property on Object.prototype to show up in the resulting Map.

The method you are looking for is probably one of the getObject* methods in ScriptRuntime.

@p-bakker
Copy link
Collaborator Author

I might be wrong, but I don't think your example applies here.

AFAICS, the ScriptableObject.get() method would only come into play when sending a JavaScript object {} from JavaScript land to the Java world and then the Java code would handle it as a Map en call .get(Object) on it.

Again, not sure, but I think if you'd send your map value back to Java land, it would just be a HashMap instance, not a ScriptableObject instance.

IMHO it would make sense if ScriptableObject.get(Object) would walk the prototype chain of the JavaScript object its called on from Java land: you don't want the logic on the Java-side to have to take into account that the Map instance is actually a JavaScript object that can have a prototype chain that you might need to walk to get to the value you're after.

@tonygermano
Copy link
Contributor

I am sending a javascript object to java land by passing an object literal to the HashMap constructor, which expects a java.util.Map.

Like I said, ECMA [[Get]] is implemented by various functions in ScriptRuntime, rather than on ScriptableObject.

I know you were just looking at String.raw while working on #904 . If you compare the original code to what I ended up implementing, you'll see I changed the ScriptableObject.get calls to ScriptRuntime.getObjectProp and ScriptRuntime.getObjectIndex so that they would walk the prototype chain. I think I originally ran into the same issue as you until I discovered I was looking in the wrong place.

For what it's worth, I do agree with your change to the String.raw implementation.

@tonygermano
Copy link
Contributor

Something else to think about is that we still need a get method that doesn't walk the prototype chain for various other features that require that.

@p-bakker
Copy link
Collaborator Author

p-bakker commented May 21, 2021

I am sending a javascript object to java land by passing an object literal to the HashMap constructor, which expects a java.util.Map

Right, so wouldn't you expect your HashMap to contain name in the example below?

var p = {name: 'test'};
var map = new java.util.HashMap(Object.create(p))

I know you were just looking at String.raw while working on #904

My musings about ScriptableObject.get(Object) having to walk the prototype chain don't particularly come from String.raw impl. Besides me thinking it would just make more sense, I'm also trying to implement Proxies and having challenges because the implementation needs to be coded against the ScriptableObject class, instead of a particular interface, cause there's no interface that defines the (internal methods) of the EcmaScript Object Type.
So I'm trying to implement such interface (and trying to keep its naming as close to the spec as possible).
I think this is also helpful going forwards as it'll allow starting to organize Rhino's code more along the lines of the EcmaScript spec, making it easier to implement new features (and easier to grasp for devs).

Something else to think about is that we still need a get method that doesn't walk the prototype chain for various other features that require that.

The existing ScriptableObject.get(int/String/Symbol, Scriptable) methods wouldn't be removed

@tonygermano
Copy link
Contributor

Right, so wouldn't you expect your HashMap to contain name in the example below?

var p = {name: 'test'};
var map = new java.util.HashMap(Object.create(p))

No, because Maps are for data, not inherited object properties. JSON.stringify(Object.create(p)) returns an empty object. Object.entries(Object.create(p)) returns an empty array.

My musings about ScriptableObject.get(Object) having to walk the prototype chain don't particularly come from String.raw impl.

The existing ScriptableObject.get(int/String/Symbol, Scriptable) methods wouldn't be removed

I was just trying to point you at some code that I knew we were both familiar with, and I think I had a similar journey as you. Various ScriptableObject.ecmaGet methods could probably be added that could refer to the ScriptRuntime methods.

@p-bakker
Copy link
Collaborator Author

No, because Maps are for data, not inherited object properties. JSON.stringify(Object.create(p)) returns an empty object. Object.entries(Object.create(p)) returns an empty array.

Right.... while in my opinion prototype properties could also be data (all depends how you're using it), I do agree with the examples of other methods (albeit all js land methods) and how they handle prototype properties.

So, assuming everyone else is of the same opinion, i have to just use some naming convention to differentiate these Ecma Internal Object methods.

What about using the exact names as used in the spec, so Get, GetOwnProperty, Set, etc? I know that's not the standard in java land, but it would make them stand out and not likely to clash with anything 🙂

If not that, what about prefixing with i (IGet) or e (eGet)?

@tonygermano
Copy link
Contributor

why not ecmaGet? I think that makes it clear what the intended purpose is.

@p-bakker
Copy link
Collaborator Author

p-bakker commented May 21, 2021 via email

@gbrail
Copy link
Collaborator

gbrail commented May 21, 2021 via email

@p-bakker
Copy link
Collaborator Author

p-bakker commented May 21, 2021 via email

@tonygermano
Copy link
Contributor

The interface for ScriptableObject is Scriptable. I think I see what you are getting at. I've been wondering how to best apply internal methods and internal slots to objects because that starts to come into play when implementing ES6 inheritance. For example, you can extend the built-in Number object, and calling the super constructor causes instances of the new class to gain an internal [[NumberData]] slot.

See https://tc39.es/ecma262/#sec-number-constructor

Maybe @gbrail has some ideas since he just reworked the SlotMap classes.

@p-bakker
Copy link
Collaborator Author

The Scriptable interface doesn't cut it anymore: when looking at Proxies for example, they expect an object that conforms to the Object type as defined in the EcmaScript specification. The Scriptable interface isn't compatible.

So, I can either build Proxies against ScriptableObject directly or (try to) introduce a new interface, allowing integrators to build compatible implementations without having to extend ScriptableObject.

And secondly having an EcmaScript Object Type compatible interface would easy implementing features in a spec compliant way, as all the abstract helper methods defined in the spec could act as building blocks

@gbrail
Copy link
Collaborator

gbrail commented May 21, 2021 via email

@p-bakker
Copy link
Collaborator Author

Closing this case, as I got my question answered. Afterwards I also realized that the Object get(Object id) methods unwraps the returned value, which isn't what should happen in the EcmaScript-defined [[Get]] operation, so the whole question was a bit mute anyway...

For the whole EcmaScriptable interface I'll open a new issue when time comes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants