CLJS-2286: Simplify JS module processing

Author: anmonteiro

.travis.yml

@@ -15,7 +15,7 @@ before_install:
 before_script:
   - script/bootstrap
   - mkdir -p builds/out-adv
-  - bin/cljsc src/test/cljs "{:optimizations :advanced :output-wrapper true :verbose true :compiler-stats true :parallel-build true :output-dir \"builds/out-adv\" :npm-deps {:lodash \"4.17.4\"} :closure-warnings {:non-standard-jsdoc :off :global-this :off} :install-deps true :foreign-libs [{:file \"src/test/cljs/calculator_global.js\" :provides [\"calculator\"] :global-exports {calculator Calculator}}]}" > builds/out-adv/core-advanced-test.js
+  - bin/cljsc src/test/cljs "{:optimizations :advanced :output-wrapper true :verbose true :compiler-stats true :parallel-build true :output-dir \"builds/out-adv\" :npm-deps {:lodash \"4.17.4\"} :closure-warnings {:non-standard-jsdoc :off :global-this :off} :install-deps true :foreign-libs [{:file \"src/test/cljs/calculator_global.js\" :provides [\"calculator\"] :global-exports {calculator Calculator}} {:file \"src/test/cljs/es6_dep.js\" :module-type :es6 :provides [\"es6_calc\"]} {:file \"src/test/cljs/calculator_amd.js\" :module-type :amd :provides [\"calculator_amd\"] :requires [\"es6_calc\"]}]}" > builds/out-adv/core-advanced-test.js
 
 script:
   - lein test

script/test

@@ -12,7 +12,7 @@ mkdir -p builds/out-adv
 possible=5
 ran=0
 
-if ! bin/cljsc src/test/cljs "{:optimizations :advanced :output-wrapper true :verbose true :compiler-stats true :parallel-build true :output-dir \"builds/out-adv\" :npm-deps {:lodash \"4.17.4\"} :closure-warnings {:non-standard-jsdoc :off :global-this :off} :install-deps true :foreign-libs [{:file \"src/test/cljs/calculator_global.js\" :provides [\"calculator\"] :global-exports {calculator Calculator}}]}" > builds/out-adv/core-advanced-test.js; then
+if ! bin/cljsc src/test/cljs "{:optimizations :advanced :output-wrapper true :verbose true :compiler-stats true :parallel-build true :output-dir \"builds/out-adv\" :npm-deps {:lodash \"4.17.4\"} :closure-warnings {:non-standard-jsdoc :off :global-this :off} :install-deps true :foreign-libs [{:file \"src/test/cljs/calculator_global.js\" :provides [\"calculator\"] :global-exports {calculator Calculator}} {:file \"src/test/cljs/es6_dep.js\" :module-type :es6 :provides [\"es6_calc\"]} {:file \"src/test/cljs/calculator_amd.js\" :module-type :amd :provides [\"calculator_amd\"] :requires [\"es6_calc\"]}]}" > builds/out-adv/core-advanced-test.js; then
   >&2 echo ClojureScript compilation failed
   exit 1
 fi;

script/test-simple

@@ -12,7 +12,7 @@ possible=5
 ran=0
 
 #bin/cljsc test >out/core-test.js
-if ! bin/cljsc src/test/cljs  "{:optimizations :simple :static-fns true :output-dir \"builds/out-simp\" :cache-analysis true :output-wrapper true :verbose true :compiler-stats true :npm-deps {:lodash \"4.17.4\"} :closure-warnings {:non-standard-jsdoc :off :global-this :off} :install-deps true :foreign-libs [{:file \"src/test/cljs/calculator_global.js\" :provides [\"calculator\"] :global-exports {calculator Calculator}}]}" > builds/out-simp/core-simple-test.js; then
+if ! bin/cljsc src/test/cljs  "{:optimizations :simple :static-fns true :output-dir \"builds/out-simp\" :cache-analysis true :output-wrapper true :verbose true :compiler-stats true :npm-deps {:lodash \"4.17.4\"} :closure-warnings {:non-standard-jsdoc :off :global-this :off} :install-deps true :foreign-libs [{:file \"src/test/cljs/calculator_global.js\" :provides [\"calculator\"] :global-exports {calculator Calculator}} {:file \"src/test/cljs/es6_dep.js\" :module-type :es6 :provides [\"es6_calc\"]} {:file \"src/test/cljs/calculator_amd.js\" :module-type :amd :provides [\"calculator_amd\"] :requires [\"es6_calc\"]}]}" > builds/out-simp/core-simple-test.js; then
   >&2 echo ClojureScript compilation failed
   exit 1
 fi;

src/main/clojure/cljs/closure.clj

@@ -1605,16 +1605,6 @@
            (js-source-file file (deps/-source lib))))
     js-modules))
 
-(defmulti convert-js-modules
-  "Takes a list JavaScript modules as an IJavaScript and rewrites them into a Google
-  Closure-compatible form. Returns list IJavaScript with the converted module
-  code set as source."
-  (fn [module-type js-modules opts]
-    (if (= module-type :amd)
-      ;; AMD modules are converted via CommonJS modules
-      :commonjs
-      module-type)))
-
 (defn make-convert-js-module-options [opts]
   (-> opts
     (select-keys
@@ -1642,25 +1632,18 @@
     (assoc ijs :source
       (.toSource closure-compiler ^Node (get result-nodes processed-file)))))
 
-(defmethod convert-js-modules :commonjs [module-type js-modules opts]
-  (let [^List externs '()
-        ^List source-files (get-source-files js-modules opts)
-        ^CompilerOptions options (doto (make-convert-js-module-options opts)
-                                   (.setProcessCommonJSModules true)
-                                   (.setTransformAMDToCJSModules (= module-type :amd)))
-        closure-compiler (doto (make-closure-compiler)
-                           (.init externs source-files options))]
-    (.parse closure-compiler)
-    (report-failure (.getResult closure-compiler))
-    (map (partial add-converted-source
-           closure-compiler (get-closure-sources closure-compiler) opts)
-      js-modules)))
-
-(defmethod convert-js-modules :es6 [module-type js-modules opts]
+(defn convert-js-modules
+  "Takes a list JavaScript modules as an IJavaScript and rewrites them into a Google
+  Closure-compatible form. Returns list IJavaScript with the converted module
+  code set as source."
+  [js-modules opts]
   (let [^List externs '()
         ^List source-files (get-source-files js-modules opts)
         ^CompilerOptions options (doto (make-convert-js-module-options opts)
                                    (.setProcessCommonJSModules true)
+                                   (.setTransformAMDToCJSModules
+                                     (boolean (some (fn [{:keys [module-type]}]
+                                                      (= module-type :amd)) js-modules)))
                                    (.setLanguageIn (lang-key->lang-mode :ecmascript6))
                                    (.setLanguageOut (lang-key->lang-mode (:language-out opts :ecmascript3))))
         closure-compiler (doto (make-closure-compiler)
@@ -1674,10 +1657,6 @@
            closure-compiler (get-closure-sources closure-compiler) opts)
       js-modules)))
 
-(defmethod convert-js-modules :default [module-type js-modules opts]
-  (ana/warning :unsupported-js-module-type @env/*compiler* (first js-modules))
-  js-modules)
-
 (defmulti js-transforms
   "Takes an IJavaScript with the source code set as source, transforms the
   source code and returns an IJavascript with the new code set as source."
@@ -2327,7 +2306,10 @@
     (if (seq js-modules)
       (util/measure (:compiler-stats opts)
         "Process JS modules"
-        (let [;; Load all modules - add :source so preprocessing and conversion can access it
+        (let [_ (when-let [unsupported (first (filter (complement #{:es6 :commonjs :amd})
+                                                      (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))))
@@ -2337,15 +2319,7 @@
                                   (preprocess-js 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.
-              ;; 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))
-                                 modules-per-type)]
-
+              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] :as ijs}]
                     (let [ijs (write-javascript opts ijs)

src/test/cljs/calculator_amd.js

@@ -0,0 +1,5 @@
+define({
+  add: function(x, y){
+    return x + y;
+  }
+});

src/test/cljs/cljs/npm_deps_test.cljs

@@ -2,7 +2,8 @@
   (:refer-clojure :exclude [array vector])
   (:require [cljs.test :refer [deftest is]]
             ["lodash/array" :as array :refer [slice] :rename {slice slc}]
-            [calculator :as vector :refer [add] :rename {add plus}]))
+            [calculator :as vector :refer [add] :rename {add plus}]
+            [es6_calc]))
 
 (def array #js [1 2 3])
 
@@ -21,3 +22,6 @@
   (is (= (array-seq array) [1 2 3]))
   ;; same should happen with global-exports
   (is (= vector [1])))
+
+(deftest test-cljs-2286
+  (is (= 3 (es6_calc/calculator.add 1 2))))

src/test/cljs/es6_dep.js

@@ -0,0 +1,3 @@
+import * as calc from './calculator_amd';
+
+export var calculator = calc;