Skip to content

Commit

Permalink
fix goog.module requires in the REPL
Browse files Browse the repository at this point in the history
the namespace local reference wasn't properly created, so results
in a runtime undefined variable error.

fixed #1052
  • Loading branch information
thheller committed Oct 9, 2022
1 parent bf38b2c commit e532400
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 7 deletions.
12 changes: 8 additions & 4 deletions src/main/shadow/build/compiler.clj
Expand Up @@ -344,6 +344,13 @@
;; most of it seems self-host related which we do not need
;; it also has a hard coded emit for cljs.core which would cause a double emit
;; since deps (correctly) contains cljs.core but not in CLJS

(defn emit-goog-module-gets [current-ns module-deps]
(comp/emitln "goog.scope(function(){")
(doseq [{:keys [goog-module ns]} module-deps]
(comp/emitln (str " " (ana/munge-goog-module-lib current-ns ns) " = goog.module.get('" goog-module "');")))
(comp/emitln "});"))

(defmethod shadow-emit :ns [{:keys [mode] :as state} {:keys [name deps] :as ast}]
;; FIXME: can't remove goog.require/goog.provide from goog sources easily
;; keeping them for CLJS for now although they are not needed in JS mode
Expand All @@ -367,10 +374,7 @@
(filter :goog-module))]

(when (seq module-deps)
(comp/emitln "goog.scope(function(){")
(doseq [{:keys [goog-module ns]} module-deps]
(comp/emitln (str " " (ana/munge-goog-module-lib name ns) " = goog.module.get('" goog-module "');")))
(comp/emitln "});")))
(emit-goog-module-gets name module-deps)))

(when (= :dev mode)
(doseq [dep deps
Expand Down
32 changes: 29 additions & 3 deletions src/main/shadow/cljs/repl.clj
Expand Up @@ -196,6 +196,9 @@
ns-info
(get-in state [:compiler-env ::ana/namespaces current-ns])

old-deps
(set (:deps ns-info))

{:keys [reload-deps] :as new-ns-info}
(-> (ns-form/merge-repl-require ns-info require-form)
;; (in-ns 'some.thing)
Expand All @@ -207,6 +210,9 @@
new-deps
(:deps new-ns-info)

added-deps
(into [] (remove old-deps) new-deps)

state
(-> state
(cond->
Expand All @@ -228,7 +234,9 @@
(build-api/compile-sources new-sources)
(load-macros-and-set-ns-info new-ns-info))

action
;; ensures all required namespaces are loaded
;; not just the one just added, just in case they aren't loaded
repl-require
(-> {:type :repl/require
:sources new-sources
:warnings (warnings-for-sources state new-sources)
Expand All @@ -245,13 +253,31 @@
(:ns rc)))
nil)))
(remove nil?)
(into [])))))]
(into [])))))

added-goog-modules
(->> added-deps
(filter symbol?)
(map #(data/get-source-by-provide state %))
(filter :goog-module))

actions
(-> [repl-require]
(cond->
;; need to pull goog.module sources into the current ns
;; cljs.user.goog$module = goog.modules.get("goog.object")
;; wrapped in a goog.scope so it doesn't complain about not being in module scope
(seq added-goog-modules)
(conj {:type :repl/invoke
:name "<eval>"
:internal true
:js (with-out-str (comp/emit-goog-module-gets current-ns added-goog-modules))})))]

(doto state
(output/flush-sources new-sources)
(async/wait-for-pending-tasks!))

(update-in state [:repl-state :repl-actions] conj action)
(update-in state [:repl-state :repl-actions] into actions)
))

(declare process-input)
Expand Down
10 changes: 10 additions & 0 deletions src/repl/shadow/cljs/repl_test.clj
Expand Up @@ -191,3 +191,13 @@
(repl/process-input "::m/max"))]

(pprint repl-state)))


(deftest test-repl-require-goog-module
(let [{:keys [repl-state] :as state}
(-> (basic-repl-setup)
(api/with-js-options {:js-provider :shadow})
(repl/process-input "(require '[goog.object :as gobj])")
(repl/process-input "(gobj/set \"foo\" \"x\" 1)"))]

(tap> repl-state)))

0 comments on commit e532400

Please sign in to comment.