CLJS-2334: Also gather dependencies from foreign-libs that are modules

Author: anmonteiro

src/main/clojure/cljs/closure.clj

@@ -2198,7 +2198,8 @@
          (next (json/read-str (str iw))))
        (do
          (when-not (.isAlive proc)
-           (println (str ew)))
+           (binding [*out* *err*]
+             (println (str ew))))
          [])))))
 
 (defn node-inputs
@@ -2330,6 +2331,9 @@
       (ana/warning :unsupported-preprocess-value @env/*compiler* js-module)
       js-module)))
 
+(defn- to-absolute-path [^String file-str]
+  (.getAbsolutePath (io/file file-str)))
+
 (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
@@ -2347,15 +2351,20 @@
                                                       (map :module-type js-modules)))]
                   (ana/warning :unsupported-js-module-type @env/*compiler* unsupported))
               ;; Load all modules - add :source so preprocessing and conversion can access it
-              js-modules (map (fn [lib]
-                                (let [js (deps/load-foreign-library lib)]
-                                  (assoc js :source (deps/-source js opts))))
-                              js-modules)
-              js-modules (map (fn [js]
-                                (if (:preprocess js)
-                                  (preprocess-js js opts)
-                                  js))
-                              js-modules)
+              js-modules (into []
+                           (comp
+                             (map (fn [lib]
+                                    (let [js (deps/load-foreign-library lib)]
+                                      (assoc js :source (deps/-source js opts)))))
+                             (map (fn [js]
+                                    (if (:preprocess js)
+                                      (preprocess-js js opts)
+                                      js)))
+                             (map (fn [js]
+                                    (cond-> (update-in js [:file] to-absolute-path)
+                                      (some? (:file-min js))
+                                      (update-in [:file-min] to-absolute-path)))))
+                           js-modules)
               js-modules (convert-js-modules js-modules opts)]
           ;; Write modules to disk, update compiler state and build new options
           (reduce (fn [new-opts {:keys [file module-type] :as ijs}]
@@ -2368,9 +2377,17 @@
                       (-> new-opts
                         (update-in [:libs] (comp vec conj) (:out-file ijs))
                         ;; 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)))
+                        (update-in [:foreign-libs]
+                          (fn [libs]
+                            (into []
+                              (remove #(= (to-absolute-path (:file %)) file))
+                              libs)))
+                        (update-in [:ups-foreign-libs]
+                          (fn [libs]
+                            (into []
+                              (remove #(= (to-absolute-path (:file %)) (to-absolute-path file)))
+                              libs))))))
+            opts js-modules)))
       opts)))
 
 (defn- load-data-reader-file [mappings ^java.net.URL url]
@@ -2437,25 +2454,30 @@
         requires (set (mapcat deps/-requires js-sources))
         ;; Select Node files that are required by Cljs code,
         ;; and create list of all their dependencies
-        node-required (set/intersection (set (keys top-level)) requires)]
-    (let [opts (-> opts
+        node-required (set/intersection (set (keys top-level)) requires)
+        expanded-libs (expand-libs (:foreign-libs opts))
+        opts (-> opts
                  (update :foreign-libs
                    (fn [libs]
                      (into (if (= target :nodejs)
                              []
                              (index-node-modules node-required))
-                       (expand-libs libs))))
+                       (into expanded-libs
+                         (node-inputs (filter (fn [{:keys [module-type]}]
+                                                (and (some? module-type)
+                                                  (not= module-type :amd)))
+                                        expanded-libs))))))
                  process-js-modules)]
-      (swap! compiler-env (fn [cenv]
-                            (-> cenv
-                              ;; we need to also track the whole top level - this is to support
-                              ;; cljs.analyze/analyze-deps, particularly in REPL contexts - David
-                              (merge {:js-dependency-index (deps/js-dependency-index opts)})
-                              (update-in [:node-module-index] (fnil into #{})
-                                (if (= target :nodejs)
-                                  (map str node-required)
-                                  (map str (keys top-level)))))))
-      opts)))
+    (swap! compiler-env (fn [cenv]
+                          (-> cenv
+                            ;; we need to also track the whole top level - this is to support
+                            ;; cljs.analyze/analyze-deps, particularly in REPL contexts - David
+                            (merge {:js-dependency-index (deps/js-dependency-index opts)})
+                            (update-in [:node-module-index] (fnil into #{})
+                              (if (= target :nodejs)
+                                (map str node-required)
+                                (map str (keys top-level)))))))
+    opts))
 
 (defn build
   "Given a source which can be compiled, produce runnable JavaScript."

src/test/cljs_build/foreign_libs_cljs_2334/core.cljs

@@ -0,0 +1,6 @@
+(ns foreign-libs-cljs-2334.core
+  (:require [mylib]))
+
+(enable-console-print!)
+
+(println "mylib:" mylib)

src/test/cljs_build/foreign_libs_cljs_2334/lib.js

@@ -0,0 +1,5 @@
+import leftPad from 'left-pad';
+
+export var lp = function() {
+  return leftPad(42, 5, 0);
+}

src/test/clojure/cljs/build_api_tests.clj

@@ -497,5 +497,29 @@
     (test/delete-out-files out)
     (build/build (build/inputs (io/file inputs "foreign_libs_dir_test/core.cljs")) opts)
     (is (.exists (io/file out "src/test/cljs_build/foreign-libs-dir/vendor/lib.js")))
-    (is (true? (boolean (re-find #"goog\.provide\(\"module\$src\$test\$cljs_build\$foreign_libs_dir\$vendor\$lib\"\)"
+    (is (true? (boolean (re-find #"goog\.provide\(\"module\$[A-Za-z0-9$_]+?src\$test\$cljs_build\$foreign_libs_dir\$vendor\$lib\"\)"
                           (slurp (io/file out "src/test/cljs_build/foreign-libs-dir/vendor/lib.js"))))))))
+
+(deftest cljs-1883-test-foreign-libs-use-relative-path
+  (test/delete-node-modules)
+  (let [out "cljs-2334-out"
+        root (io/file "src" "test" "cljs_build")
+        opts {:foreign-libs
+              [{:file (str (io/file root "foreign_libs_cljs_2334" "lib.js"))
+                :module-type :es6
+                :provides  ["mylib"]}]
+              :npm-deps {:left-pad "1.1.3"}
+              :install-deps true
+              :output-dir (str out)}]
+    (test/delete-out-files out)
+    (build/build (build/inputs (io/file root "foreign_libs_cljs_2334")) opts)
+    (let [foreign-lib-file (io/file out (-> opts :foreign-libs first :file))]
+      (is (.exists foreign-lib-file))
+      ;; assert Closure finds and processes the left-pad dep in node_modules
+      ;; if it can't be found the require will be issued to module$left_pad
+      ;; so we assert it's of the form module$path$to$node_modules$left_pad$index
+      (is (some? (re-find
+                   #"(?s).*?goog\.require\(\"[A-Za-z0-9$_]+?node_modules\$left_pad\$index\"\);.*"
+                   (slurp foreign-lib-file)))))
+    (test/delete-out-files out)
+    (test/delete-node-modules)))

src/test/clojure/cljs/module_processing_tests.clj

@@ -1,13 +1,15 @@
 (ns cljs.module-processing-tests
   (:require [clojure.java.io :as io]
             [cljs.closure :as closure]
-            [clojure.test :refer :all]
+            [clojure.string :as string]
+            [clojure.test :refer [deftest is]]
             [cljs.env :as env]
             [cljs.analyzer :as ana]
             [cljs.compiler :as comp]
             [cljs.js-deps :as deps]
             [cljs.util :as util]
-            [cljs.test-util :as test]))
+            [cljs.test-util :as test])
+  (:import [java.io File]))
 
 ;; Hard coded JSX transform for the test case
 (defn preprocess-jsx [ijs _]
@@ -23,6 +25,24 @@
                             "React.createElement(\"circle\", {cx:\"100px\", cy:\"100px\", r:\"100px\", fill:this.props.color})"
                             ")"))))
 
+(defn absolute-module-path
+  ([relpath]
+   (absolute-module-path relpath false))
+  ([relpath code?]
+   (let [filename (as-> (subs relpath (inc (.lastIndexOf relpath "/"))) $
+                    (string/replace $ "_" "-")
+                    (subs $ 0 (.lastIndexOf $ ".")))
+         dirname (as-> (io/file relpath) $
+                   (.getAbsolutePath $)
+                   (subs $ 0 (.lastIndexOf $ (str File/separator)))
+                   (string/replace $ "/" "$")
+                   ;; Windows
+                   (string/replace $ "\\" "$")
+                   (if code?
+                     (string/replace $ ":" "_")
+                     (string/replace $ ":" "-")))]
+     (str "module" (when-not (.startsWith dirname "$") "$") dirname "$" filename))))
+
 (defmethod closure/js-transforms :jsx [ijs opts]
   (preprocess-jsx ijs opts))
 
@@ -47,10 +67,9 @@
                                  :preprocess  :jsx}]
                  :closure-warnings {:non-standard-jsdoc :off}})))
         "processed modules are added to :libs"))
-
-    (is (= {"React" {:name "module$src$test$cljs$reactJS"
+    (is (= {"React" {:name (absolute-module-path "src/test/cljs/reactJS.js")
                      :module-type :commonjs}
-            "Circle" {:name "module$src$test$cljs$Circle"
+            "Circle" {:name (absolute-module-path "src/test/cljs/Circle.js")
                       :module-type :commonjs}}
            (:js-module-index @cenv))
         "Processed modules are added to :js-module-index")))
@@ -74,13 +93,14 @@
                   :closure-warnings {:non-standard-jsdoc :off}})))
           "processed modules are added to :libs")
 
-      (is (= {"es6-hello" {:name "module$src$test$cljs$es6-hello"
+      (is (= {"es6-hello" {:name (absolute-module-path "src/test/cljs/es6_hello.js")
                            :module-type :es6}}
              (:js-module-index @cenv))
           "Processed modules are added to :js-module-index")
 
-      (is (= "goog.provide(\"module$src$test$cljs$es6_hello\");var sayHello$$module$src$test$cljs$es6_hello=function(){console.log(\"Hello, world!\")};module$src$test$cljs$es6_hello.sayHello=sayHello$$module$src$test$cljs$es6_hello"
-             (slurp "out/src/test/cljs/es6_hello.js"))))))
+      (is (re-find
+            #"goog.provide\(\"module\$[a-zA-Z0-9$_]+?src\$test\$cljs\$es6_hello\"\);"
+            (slurp "out/src/test/cljs/es6_hello.js"))))))
 
 (deftest test-module-name-substitution
   (test/delete-out-files)
@@ -93,19 +113,21 @@
                       (with-out-str
                         (comp/emit (ana/analyze (ana/empty-env) form))))
             crlf (if util/windows? "\r\n" "\n")
-            output (str "module$src$test$cljs$calculator.add((3),(4));" crlf)]
+            output (str (absolute-module-path "src/test/cljs/calculator.js" true) ".add((3),(4));" crlf)]
         (swap! cenv
                #(assoc % :js-dependency-index (deps/js-dependency-index opts)))
         (binding [ana/*cljs-ns* 'cljs.user]
           (is (= (compile '(ns my-calculator.core (:require [calculator :as calc :refer [subtract add] :rename {subtract sub}])))
                 (str "goog.provide('my_calculator.core');" crlf
                      "goog.require('cljs.core');" crlf
-                     "goog.require('module$src$test$cljs$calculator');" crlf)))
+                     "goog.require('" (absolute-module-path "src/test/cljs/calculator.js" true) "');"
+                     crlf)))
           (is (= (compile '(calc/add 3 4)) output))
           (is (= (compile '(calculator/add 3 4)) output))
           (is (= (compile '(add 3 4)) output))
           (is (= (compile '(sub 5 4))
-                (str "module$src$test$cljs$calculator.subtract((5),(4));" crlf))))))))
+                (str (absolute-module-path "src/test/cljs/calculator.js" true)
+                     ".subtract((5),(4));" crlf))))))))
 
 (deftest test-cljs-1822
   (test/delete-out-files)
@@ -132,9 +154,9 @@
                                  :preprocess  :jsx}]
                  :closure-warnings {:non-standard-jsdoc :off}})))
         "processed modules are added to :libs"))
-    (is (= {"React" {:name "module$src$test$cljs$react-min"
+    (is (= {"React" {:name (absolute-module-path "src/test/cljs/react-min.js")
                      :module-type :commonjs}
-            "Circle" {:name "module$src$test$cljs$Circle-min"
+            "Circle" {:name (absolute-module-path "src/test/cljs/Circle-min.js")
                       :module-type :commonjs}}
            (:js-module-index @cenv))
         "Processed modules are added to :js-module-index")))
@@ -161,9 +183,9 @@
                  :closure-warnings {:non-standard-jsdoc :off}})))
         "processed modules are added to :libs"))
 
-    (is (= {"React" {:name "module$src$test$cljs$reactJS"
+    (is (= {"React" {:name (absolute-module-path "src/test/cljs/reactJS.js")
                      :module-type :commonjs}
-            "Circle" {:name "module$src$test$cljs$Circle"
+            "Circle" {:name (absolute-module-path "src/test/cljs/Circle.js")
                       :module-type :commonjs}}
            (:js-module-index @cenv))
         "Processed modules are added to :js-module-index")))