oracle / graaljs

GraalJS – A high-performance, ECMAScript compliant, and embeddable JavaScript runtime for Java
https://www.graalvm.org/javascript/
Universal Permissive License v1.0
1.81k stars 190 forks source link

Incompatibility between graaljs and Google Closure Library `goog.provide` #164

Closed kommen closed 3 years ago

kommen commented 5 years ago

Google Closure Library's goog.provide doesn't work in graaljs when the given namespace starts with com.

This does work in other JS engines like nodejs. For example:

$ curl -O https://raw.githubusercontent.com/google/closure-library/66700f582b1f94d6ef6e8f9e7d9dbb2ca7d6d792/closure/goog/base.js

It works in nodejs:

$ node          
Welcome to Node.js v12.1.0.
Type ".help" for more information.
> .load base.js
> goog.provide('com.bar.my_place')
undefined
> com.bar
{ my_place: {} }
> 

But fails with graaljs version 19.0.0:

$ js
>  load("base.js")
function() {
  var /** !Object<string, boolean> */ requiresTranspilation = {'es3': false};
  var transpilationRequiredForAllLaterModes = false;

  /**
   * Adds an entry to requiresTranspliation ...<omitted>...
}
> goog.provide('com.bar.my_place')
Error: Namespace "com.bar.my_place" already declared.
    at <js> goog.provide(base.js:305:10391-10441)
    at <js> :program(<shell>:4:1:0-31)
> goog.provide('foo.bar.my_place')
> foo.bar
{my_place: {...}}

Note that defining a namespace which's first namespace part is not com works as expected. It seems com is already defined to some special thing, which breaks goog.provide.

> com
JavaPackage {Symbol(Symbol.toPrimitive): {...}}

The same problem exists also with the node binary which ships with graal.

This is a problem for ClojureScript, which uses Google Closure Compiler, as users are encouraged to use the reverse domain names for namespaces. None of that ClojureScript code, after being compiled to JS, would work with graaljs because of this.

kommen commented 5 years ago

ClojureScript issue: https://dev.clojure.org/jira/browse/CLJS-3087

kommen commented 5 years ago

As @mfikes noted over at the ClojureScript jira, probably the best solution would be to set js.java-package-globals to "false" for the ClojureScript environment.

wirthi commented 5 years ago

@kommen thanks, interesting find. I'll check why we activate js.java-package-globals by default on a non-JVM runtime.

pmlopes commented 5 years ago

@wirthi is js.java-package-globals an experimental feature? I got a message that I shouldn't use it in production... why is that? This seems like a safe feature to disable

wirthi commented 5 years ago

@pmlopes yet it is; we only marked a small set of flags stable (or "supported"). All other flags are available, but we don't guarantee long-term support for them. It is safe to use them currently, but future releases might change behavior, drop the option, etc. (similar to Java's XX-options).

We might also decide for future releases that an option is really crucial and add it to the "supported" list.

Best, Christian

iamstolis commented 5 years ago

The root of the problem is that JavaPackage objects occupy the whole namespace with several prefixes (like com). goog.provide refuses to define anything with these prefixes because it thinks (through goog.isProvided_) that something useful already exists under the specified name.

Setting js.java-package-globals to false would solve this issue because it would remove the mentioned JavaPackage objects. Unfortunately, users would not be able to use a very handy syntax like new java.util.ArrayList() with such change. It would be ideal to have the best from both worlds: allow the Java-friendly syntax and don't have goog.provide broken.

This can be solved by patching goog.isProvided_ such that it returns false for JavaPackage objects. Would that be an acceptable solution for you? I saw that there are some patches related to graaljs in clojurescript already.

I am not sure what is the best/most reliable identification of JavaPackage objects but something like object => typeof object === 'object' && typeof Packages === 'object' && Packages[Symbol.toPrimitive] === object[Symbol.toPrimitive] may work well.

kommen commented 5 years ago

This can be solved by patching goog.isProvided_ such that it returns false for JavaPackage objects. Would that be an acceptable solution for you?

It would be a solution, but one would also risk collisions of names within those namespaces. I don't know how probable this would be, but I guess it would be a nightmare to debug and find out what is going on.

Setting js.java-package-globals to false would solve this issue because it would remove the mentioned JavaPackage objects.

It think the most sensible thing regarding to ClojureScript would be to just default js.java-package-globals to false for graaljs engines it instantiates itself - which one of the patches you're referring to does.

However, I'm still not clear why js.java-package-globals is on by default in non-jvm runtimes, as the globals are not useable anyway:

 $ js 
> new java.util.ArrayList()
TypeError: Access to host class java.util.ArrayList is not allowed or does not exist.
    at <js> :program(Unknown)
> java
JavaPackage {Symbol(Symbol.toPrimitive): {...}}
> 
wirthi commented 5 years ago

Hi @kommen

yes, our suggestion is to set the js.java-package-globals flag to false in your usecase. We don't want to change the default behavior there, because that would conflict with other usecases.

... is on by default in non-jvm runtimes ...

We are blurring the border here a bit. Even in js --native mode, we have some limited support for Java interoperability, e.g.:

$ js --native
> Java.type("int[]")
JavaClass[int[]]

We might extend that in the future. The default will be false if hostClassLookup is deactivated (see allowHostClassLookup).

Best, Christian

kommen commented 5 years ago

Thanks @wirthi for the clarification. Let's see if my patch to make js.java-package-globals false makes it into into ClojureScript. This would be a first step, but would still leave users in the dark when they run a javascript artifact produced with ClojureScript using graal's nodejs and js commadline tools.

wirthi commented 3 years ago

We won't change the default of js.java-package-globals, as access to Java is a fundamental feature of GraalVM JavaScript.

As there has not been an update in this ticket for more than a year, I am closing it. Please re-open or create a new ticket if there are additional questions.