Namespace starting with `com` broken in graaljs replenv

Question

With graal installed:


$ java -version
openjdk version "1.8.0_212"
OpenJDK Runtime Environment (build 1.8.0_212-20190420112649.buildslave.jdk8u-src-tar--b03)
OpenJDK GraalVM CE 19.0.0 (build 25.212-b03-jvmci-19-b01, mixed mode)



$ clj -Sdeps '{:deps {org.clojure/clojurescript {:mvn/version "1.10.520"}}}' -m cljs.main -re graaljs
ClojureScript 1.10.520
cljs.user=> (ns com.foo)

com.foo=> (defn bar [])
#'com.foo/bar
com.foo=> (bar)
Execution error (Error) at (<cljs repl>:1).
(intermediate value).foo.bar.call is not a function

Answer 1

Comment made by: kommen

GraalJS issue: https://github.com/graalvm/graaljs/issues/164

Answer 2

Comment made by: mfikes

Reminds me of CLJS-2770

Answer 3

_Comment made by: mfikes_

I wonder if setting {{js.java-package-globals}} to disable them is sufficient for this case, or if that brings in other issues:


$ clj -Sdeps '{:deps {org.clojure/clojurescript {:mvn/version "1.10.520"}}}' -m cljs.main -re graaljs -ro '{"js.java-package-globals" "false"}' -r
ClojureScript 1.10.520
cljs.user=> (ns com.foo)

com.foo=> (defn bar [])
#'com.foo/bar
com.foo=> (bar)
nil


See https://clojurescript.org/reference/repl-options#_graal_js_repl_options

Answer 4

Comment made by: kommen

Using {{js.java-package-globals}} does solve the problem as well, and as there are more "tlds" provided (Packages, java, javafx, javax, com, org, edu) this would fix usage of those as well. As Mike pointed out, this would be a breaking change though, in the case where somebody would rely on those globals to exist when running in a graal env.

Answer 5

Comment made by: kommen

CLJS-3087-2.patch turns off the java package globals by default.

I would favor compatibility with other JS engines and between Clojure and ClojureScript running on graaljs more than having the java package globals available as a shortcut. The functionality would still be available with {{Java.type}} as used in (link: https://github.com/clojure/clojurescript/blob/3a0c07477ae781bf521bdc2b074ed7b783bb93f3/src/main/cljs/cljs/bootstrap_graaljs.js#L4 text: graaljs_load).

Answer 6

Comment made by: mfikes

Even though this is, strictly speaking, a breaking change, GraalJS hasn't been out for that long yet so such a change is unlikely to break anyone. If it does, it is possible to override the default.

(link: ~kommen) You had pointed out you saw the use of Java packages here: https://github.com/clojure/clojurescript/blob/master/src/main/clojure/cljs/closure.clj#L1672-L1676

I haven't looked, are those unaffected if we were to revise the default?

Answer 7

Comment made by: kommen

(link: ~mfikes) As far as I can see these are unaffected by changing the default option, since the GraalJS engines created with the default options are not used to run artifacts produced by {{output-main-file}}. But I would still suggest to change those usages to use {{Java.type}} instead (pending compatibility with Nashorn), so that one can use GraalJS' {{js}} command line tool with {{js.java-package-globals}} turned off and still have the produced main files work.

Answer 8

Comment made by: mfikes

CLJS-3087-2.patch passes CI (/)

Answer 9

Comment made by: mfikes

CLJS-3087-2.patch added to Patch Tender (i)

Answer 10

Comment made by: mfikes

CLJS-3087-2.patch LGTM (y)

Answer 11

Comment made by: kommen

Over at the (link: https://github.com/graalvm/graaljs/issues/164 text: graaljs issue), they do not intend to change the default value for {{js.java-package-globals}}, so while CLJS-3087-2.patch would fix the issue for the GraalJS repl, people would still need to find out the need to turn it off when using the {{js}} or {{nodejs}} commandline tools.

Answer 12

_Comment made by: kommen_

A real world example running into this issue:



clj -Sdeps '{:deps {org.clojure/clojurescript {:mvn/version "1.10.520"} com.cognitect/transit-cljs {:mvn/version "0.8.256"}}}' -m cljs.main -re graaljs
ClojureScript 1.10.520
cljs.user=> (require '[cognitect.transit :as t])
Execution error () at (<cljs repl>:1).
null

Answer 13

Comment made by: dnolen

Dieter, I don't understand you last comment about js and nodejs. How are they affected?

Answer 14

Comment made by: dnolen

I'm leaning towards the second patch which disables implicit global imports. It's an ugly thing to rely on anyway ... for another ticket - but wondering if we should have require support for Java packages.

Answer 15

Comment made by: kommen

(link: ~dnolen) GraalVM ships with a {{bin/js}} and {{bin/nodejs}} binary, both use the GraalJS engine, but they default {{js.java-package-globals}} to {{true}}. So if one would use GraalVM's {{nodejs}} to run a ClojureScript artifact, they would still encounter issues regarding namespaces with one of the problematic prefixes. This of course is out of ClojureScript's control, but I thought it puts another perspective on the patch, as it doesn't cover all possible paths on how one can run into this issue.