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

Accessor order in Nashorn compatibility mode #343

Closed
provegard opened this issue Sep 18, 2020 · 4 comments
Closed

Accessor order in Nashorn compatibility mode #343

provegard opened this issue Sep 18, 2020 · 4 comments
Assignees
Labels
interop Polyglot Language Interoperability

Comments

@provegard
Copy link

Hi!

I have the following Scala class:

case class SomeClass(id: String) {
   def getId() = id // provided for Nashorn compatibility
}

With Nashorn, I could write someClassInstance.id, and Nashorn would call getId for me.

With GraalJS with Nashorn compatibility enabled, the id function (vals in Scala compile to functions) takes precedence over getId, so the result of someClassInstance.id will be a host function rather than a string.

This is documented in the Nashorn migration guide, so I it is clearly designed behavior.

However, from a compatibility perspective, it seems a bit odd to use a different accessor order and thus not be fully compatible after all.

Questions:

  1. Out of curiosity, why is the accessor order not compatible?
  2. Is there any experimental option that can give me true Nashorn compatibility for this particular situation?
@wirthi wirthi added the interop Polyglot Language Interoperability label Sep 30, 2020
@wirthi
Copy link
Member

wirthi commented Sep 30, 2020

Hi @provegard

thanks for your questions.

Yes, I can confirm we differ from Nashorn in this area, and this even in nashorn-compat mode. This was an intentional decision when we first made it, as we strongly disagree with the design decision of Nashorn in that aspect.

  • Nashorn's ordering seems to be: getId > isId > id.
  • Graal.js' ordering is: id > getId > isID.

However, with our clear separation in supported features and nashorn-compat mode, we should revisit this decision. We will see if we can support this. On first sight, it seems feasible, although with a certain performance impact (as we have to check whether the two getter functions exist in any case, instead of doing this in a fallback case only when we cannot read from the field). We will check and see what we can do.

As usual, I want to highlight we don't recommend using the nashorn-compat mode and suggest not to use it unless really necessary. The Nashorn mode locks you in some questionable design decisions and might prevent moving forward along the ECMAScript specification changes.

Best,
Christian

@provegard
Copy link
Author

Hi @wirthi!

Thank you for responding. Yes, I realize that nashorn-compat mode is less than ideal, but I also have a platform where most behavior is encoded in user scripts, and I'd like to break them as little as possible. Right now I'm in the "getting all tests green" phase. Once that is done, I can see more precisely what the consequences are of not using the compat mode.

One might say that one problem here is that I'm using Scala. Had it been possible to force a Scala to emit a plain field for a val, this issue would go away. But I don't know of a way to do that.

Currently, I have worked around the issue by wrapping the "offending" object. It's one extra layer of indirection, but the wrapper is a plain host object rather than a proxy, so I suppose the performance impact is negligible (I haven't measured).

I don't have a strong opinion about accessor order, but I do think that a compatibility mode that is truly compatible reduces confusion. :)

Best regards,
Per

@wirthi
Copy link
Member

wirthi commented Oct 2, 2020

Hi @provegard

this should be fixed by cb05779

Note there is still one difference compared to Nashorn that we cannot easily solve right now: Nashorn would only call an isXyz if that was a function returning boolean; with our current interop protocol, we cannot check the static return type before the actual call, only the dynamic one of the value actually returned. So we DO call isXyz if it is available and return it's result, even if that's not a boolean.

This will be part of GraalVM 20.3. and should be in the nightly builds in a few days.

Best,
Christian

@provegard
Copy link
Author

Thank you @wirthi !

@wirthi wirthi closed this as completed Oct 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
interop Polyglot Language Interoperability
Projects
None yet
Development

No branches or pull requests

3 participants