ClojureScript

Node REPL can't load converted JS modules

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Completed
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None
  • Environment:
    Node REPL with the latest CommonJS loading impl

Description

I've been trying the CommonJS support and have been failing to get it to work in Node, and likewise so has Maria Neise. This is in the case of invoking build prior to launching the REPL as well as trying a new suggestion in CLJS-1313.

I cranked up the verbosity to see what is going on and ultimately hit upon the idea that

goog.require('module$libs$german');

isn't going to work in Node and instead needs the same alternative logic that is employed for :foreign-libs in cljs.compiler/load-libs, in particular this needs to be emitted.

cljs.core.load_file("out/german.js");

I hacked the code a bit and got the "german" CommonJS module to load in Node and be useable:

To quit, type: :cljs/quit
cljs.user=> (require '[german :refer [hello]])
Compiling out/cljs/core.cljs
Using cached cljs.core out/cljs/core.cljs
Compiling out/cljs/core.cljs
Using cached cljs.core out/cljs/core.cljs
goog.provide('cljs.user');
goog.require('cljs.core');
goog.require('cljs.repl');
goog.require('cljs.pprint');
cljs.core.load_file("out/german.js");

nil
cljs.user=> (hello)
module$libs$german.hello.call(null)
"Hallo"

The attached patch illustrates the hack to get the above to work. The real fix would be a generalization of this.

Activity

Hide
Mike Fikes added a comment -

(Sorry for the overly generalized ticket title; it should probably be weakened to just talk about foreign libs that have been converted to native libs.)

Show
Mike Fikes added a comment - (Sorry for the overly generalized ticket title; it should probably be weakened to just talk about foreign libs that have been converted to native libs.)
Hide
Mike Fikes added a comment - - edited

Reproduction steps (note all of this is also in a repo at https://github.com/mfikes/test-commonjs ):

Set up a node_repl.clj:

(require 'cljs.repl)
(require 'cljs.build.api)
(require 'cljs.repl.node)

(def foreign-libs [{:file "libs/greeting.js"
       :provides ["greeting"]
       :module-type :commonjs}
      {:file "libs/german.js"
       :provides ["german"]
       :module-type :commonjs}])

(cljs.build.api/build "src"
  {:main 'foo.bar
   :output-to "out/main.js"
   :verbose true
   :foreign-libs foreign-libs})

(cljs.repl/repl (cljs.repl.node/repl-env)
  :watch "src"
  :output-dir "out"
  :foreign-libs foreign-libs)

Make a libs directory with german.js:

exports.hello = function() {
    return "Hallo";
};

and greeting.js:

var german = require("german");

exports.hello = function(name) {
    return german.hello() + ", " + name;
};

create a src/foo/bar.cljs with

(ns foo.bar
  (:require [greeting :refer [hello]]))

(enable-console-print!)

(println (hello "Welt!"))

Make a QuickStart cljs.jar uberjar and place it at root of tree and then

rlwrap java -cp cljs.jar:src clojure.main node_repl.clj

followed by

(require 'foo.bar)

You should see:

../out/foo/bar.js:6
cljs.core.println.call(null,module$libs$greeting.hello.call(null,"Welt!"));
                                                      ^
TypeError: Cannot read property 'call' of undefined
    at Object.<anonymous> (/Users/mfikes/Projects/test-commonjs/out/foo/bar.js:6:55)
    at Module._compile (module.js:460:26)
    at Object.Module._extensions..js (module.js:478:10)
    at Module.load (module.js:355:32)
    at Function.Module._load (module.js:310:12)
    at Module.require (module.js:365:17)
    at require (module.js:384:17)
    at global.CLOSURE_IMPORT_SCRIPT (repl:75:16)
    at Object.goog.require (repl:19:60)
    at repl:5:6
Show
Mike Fikes added a comment - - edited Reproduction steps (note all of this is also in a repo at https://github.com/mfikes/test-commonjs ): Set up a node_repl.clj:
(require 'cljs.repl)
(require 'cljs.build.api)
(require 'cljs.repl.node)

(def foreign-libs [{:file "libs/greeting.js"
       :provides ["greeting"]
       :module-type :commonjs}
      {:file "libs/german.js"
       :provides ["german"]
       :module-type :commonjs}])

(cljs.build.api/build "src"
  {:main 'foo.bar
   :output-to "out/main.js"
   :verbose true
   :foreign-libs foreign-libs})

(cljs.repl/repl (cljs.repl.node/repl-env)
  :watch "src"
  :output-dir "out"
  :foreign-libs foreign-libs)
Make a libs directory with german.js:
exports.hello = function() {
    return "Hallo";
};
and greeting.js:
var german = require("german");

exports.hello = function(name) {
    return german.hello() + ", " + name;
};
create a src/foo/bar.cljs with
(ns foo.bar
  (:require [greeting :refer [hello]]))

(enable-console-print!)

(println (hello "Welt!"))
Make a QuickStart cljs.jar uberjar and place it at root of tree and then
rlwrap java -cp cljs.jar:src clojure.main node_repl.clj
followed by
(require 'foo.bar)
You should see:
../out/foo/bar.js:6
cljs.core.println.call(null,module$libs$greeting.hello.call(null,"Welt!"));
                                                      ^
TypeError: Cannot read property 'call' of undefined
    at Object.<anonymous> (/Users/mfikes/Projects/test-commonjs/out/foo/bar.js:6:55)
    at Module._compile (module.js:460:26)
    at Object.Module._extensions..js (module.js:478:10)
    at Module.load (module.js:355:32)
    at Function.Module._load (module.js:310:12)
    at Module.require (module.js:365:17)
    at require (module.js:384:17)
    at global.CLOSURE_IMPORT_SCRIPT (repl:75:16)
    at Object.goog.require (repl:19:60)
    at repl:5:6
Hide
Mike Fikes added a comment - - edited

Another comment: My "hack" works, but to be honest, I don't appreciate why the code doesn't work without the hack. And I couldn't defend it if you asked me to. Node can evidently load Closure modules that were produced by CLJS, so why would they fail for Closure modules produced by CommonJS processing? I bet there is more behind this and my hack may be papering over some deeper more fundamental problem.

For reference, in Node, my :js-dependency-index has

{nil {:requires [], :provides ["module$libs$german"], :url #object[java.net.URL 0x37d3d232 "file:/Users/mfikes/Projects/test-commonjs/out/german.js"], :closure-lib true, :lib-path "out/german.js"}

while in Ambly, the same has

{nil {:requires [], :provides ["module$libs$german"], :url #object[java.net.URL 0x34652065 "file:/Volumes/Ambly-81C53995/german.js"], :closure-lib true, :lib-path "/Volumes/Ambly-81C53995/german.js"}

(To clarfy, the above two :js-dependency-index bits are only part of the index... there is also all of the normal goog stuff.)

Show
Mike Fikes added a comment - - edited Another comment: My "hack" works, but to be honest, I don't appreciate why the code doesn't work without the hack. And I couldn't defend it if you asked me to. Node can evidently load Closure modules that were produced by CLJS, so why would they fail for Closure modules produced by CommonJS processing? I bet there is more behind this and my hack may be papering over some deeper more fundamental problem. For reference, in Node, my :js-dependency-index has
{nil {:requires [], :provides ["module$libs$german"], :url #object[java.net.URL 0x37d3d232 "file:/Users/mfikes/Projects/test-commonjs/out/german.js"], :closure-lib true, :lib-path "out/german.js"}
while in Ambly, the same has
{nil {:requires [], :provides ["module$libs$german"], :url #object[java.net.URL 0x34652065 "file:/Volumes/Ambly-81C53995/german.js"], :closure-lib true, :lib-path "/Volumes/Ambly-81C53995/german.js"}
(To clarfy, the above two :js-dependency-index bits are only part of the index... there is also all of the normal goog stuff.)
Hide
Maria Geller added a comment -

We are making progress with this one. A PR has just been merged into the Google Closure compiler that fixes the problem for CommonJS and AMD modules: https://github.com/google/closure-compiler/pull/1071. We will need to make a second PR for ECMAScript 6.

The reason that this failed before is, that when the JS modules are converted to Google Closure modules the new namespaces are declared as local, which is different from "normal" Google Closure modules. Since the Node REPL runs scripts in a separate context, the namespace functionality will not be seen outside this context.

Show
Maria Geller added a comment - We are making progress with this one. A PR has just been merged into the Google Closure compiler that fixes the problem for CommonJS and AMD modules: https://github.com/google/closure-compiler/pull/1071. We will need to make a second PR for ECMAScript 6. The reason that this failed before is, that when the JS modules are converted to Google Closure modules the new namespaces are declared as local, which is different from "normal" Google Closure modules. Since the Node REPL runs scripts in a separate context, the namespace functionality will not be seen outside this context.
Hide
Maria Geller added a comment -

The fix for converted ES6 modules has been merged into the Google Closure compiler: https://github.com/google/closure-compiler/pull/1081.

This should work now for all three module types. I think, this can be closed

Show
Maria Geller added a comment - The fix for converted ES6 modules has been merged into the Google Closure compiler: https://github.com/google/closure-compiler/pull/1081. This should work now for all three module types. I think, this can be closed
Hide
David Nolen added a comment -

fixed

Show
David Nolen added a comment - fixed
Hide
Mike Fikes added a comment -

Hey Maria, if I try the repro steps above (at http://dev.clojure.org/jira/browse/CLJS-1314?focusedCommentId=39408&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-39408) I get a JSC_ES6_MODULE_LOAD_ERROR that looks like this:

$ rlwrap java -cp cljs.jar:src clojure.main node_repl.clj
ERROR: JSC_ES6_MODULE_LOAD_ERROR. Failed to load module "german" at libs/greeting.js line 1 : 13
Reading analysis cache for jar:file:/Users/mfikes/Desktop/test-commonjs/cljs.jar!/cljs/core.cljs
Compiling src/foo/bar.cljs
Compiling out/cljs/core.cljs
Using cached cljs.core out/cljs/core.cljs
ClojureScript Node.js REPL server listening on 53411
Watch compilation log available at: out/watch.log
To quit, type: :cljs/quit
cljs.user=>

Do you think this issue is actually fixed? I'm wondering if I'm doing something wrong, or if this ticket should be reopened.

Show
Mike Fikes added a comment - Hey Maria, if I try the repro steps above (at http://dev.clojure.org/jira/browse/CLJS-1314?focusedCommentId=39408&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-39408) I get a JSC_ES6_MODULE_LOAD_ERROR that looks like this:
$ rlwrap java -cp cljs.jar:src clojure.main node_repl.clj
ERROR: JSC_ES6_MODULE_LOAD_ERROR. Failed to load module "german" at libs/greeting.js line 1 : 13
Reading analysis cache for jar:file:/Users/mfikes/Desktop/test-commonjs/cljs.jar!/cljs/core.cljs
Compiling src/foo/bar.cljs
Compiling out/cljs/core.cljs
Using cached cljs.core out/cljs/core.cljs
ClojureScript Node.js REPL server listening on 53411
Watch compilation log available at: out/watch.log
To quit, type: :cljs/quit
cljs.user=>
Do you think this issue is actually fixed? I'm wondering if I'm doing something wrong, or if this ticket should be reopened.
Hide
Maria Geller added a comment -

Hi Mike, this problem was very likely caused by changes to the Google Closure ES6ModuleLoader. It can be fixed by requiring the german.js module in greeting.js with require("./german") instead of just require("german")

Show
Maria Geller added a comment - Hi Mike, this problem was very likely caused by changes to the Google Closure ES6ModuleLoader. It can be fixed by requiring the german.js module in greeting.js with require("./german") instead of just require("german")
Hide
Mike Fikes added a comment -

Thank you Maria. I can confirm that, after revising the greeting.js file to require("./german"), I get a successful result:

cljs.user=> (require 'foo.bar)
Hallo, Welt!
nil
Show
Mike Fikes added a comment - Thank you Maria. I can confirm that, after revising the greeting.js file to require("./german"), I get a successful result:
cljs.user=> (require 'foo.bar)
Hallo, Welt!
nil

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: