nrepl / weasel

ClojureScript browser REPL using WebSockets
The Unlicense
324 stars 35 forks source link

ExceptionInfo: No such namespace: cljsjs.react at line 1 #43

Closed zentrope closed 9 years ago

zentrope commented 9 years ago

Using:

[org.clojure/clojurescript "0.0-2760"]
[org.omcljs/om             "0.8.8"]
[com.cemerick/piggieback   "0.1.5"]
[weasel                    "0.5.0"]

I then run "cider-jack-in" from Emacs, get a normal repl, then:

(require 'weasel.repl.websocket)

(cemerick.piggieback/cljs-repl
 :repl-env (weasel.repl.websocket/repl-env
            :ip "0.0.0.0" :port 9001))

which results in:

Unhandled clojure.lang.ExceptionInfo
   No such namespace: cljsjs.react at line 1
   file:/Users/keith/.m2/repository/sablono/sablono/0.3.1/sablono-0.3.1.jar!/sablono/core.cljs
{:tag :cljs/analysis-error,
 :file
 "file:/Users/keith/.m2/repository/sablono/sablono/0.3.1/sablono-0.3.1.jar!/sablono/core.cljs",
 :line 1,
 :column 1}

According to David Nolen:

Weasel and piggieback likely need to be changed to load foreign dependencies.

Not sure how much that helps, but this seems to have been experienced by a few people if the clojurescript mailing list is any indication.

A fellow sufferer fixed it via a hack:

https://github.com/darwin/weasel/commit/c2b5b963bcf10a7c341a4cbc8f21121d3adef6b0

if that helps any.

martinklepsch commented 9 years ago

I looked into this yesterday and think the fix is something along the lines of this:

diff --git a/src/clj/weasel/repl/websocket.clj b/src/clj/weasel/repl/websocket.clj
index f2f9ce6..7710f4c 100644
--- a/src/clj/weasel/repl/websocket.clj
+++ b/src/clj/weasel/repl/websocket.clj
@@ -39,7 +39,11 @@
 (defn repl-env
   "Returns a JS environment to pass to repl or piggieback"
   [& {:as opts}]
-  (let [compiler-env (env/default-compiler-env opts)
+  (let [ups-deps (cljsc/get-upstream-deps)
+        opts (assoc opts
+               :ups-libs (:libs ups-deps)
+               :ups-foreign-libs (:foreign-libs ups-deps))
+        compiler-env (env/default-compiler-env opts)
         opts (merge (WebsocketEnv.)
                {::env/compiler compiler-env
                 :ip "127.0.0.1"
@@ -47,7 +51,9 @@
                 :preloaded-libs []
                 :src "src/"}
                opts)]
-    (swap! compiler-env assoc :js-dependency-index (js-deps/js-dependency-index opts))
+    ;; might be just no longer required
+    ;; (swap! compiler-env assoc :js-dependency-index (js-deps/js-dependency-index opts))
+    (prn ups-deps)
     (env/with-compiler-env (::env/compiler opts)
       (reset! preloaded-libs
         (set/union

I didn't get to test this due to some other issues but I think basically that's whats missing to make weasel work with :foreign-libs stuff.

the-kenny commented 9 years ago

The diff above doesn't seem to suffice for me. Keep getting nil from (cljsc/get-upstream-deps). From IRC: This seems to happen because weasel doesn't call this from the main thread, and cljs.closure/get-upstream-deps* used the current thread's classloader.

A solution which works for me is patching cljs.closure/get-upstream-deps* to use (java.lang.ClassLoader/getSystemClassLoader).

martinklepsch commented 9 years ago

So. I found another solution:

(swap! compiler-env assoc :js-dependency-index (js-deps/js-dependency-index opts))

When this function is called in line 50 in websocket.clj opts needs to contain an additional key to make :foreign-libs dependencies work. That key is :ups-foreign-libs. The value of this key needs to look something like this:

({:file "cljsjs/development/firebase.inc.js",
  :file-min "cljsjs/production/firebase.min.inc.js",
  :provides ["cljsjs.firebase"]}
 {:file "cljsjs/development/react.inc.js",
  :file-min "cljsjs/production/react.min.inc.js",
  :provides ["cljsjs.react"]})

This data can be generated from deps.cljs files which come with CLJSJS jars. The Clojurescript compiler is doing something like this in cljs.closure/get-upstream-deps* but only uses the current threads class loader which can lead to some deps.cljs files not being found.

@the-kenny opened a JIRA ticket to allow additional class loaders as arguments to get-upstream-deps.

You don't have to use get-upstream-deps to gather this information though. I wrote the following to do it in the boot-cljs-repl project which is using Weasel:

;; there probably a lot more elegant ways to do this
(defn get-upstream-deps []
  (let [res (pod/classloader-resources "deps.cljs")]
    (->> (for [[cl uris] res]
          (if uris
            (map #(-> % slurp read-string) uris)))
        flatten
        distinct
        (remove nil?)
        (apply merge-with concat))))

This is later used like so:

(def ups-deps (get-upstream-deps))
(weasel.repl.websocket/repl-env :ups-libs (:libs ups-deps) 
                                :ups-foreign-libs (:foreign-libs ups-deps)

This produced a working REPL for me.

the-kenny commented 9 years ago

Yup, I can confirm this approach is working. One thing to note is that we can do that without duplicating behavior (e.g. get-upstream-deps) as soon as CLJS-1002 is applied.

We also need to monkey-patch goog.require like https://github.com/clojure/clojurescript/commit/5a353c418769cc6d4c212db87e5f51b9511f7c33#diff-c706571c79101d1f03c9f7073570741eR112 to fix require.

I'll have a pull-request ready in a few minutes - we need to wait for the JIRA ticket though.