Skip to content

Commit 27c27e0

Browse files
mfikesdnolen
authored andcommitted
CLJS-1677: Requiring [goog] breaks an :advanced build, but the compiler exits successfully
This one is a little tricky: Since goog is added as an implicit dependency at the end of the compilation pipeline, there is no collision to detect when parsing an ns form that explicitly requires goog. You could handle this situation by filtering goog when parsing the ns form, but this leads to a challenging corner case where goog is required and also aliased: In that case you want to preserve goog and its alias so that the alias machinery works properly. But this means that goog will be duplicated later when add-goog-base is called. This patch takes the alternative approach of patching things up at the end: It filters out any existing goog dep before consing it onto the list of deps, placing a remove-goog-base prior to add-goog-base in the pipeline. This fixes the duplicate problem, but oddly results in JSC_MISSING_PROVIDE_ERROR. required "goog" namespace never provided if goog.require('goog') is emitted into the JavaScript. This final aspect is patched up to ensure that this is not emitted. A unit test is added covering the alias case.
1 parent 2e15a5c commit 27c27e0

File tree

3 files changed

+14
-2
lines changed

3 files changed

+14
-2
lines changed

src/main/clojure/cljs/closure.clj

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1080,6 +1080,10 @@
10801080
; - ns-info -> ns -> cljs file relpath -> js relpath
10811081
(merge opts {:output-file (comp/rename-to-js (util/ns->relpath (:ns ns-info)))})))))))))
10821082

1083+
(defn remove-goog-base
1084+
[inputs]
1085+
(remove #(= (:provides %) ["goog"]) inputs))
1086+
10831087
(defn add-goog-base
10841088
[inputs]
10851089
(cons (javascript-file nil (io/resource "goog/base.js") ["goog"] nil)
@@ -2904,6 +2908,7 @@
29042908
(cond-> (= :nodejs (:target opts)) (concat [(-compile (io/resource "cljs/nodejs.cljs") opts)]))
29052909
deps/dependency-order
29062910
(add-preloads opts)
2911+
remove-goog-base
29072912
add-goog-base
29082913
(cond-> (= :nodejs (:target opts)) (concat [(-compile (io/resource "cljs/nodejscli.cljs") opts)]))
29092914
(->> (map #(source-on-disk opts %)) doall)

src/main/clojure/cljs/compiler.cljc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1169,7 +1169,8 @@
11691169
(emitln "goog.require('" (munge lib) "', 'reload-all');")
11701170

11711171
:else
1172-
(emitln "goog.require('" (munge lib) "');")))
1172+
(when-not (= lib 'goog)
1173+
(emitln "goog.require('" (munge lib) "');"))))
11731174
(doseq [lib node-libs]
11741175
(emitln (munge ns-name) "."
11751176
(ana/munge-node-lib lib)

src/test/cljs/cljs/ns_test.cljs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@
1111
(:require-macros [clojure.core :as lang :refer [when when-let] :rename {when always
1212
when-let always-let}]
1313
[cljs.test :refer [deftest is]])
14-
(:require [cljs.test]
14+
(:require [goog :as goog-alias]
15+
[cljs.test]
1516
[cljs.ns-test.foo :refer [baz]]
1617
[clojure.set :as s :refer [intersection] :rename {intersection itsc}]
1718
[cljs.analyzer :as ana])
@@ -36,3 +37,8 @@
3637
(is (= (always true 42) 42))
3738
(is (= (core-mapv inc [1 2]) [2 3]))
3839
(is (= (always-let [foo 42] foo) 42)))
40+
41+
(deftest test-cljs-1677
42+
(is (.isNumber js/goog 3))
43+
(is (goog/isNumber 3))
44+
(is (goog-alias/isNumber 3)))

0 commit comments

Comments
 (0)