Skip to content

Commit 238028c

Browse files
anmonteiroswannodette
anmonteiro
authored and
swannodette
committed
CLJS-2356: Self-host: circular dependency detection is not correct
1 parent 4b5fd54 commit 238028c

File tree

2 files changed

+122
-75
lines changed

2 files changed

+122
-75
lines changed

‎src/main/cljs/cljs/js.cljs‎

Lines changed: 91 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,7 @@
288288
"*load-fn* may only return a map or nil")
289289
(if resource
290290
(let [{:keys [lang source cache source-map file]} resource]
291-
(condp = lang
291+
(condp keyword-identical? lang
292292
:clj (do
293293
(pre-file-side-effects (:*compiler* bound-vars) aname file opts)
294294
(eval-str* bound-vars source name (assoc opts :cljs-file file)
@@ -374,88 +374,100 @@
374374
(debug-prn "Loading dependencies for" lib))
375375
(binding [ana/*cljs-dep-set* (vary-meta (conj (:*cljs-dep-set* bound-vars) lib)
376376
update-in [:dep-path] conj lib)]
377-
(assert (every? #(not (contains? (:*cljs-dep-set* bound-vars) %)) deps)
378-
(str "Circular dependency detected "
379-
(-> (:*cljs-dep-set* bound-vars) meta :dep-path)))
380-
(if (seq deps)
381-
(let [dep (first deps)
382-
opts' (-> opts
383-
(dissoc :context)
384-
(dissoc :ns))]
385-
(require bound-vars dep reload opts'
386-
(fn [res]
387-
(when (:verbose opts)
388-
(debug-prn "Loading result:" res))
389-
(if-not (:error res)
390-
(load-deps bound-vars ana-env lib (next deps) nil opts cb)
391-
(if-let [cljs-dep (let [cljs-ns (ana/clj-ns->cljs-ns dep)]
392-
(get {dep nil} cljs-ns cljs-ns))]
393-
(require bound-vars cljs-dep opts'
394-
(fn [res]
395-
(if (:error res)
396-
(cb res)
397-
(do
398-
(patch-alias-map (:*compiler* bound-vars) lib dep cljs-dep)
399-
(load-deps bound-vars ana-env lib (next deps) nil opts
400-
(fn [res]
401-
(if (:error res)
402-
(cb res)
403-
(cb (update res :aliased-loads assoc dep cljs-dep)))))))))
404-
(cb res))))))
405-
(cb {:value nil})))))
377+
(let [bound-vars (assoc bound-vars :*cljs-dep-set* ana/*cljs-dep-set*)]
378+
(if-not (every? #(not (contains? ana/*cljs-dep-set* %)) deps)
379+
(cb (wrap-error
380+
(ana/error ana-env
381+
(str "Circular dependency detected "
382+
(apply str
383+
(interpose " -> "
384+
(conj (-> ana/*cljs-dep-set* meta :dep-path)
385+
(some ana/*cljs-dep-set* deps))))))))
386+
(if (seq deps)
387+
(let [dep (first deps)
388+
opts' (-> opts
389+
(dissoc :context)
390+
(dissoc :ns))]
391+
(require bound-vars dep reload opts'
392+
(fn [res]
393+
(when (:verbose opts)
394+
(debug-prn "Loading result:" res))
395+
(if-not (:error res)
396+
(load-deps bound-vars ana-env lib (next deps) nil opts cb)
397+
(if-let [cljs-dep (let [cljs-ns (ana/clj-ns->cljs-ns dep)]
398+
(get {dep nil} cljs-ns cljs-ns))]
399+
(require bound-vars cljs-dep opts'
400+
(fn [res]
401+
(if (:error res)
402+
(cb res)
403+
(do
404+
(patch-alias-map (:*compiler* bound-vars) lib dep cljs-dep)
405+
(load-deps bound-vars ana-env lib (next deps) nil opts
406+
(fn [res]
407+
(if (:error res)
408+
(cb res)
409+
(cb (update res :aliased-loads assoc dep cljs-dep)))))))))
410+
(cb res))))))
411+
(cb {:value nil})))))))
406412

407413
(declare analyze-str*)
408414

409415
(defn- analyze-deps
410416
([bound-vars ana-env lib deps cb]
411417
(analyze-deps bound-vars ana-env lib deps nil cb))
412418
([bound-vars ana-env lib deps opts cb]
413-
(let [compiler @(:*compiler* bound-vars)]
414-
(binding [ana/*cljs-dep-set* (vary-meta (conj (:*cljs-dep-set* bound-vars) lib)
415-
update-in [:dep-path] conj lib)]
416-
(assert (every? #(not (contains? (:*cljs-dep-set* bound-vars) %)) deps)
417-
(str "Circular dependency detected "
418-
(-> (:*cljs-dep-set* bound-vars) meta :dep-path)))
419-
(if (seq deps)
420-
(let [dep (first deps)]
421-
(try
422-
((:*load-fn* bound-vars) {:name dep :path (ns->relpath dep)}
423-
(fn [resource]
424-
(assert (or (map? resource) (nil? resource))
425-
"*load-fn* may only return a map or nil")
426-
(if-not resource
427-
(if-let [cljs-dep (let [cljs-ns (ana/clj-ns->cljs-ns dep)]
428-
(get {dep nil} cljs-ns cljs-ns))]
429-
(do
430-
(patch-alias-map (:*compiler* bound-vars) lib dep cljs-dep)
431-
(analyze-deps bound-vars ana-env lib (cons cljs-dep (next deps)) opts
432-
(fn [res]
433-
(if (:error res)
434-
(cb res)
435-
(cb (update res :aliased-loads assoc dep cljs-dep))))))
436-
(cb (wrap-error
419+
(binding [ana/*cljs-dep-set* (vary-meta (conj (:*cljs-dep-set* bound-vars) lib)
420+
update-in [:dep-path] conj lib)]
421+
(let [compiler @(:*compiler* bound-vars)
422+
bound-vars (assoc bound-vars :*cljs-dep-set* ana/*cljs-dep-set*)]
423+
(if-not (every? #(not (contains? ana/*cljs-dep-set* %)) deps)
424+
(cb (wrap-error
425+
(ana/error ana-env
426+
(str "Circular dependency detected "
427+
(apply str
428+
(interpose " -> "
429+
(conj (-> ana/*cljs-dep-set* meta :dep-path)
430+
(some ana/*cljs-dep-set* deps))))))))
431+
(if (seq deps)
432+
(let [dep (first deps)]
433+
(try
434+
((:*load-fn* bound-vars) {:name dep :path (ns->relpath dep)}
435+
(fn [resource]
436+
(assert (or (map? resource) (nil? resource))
437+
"*load-fn* may only return a map or nil")
438+
(if-not resource
439+
(if-let [cljs-dep (let [cljs-ns (ana/clj-ns->cljs-ns dep)]
440+
(get {dep nil} cljs-ns cljs-ns))]
441+
(do
442+
(patch-alias-map (:*compiler* bound-vars) lib dep cljs-dep)
443+
(analyze-deps bound-vars ana-env lib (cons cljs-dep (next deps)) opts
444+
(fn [res]
445+
(if (:error res)
446+
(cb res)
447+
(cb (update res :aliased-loads assoc dep cljs-dep))))))
448+
(cb (wrap-error
449+
(ana/error ana-env
450+
(ana/error-message :undeclared-ns
451+
{:ns-sym dep :js-provide (name dep)})))))
452+
(let [{:keys [name lang source file]} resource]
453+
(condp keyword-identical? lang
454+
:clj (do
455+
(pre-file-side-effects (:*compiler* bound-vars) name file opts)
456+
(analyze-str* bound-vars source name (assoc opts :cljs-file file)
457+
(fn [res]
458+
(post-file-side-effects file opts)
459+
(if-not (:error res)
460+
(analyze-deps bound-vars ana-env lib (next deps) opts cb)
461+
(cb res)))))
462+
:js (analyze-deps bound-vars ana-env lib (next deps) opts cb)
463+
(wrap-error
437464
(ana/error ana-env
438-
(ana/error-message :undeclared-ns
439-
{:ns-sym dep :js-provide (name dep)})))))
440-
(let [{:keys [name lang source file]} resource]
441-
(condp = lang
442-
:clj (do
443-
(pre-file-side-effects (:*compiler* bound-vars) name file opts)
444-
(analyze-str* bound-vars source name (assoc opts :cljs-file file)
445-
(fn [res]
446-
(post-file-side-effects file opts)
447-
(if-not (:error res)
448-
(analyze-deps bound-vars ana-env lib (next deps) opts cb)
449-
(cb res)))))
450-
:js (analyze-deps bound-vars ana-env lib (next deps) opts cb)
451-
(wrap-error
452-
(ana/error ana-env
453-
(str "Invalid :lang specified " lang ", only :clj or :js allowed"))))))))
454-
(catch :default cause
455-
(cb (wrap-error
456-
(ana/error ana-env
457-
(str "Could not analyze dep " dep) cause))))))
458-
(cb {:value nil}))))))
465+
(str "Invalid :lang specified " lang ", only :clj or :js allowed"))))))))
466+
(catch :default cause
467+
(cb (wrap-error
468+
(ana/error ana-env
469+
(str "Could not analyze dep " dep) cause))))))
470+
(cb {:value nil})))))))
459471

460472
(defn- load-macros [bound-vars k macros lib reload reloads opts cb]
461473
(if (seq macros)
@@ -738,6 +750,7 @@
738750
:*data-readers* tags/*cljs-data-readers*
739751
:*passes* (or (:passes opts) ana/*passes*)
740752
:*analyze-deps* (:analyze-deps opts true)
753+
:*cljs-dep-set* ana/*cljs-dep-set*
741754
:*load-macros* (:load-macros opts true)
742755
:*load-fn* (or (:load opts) *load-fn*)
743756
:*eval-fn* (or (:eval opts) *eval-fn*)}
@@ -845,6 +858,7 @@
845858
{:*compiler* state
846859
:*data-readers* tags/*cljs-data-readers*
847860
:*analyze-deps* (:analyze-deps opts true)
861+
:*cljs-dep-set* ana/*cljs-dep-set*
848862
:*load-macros* (:load-macros opts true)
849863
:*load-fn* (or (:load opts) *load-fn*)
850864
:*eval-fn* (or (:eval opts) *eval-fn*)}
@@ -972,6 +986,7 @@
972986
(compile-str*
973987
{:*compiler* state
974988
:*data-readers* tags/*cljs-data-readers*
989+
:*cljs-dep-set* ana/*cljs-dep-set*
975990
:*analyze-deps* (:analyze-deps opts true)
976991
:*load-macros* (:load-macros opts true)
977992
:*load-fn* (or (:load opts) *load-fn*)
@@ -1140,6 +1155,7 @@
11401155
{:*compiler* state
11411156
:*data-readers* tags/*cljs-data-readers*
11421157
:*analyze-deps* (:analyze-deps opts true)
1158+
:*cljs-dep-set* ana/*cljs-dep-set*
11431159
:*load-macros* (:load-macros opts true)
11441160
:*load-fn* (or (:load opts) *load-fn*)
11451161
:*eval-fn* (or (:eval opts) *eval-fn*)}

‎src/test/self/self_host/test.cljs‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1228,6 +1228,37 @@
12281228
(is (re-find #"goog\.require\('cljs.x'\)" value))
12291229
(inc! l))))))
12301230

1231+
(deftest test-cljs-2356
1232+
(async done
1233+
(let [st (cljs/empty-state)
1234+
load (fn [{:keys [name macros]} cb]
1235+
(cb (cond
1236+
(= name 'circular.a)
1237+
{:lang :clj
1238+
:source "(ns circular.a (:require circular.b))"}
1239+
1240+
(= name 'circular.b)
1241+
{:lang :clj
1242+
:source "(ns circular.b (:require circular.a))"})))
1243+
l (latch 2 done)]
1244+
(binding [ana/*cljs-dep-set* (with-meta #{} {:dep-path []})]
1245+
(cljs.js/compile-str st "(ns circular.a (:require circular.b))" nil
1246+
{:load load}
1247+
(fn [{:keys [error value] :as m}]
1248+
(is (some? error))
1249+
(is (= "Circular dependency detected circular.a -> circular.b -> circular.a"
1250+
(.-message error)))
1251+
(inc! l))))
1252+
(binding [ana/*cljs-dep-set* (with-meta #{} {:dep-path []})]
1253+
(cljs.js/eval-str st "(ns circular.a (:require circular.b))" nil
1254+
{:load load
1255+
:eval node-eval}
1256+
(fn [{:keys [error value] :as m}]
1257+
(is (some? error))
1258+
(is (= "Circular dependency detected circular.a -> circular.b -> circular.a"
1259+
(.-message error)))
1260+
(inc! l)))))))
1261+
12311262
(defn -main [& args]
12321263
(run-tests))
12331264

0 commit comments

Comments
 (0)