cljs.diff

diff --git a/src/main/clojure/cljs/closure.clj b/src/main/clojure/cljs/closure.clj
index 898b43d..4ab6243 100644
--- a/src/main/clojure/cljs/closure.clj
+++ b/src/main/clojure/cljs/closure.clj
@@ -1495,11 +1495,20 @@
        :language-in :language-out])
     (set-options (CompilerOptions.))))
 
+(defn get-js-root [closure-compiler]
+  (.getSecondChild (.getRoot closure-compiler)))
+
+(defn get-closure-sources
+  "Gets map of source file name -> Node, for files in Closure Compiler js root."
+  [closure-compiler]
+  (let [source-nodes (.children (get-js-root closure-compiler))]
+    (into {} (map (juxt #(.getSourceFileName ^Node %) identity) source-nodes))))
+
+(defn add-converted-source [closure-compiler result-nodes {:keys [file] :as ijs}]
+  (assoc ijs :source (.toSource closure-compiler ^Node (get result-nodes file))))
+
 (defmethod convert-js-modules :commonjs [module-type js-modules opts]
   (let [^List externs '()
-        ;; FIXME: Should this contain other module types also?
-        ;; And both from foreign-libs and ups-foreign-libs (cljs-1682)?
-        ;; But probably other module-types can't be parsed by this compiler?
         ^List source-files (get-source-files js-modules)
         ^CompilerOptions options (doto (make-convert-js-module-options opts)
                                    (.setProcessCommonJSModules true)
@@ -1507,16 +1516,8 @@
         closure-compiler (doto (make-closure-compiler)
                            (.init externs source-files options))]
     (.parse closure-compiler)
-    (let [;; compiler root has two childs, externRoot and jsRoot
-          js-root (.getSecondChild (.getRoot closure-compiler))
-          ;; select root matching the ijs file
-          ;; source file => root
-          ;; FIXME: Should move this to a function as this is same as used by :es6
-          result-roots (into {} (map (juxt #(.getSourceFileName ^Node %) identity) (.children js-root)))]
-      (report-failure (.getResult closure-compiler))
-      (map (fn [{:keys [file] :as ijs}]
-             (assoc ijs :source (.toSource closure-compiler ^Node (get result-roots file))))
-           js-modules))))
+    (report-failure (.getResult closure-compiler))
+    (map (partial add-converted-source closure-compiler (get-closure-sources closure-compiler)) js-modules)))
 
 (defmethod convert-js-modules :es6 [module-type js-modules opts]
   (let [^List externs '()
@@ -1527,16 +1528,11 @@
         closure-compiler (doto (make-closure-compiler)
                            (.init externs source-files options))]
     (.parse closure-compiler)
-    (let [js-root (.getSecondChild (.getRoot closure-compiler))
-          result-roots (into {} (map (juxt #(.getSourceFileName ^Node %) identity) (.children js-root)))]
-      (report-failure (.getResult closure-compiler))
-      (map (fn [{:keys [file] :as ijs}]
-             (assoc ijs :source (.toSource closure-compiler ^Node (get result-roots file))))
-           js-modules))))
+    (report-failure (.getResult closure-compiler))
+    (map (partial add-converted-source closure-compiler (get-closure-sources closure-compiler)) js-modules)))
 
 (defmethod convert-js-modules :default [module-type js-modules opts]
-  ;; FIXME: Warning needs currently file information
-  (ana/warning :unsupported-js-module-type @env/*compiler* {:module-type module-type :file nil})
+  (ana/warning :unsupported-js-module-type @env/*compiler* (first js-modules))
   js-modules)
 
 (defmulti js-transforms
@@ -1830,22 +1826,30 @@
         (not (false? (:static-fns opts))) (assoc :static-fns true)
         (not (false? (:optimize-constants opts))) (assoc :optimize-constants true)))))
 
-(defn- process-js-modules*
-  [opts k]
-  (let [js-modules (filter :module-type (k opts))
-        ;; Load all modules
-        ;; Add :source so preprocessing and conversion can access it
+(defn process-js-modules
+  "Given the current compiler options, converts JavaScript modules to Google
+  Closure modules and writes them to disk. Adds mapping from original module
+  namespace to new module namespace to compiler env. Returns modified compiler
+  options where new modules are passed with :libs option."
+  [opts]
+  (let [;; Modules from both :foreign-libs (compiler options) and :ups-foreign-libs (deps.cljs)
+        ;; are processed together, so that files from both sources can depend on each other.
+        ;; e.g. commonjs module in :foreign-libs can depend on commonjs module from :ups-foreign-libs.
+        js-modules (filter :module-type (concat (:foreign-libs opts) (:ups-foreign-libs opts)))
+        ;; Load all modules - add :source so preprocessing and conversion can access it
         js-modules (map (fn [lib]
-                          (let [js (deps/load-foreign-library lib)
-                                js (assoc js :source (deps/-source js))
-                                js (if (:preprocess js)
-                                     (js-transforms js opts)
-                                     js)]
+                          (let [js (deps/load-foreign-library lib)]
+                            (assoc js :source (deps/-source js))))
+                        js-modules)
+        js-modules (map (fn [js]
+                          (if (:preprocess js)
+                            (js-transforms js opts)
                             js))
                         js-modules)
         ;; Conversion is done per module-type, because Compiler needs to process e.g. all CommonJS
         ;; modules on one go, so it can handle the dependencies between modules.
-        ;; FIXME: amd type should be grouped with commonjs.
+        ;; Amdjs modules are converted separate from CommonJS modules so they can't
+        ;; depend on each other.
         modules-per-type (group-by :module-type js-modules)
         js-modules (mapcat (fn [[module-type js-modules]]
                              (convert-js-modules module-type js-modules opts))
@@ -1860,22 +1864,11 @@
                     #(update-in % [:js-module-index] assoc provide module-name)))
                 (-> new-opts
                     (update-in [:libs] (comp vec conj) (:out-file ijs))
-                    (update-in [k]
-                      (comp vec (fn [libs] (remove #(= (:file %) file) libs)))))))
+                    ;; js-module might be defined in either, so update both
+                    (update-in [:foreign-libs] (comp vec (fn [libs] (remove #(= (:file %) file) libs))))
+                    (update-in [:ups-foreign-libs] (comp vec (fn [libs] (remove #(= (:file %) file) libs)))))))
             opts js-modules)))
 
-(defn process-js-modules
-  "Given the current compiler options, converts JavaScript modules to Google
-  Closure modules and writes them to disk. Adds mapping from original module
-  namespace to new module namespace to compiler env. Returns modified compiler
-  options where new modules are passed with :libs option."
-  [opts]
-  ;; FIXME: Probably both should be processed at once go, so that same type modules from both sources
-  ;; can depend on each other.
-  (-> opts
-      (process-js-modules* :foreign-libs)
-      (process-js-modules* :ups-foreign-libs)))
-
 (defn build
   "Given a source which can be compiled, produce runnable JavaScript."
   ([source opts]
diff --git a/src/test/clojure/cljs/module_processing_tests.clj b/src/test/clojure/cljs/module_processing_tests.clj
index 17af481..ceda629 100644
--- a/src/test/clojure/cljs/module_processing_tests.clj
+++ b/src/test/clojure/cljs/module_processing_tests.clj
@@ -31,6 +31,7 @@
   (alter-var-root #'cljs.js-deps/load-library (constantly (memoize cljs.js-deps/load-library*)))
 
   (is (= {:foreign-libs []
+          :ups-foreign-libs []
           :libs ["out/react.js"
                  "out/Circle.js"]
           :closure-warnings {:non-standard-jsdoc :off}}
@@ -49,4 +50,4 @@
   (is (= {"React" "module$src$test$cljs$react"
           "Circle" "module$src$test$cljs$Circle"}
          (:js-module-index @cenv))
-      "Processed modules are added to :js-module-index"))
\ No newline at end of file
+      "Processed modules are added to :js-module-index"))