graaljs icon indicating copy to clipboard operation
graaljs copied to clipboard

[Interop] JS modules do not answer KEYS message correctly

Open fniephaus opened this issue 7 years ago • 9 comments

I assume the interop KEYS message is answered using Object.keys(theJSObject) which results in the following behavior:

GraalVM MultiLanguage Shell 1.0.0-rc10
Copyright (c) 2013-2018, Oracle and/or its affiliates
  JavaScript version 1.0
  Python version 3.7.0
  R version 3.4.0
  Ruby version 2.4.4
Usage:
  Use Ctrl+L to switch language and Ctrl+D to exit.
  Enter -usage to get a list of available commands.
python> import polyglot
python> math = polyglot.eval(language='js', string='Math')
python> dir(math)
[]
python> x.min(1, 2) # message is understood even though not listed
1
python> json = polyglot.eval(language='js', string='JSON')
python> dir(json)
[]

Additionally, I think the answer should also include all non-enumerable properties (via Object. getOwnPropertyNames(theJSObject)) including inherited properties by walking up the prototype chain.

fniephaus avatar Dec 12 '18 15:12 fniephaus

I agree with your observation. However, there are other cases where you don't want to expose all non-enumerable keys from a JavaScript object in its KEYS. For example if you map a JavaScript object to a java.util.Map. That is the reason why we currently expose less in KEYS, but that is temporary. We will introduce new interop APIs to resolve this, namely a new namespace for properties/hash/dictionaries, then we can make KEYS return more elements and use the new dictionary namespace to map to HashMaps.

chumer avatar Dec 12 '18 15:12 chumer

Note that we did enable non-enumerable properties in JavaScript objects in RC8 (or RC7) with quite some negative feedback so we rolled back.

chumer avatar Dec 12 '18 15:12 chumer

Thanks, I didn't know about all of that. Sounds reasonable!

fniephaus avatar Dec 12 '18 15:12 fniephaus

I believe everything needed to make this work correctly is now available with RC15. I was curious to see whether the behavior has changed and it looks like it hasn't. Here's another example running on RC15:

GraalVM MultiLanguage Shell 1.0.0-rc15
Copyright (c) 2013-2019, Oracle and/or its affiliates
  JavaScript version 1.0
  Python version 3.7.0
  R version 3.5.1
  Ruby version 2.6.2
js> python>
python> import polyglot
python> x = polyglot.eval(language='js', string='Object({"a" : 1, "b" : 2})')
python> dir(x)
['a', 'b']
python> x.toString()
'[object Object]'
python> x.indexOf('a')
AttributeError: foreign object DynamicObject<JSUserObject>@17fbfb02 has no attribute indexOf
  • In case of dir(x), I would expect a list of all messages the object understands and its keys.
  • x.toString works as expected.
  • In case of x.indexOf('a'), I was surprised that internal implementation details are visible (DynamicObject). Maybe thats a GraalPython bug? /cc @timfel

fniephaus avatar Apr 11 '19 14:04 fniephaus

in case of dir(x), I would expect a list of all messages the object understands and its keys.

Yes, that is known, we are working on that.

In case of x.indexOf('a'), I was surprised that internal implementation details are visible (DynamicObject). Maybe thats a GraalPython bug? /cc @timfel

True that should not happen.

chumer avatar Apr 12 '19 11:04 chumer

@chumer any updates on this? Maybe interop needs to support a more fine-grained members mechanism, such as getInstanceVariables, getMethods, and so on?

fniephaus avatar Nov 11 '19 10:11 fniephaus

This is still an issue in 20.1: image

fniephaus avatar May 07 '20 08:05 fniephaus

The Date example above seems to clearly show the problem. I guess once we have the dictionary interop messages, it will be finally possible to list methods of a JavaScript object, just like it's already the case in other languages?

eregon avatar Dec 10 '20 18:12 eregon

Now that hashes are supported by the interop protocol, this can finally be addressed? Is the plan to expose field properties as hashes and function properties as invocable and readable members?

fniephaus avatar Mar 19 '21 07:03 fniephaus