Skip to content

Commit

Permalink
CLJS-2389: Update Closure-compiler
Browse files Browse the repository at this point in the history
New Closure has three changes that affect ClojureScript:

Closure doesn't add goog.provide/require calls to the processed
files. Goog.provide call is used by ClojureScript compiler to check
which names are available. Easiest fix to be seems to emit the
call ourselves instead of trying to find places where it is needed.

Another change is that all CJS exports are now under `default` property,
so compiler needs to emit ["default"] when accessing vars from JS
modules.

Closure now uses ES2017 as default languageIn option, and some
optimization passes don't work with that. We can default to ES5 as JS
modules are transpiled separately.

Advanced optimization now removes unncessary backslashes from regex
literals, so pr-str test was changed.
google/closure-compiler@179b62c

This commit also adds new closure-snapshot lein profile which can be
used to test ClojureScript with the latest Closure snapshot, e.g.

    lein with-profile +closure-snapshot repl
  • Loading branch information
Deraen authored and swannodette committed Feb 10, 2018
1 parent 2f9e50c commit 72e2ab6
Show file tree
Hide file tree
Showing 10 changed files with 161 additions and 84 deletions.
4 changes: 2 additions & 2 deletions deps.edn
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@
org.clojure/spec.alpha {:mvn/version "0.1.143"}
org.clojure/core.specs.alpha {:mvn/version "0.1.24"}
org.clojure/data.json {:mvn/version "0.2.6"}
com.google.javascript/closure-compiler-unshaded {:mvn/version "v20180101"}
com.google.javascript/closure-compiler-unshaded {:mvn/version "v20180204"}
org.clojure/google-closure-library {:mvn/version "0.0-20170809-b9c14c6b"}
org.mozilla/rhino {:mvn/version "1.7R5"}}
:aliases
{:test {:extra-paths ["src/test/cljs" "src/test/cljs_build" "src/test/cljs_cp"
"src/test/clojure" "src/test/self"]}}}
"src/test/clojure" "src/test/self"]}}}
2 changes: 1 addition & 1 deletion pom.template.xml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
<dependency>
<groupId>com.google.javascript</groupId>
<artifactId>closure-compiler-unshaded</artifactId>
<version>v20170910</version>
<version>v20180204</version>
</dependency>
<dependency>
<groupId>org.clojure</groupId>
Expand Down
8 changes: 5 additions & 3 deletions project.clj
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,12 @@
[org.clojure/test.check "0.10.0-alpha2" :scope "test"]
[com.cognitect/transit-clj "0.8.300"]
[org.clojure/google-closure-library "0.0-20170809-b9c14c6b"]
[com.google.javascript/closure-compiler-unshaded "v20170910"]
[com.google.javascript/closure-compiler-unshaded "v20180204"]
[org.mozilla/rhino "1.7R5"]]
:profiles {:1.6 {:dependencies [[org.clojure/clojure "1.6.0"]]}
:uberjar {:aot :all :main clojure.main}}
:uberjar {:aot :all :main clojure.main}
:closure-snapshot {:dependencies [[com.google.javascript/closure-compiler-unshaded "1.0-SNAPSHOT"]]}}
:aliases {"test-all" ["with-profile" "test,1.5:test,1.6" "test"]
"check-all" ["with-profile" "1.5:1.6" "check"]}
:min-lein-version "2.0.0")
:min-lein-version "2.0.0"
:repositories {"sonatype-snapshot" {:url "https://oss.sonatype.org/content/repositories/snapshots"}})
2 changes: 1 addition & 1 deletion script/bootstrap
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ set -e
CLOJURE_RELEASE="1.9.0-alpha17"
SPEC_ALPHA_RELEASE="0.1.143"
CORE_SPECS_ALPHA_RELEASE="0.1.24"
CLOSURE_RELEASE="20170910"
CLOSURE_RELEASE="20180204"
DJSON_RELEASE="0.2.6"
TRANSIT_RELEASE="0.8.300"
GCLOSURE_LIB_RELEASE="0.0-20170809-b9c14c6b"
Expand Down
70 changes: 49 additions & 21 deletions src/main/clojure/cljs/closure.clj
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,14 @@
[java.util.concurrent
TimeUnit LinkedBlockingDeque Executors CountDownLatch]
[com.google.javascript.jscomp CompilerOptions CompilationLevel
CompilerInput CompilerInput$ModuleType DependencyOptions
CompilerOptions$LanguageMode SourceMap$Format
SourceMap$DetailLevel ClosureCodingConvention SourceFile
Result JSError CheckLevel DiagnosticGroups
CommandLineRunner AnonymousFunctionNamingPolicy
JSModule SourceMap Es6RewriteModules VariableMap]
[com.google.javascript.jscomp.deps ModuleLoader$ResolutionMode]
JSModule SourceMap Es6RewriteModules VariableMap
ProcessCommonJSModules Es6RewriteScriptsToModules]
[com.google.javascript.jscomp.deps ModuleLoader$ResolutionMode ModuleNames]
[com.google.javascript.rhino Node]
[java.nio.file Path Paths Files StandardWatchEventKinds WatchKey
WatchEvent FileVisitor FileVisitResult]
Expand Down Expand Up @@ -108,7 +110,6 @@
:check-useless-code DiagnosticGroups/CHECK_USELESS_CODE
:check-variables DiagnosticGroups/CHECK_VARIABLES
:closure-dep-method-usage-checks DiagnosticGroups/CLOSURE_DEP_METHOD_USAGE_CHECKS
:common-js-module-load DiagnosticGroups/COMMON_JS_MODULE_LOAD
:conformance-violations DiagnosticGroups/CONFORMANCE_VIOLATIONS
:const DiagnosticGroups/CONST
:constant-property DiagnosticGroups/CONSTANT_PROPERTY
Expand Down Expand Up @@ -226,7 +227,7 @@
:mapped AnonymousFunctionNamingPolicy/MAPPED
(throw (IllegalArgumentException. (str "Invalid :anon-fn-naming-policy value " policy " - only :off, :unmapped, :mapped permitted")))))))

(when-let [lang-key (:language-in opts)]
(when-let [lang-key (:language-in opts :ecmascript5)]
(.setLanguageIn compiler-options (lang-key->lang-mode lang-key)))

(when-let [lang-key (:language-out opts)]
Expand Down Expand Up @@ -1695,24 +1696,39 @@
(assoc-in [:closure-warnings :non-standard-jsdoc] :off)
(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 module-type->keyword [^CompilerInput$ModuleType module-type]
(case (.name module-type)
"NONE" :none
"GOOG" :goog
"ES6" :es6
"COMMONJS" :commonjs
"JSON" :json
"IMPORTED_SCRIPT" :imported-script))

(defn add-converted-source
[closure-compiler result-nodes opts {:keys [file-min file] :as ijs}]
[closure-compiler inputs-by-name opts {:keys [file-min file requires] :as ijs}]
(let [processed-file (if-let [min (and (#{:advanced :simple} (:optimizations opts))
file-min)]
min
file)
processed-file (string/replace processed-file "\\" "/")]
(assoc ijs :source
(.toSource closure-compiler ^Node (get result-nodes processed-file)))))
processed-file (string/replace processed-file "\\" "/")
^CompilerInput input (get inputs-by-name processed-file)
^Node ast-root (.getAstRoot input closure-compiler)
module-name (ModuleNames/fileToModuleName processed-file)
;; getJsModuleType returns NONE for ES6 files, but getLoadsFlags module returns es6 for those
module-type (or (some-> (.get (.getLoadFlags input) "module") keyword)
(module-type->keyword (.getJsModuleType input)))]
(assoc ijs
:module-type module-type
:source
;; Add goog.provide/require calls ourselves, not emited by Closure since
;; https://github.com/google/closure-compiler/pull/2641
(str
"goog.provide(\"" module-name "\");\n"
(apply str (map (fn [n]
(str "goog.require(\"" n "\");\n"))
(.getRequires input)))
(.toSource closure-compiler ast-root)))))

(defn convert-js-modules
"Takes a list JavaScript modules as an IJavaScript and rewrites them into a Google
Expand All @@ -1724,18 +1740,26 @@
^CompilerOptions options (doto (make-convert-js-module-options opts)
(.setProcessCommonJSModules true)
(.setLanguageIn (lang-key->lang-mode :ecmascript6))
(.setLanguageOut (lang-key->lang-mode (:language-out opts :ecmascript3))))
(.setLanguageOut (lang-key->lang-mode (:language-out opts :ecmascript3)))
(.setDependencyOptions (doto (DependencyOptions.)
(.setDependencySorting true))))
_ (when (= (:target opts) :nodejs)
(.setPackageJsonEntryNames options ^List '("module", "main")))
closure-compiler (doto (make-closure-compiler)
(.init externs source-files options))
_ (.parse closure-compiler)
_ (report-failure (.getResult closure-compiler))
root (.getRoot closure-compiler)]
(.process (Es6RewriteModules. closure-compiler)
(.getFirstChild root) (.getSecondChild root))
^Node extern-and-js-root (.getRoot closure-compiler)
^Node extern-root (.getFirstChild extern-and-js-root)
^Node js-root (.getSecondChild extern-and-js-root)
inputs-by-name (into {} (map (juxt #(.getName %) identity) (vals (.getInputsById closure-compiler))))]

(.process (Es6RewriteScriptsToModules. closure-compiler) extern-root js-root)
(.process (Es6RewriteModules. closure-compiler) extern-root js-root)
(.process (ProcessCommonJSModules. closure-compiler) extern-root js-root)

(map (partial add-converted-source
closure-compiler (get-closure-sources closure-compiler) opts)
closure-compiler inputs-by-name opts)
js-modules)))

(defmulti js-transforms
Expand Down Expand Up @@ -2272,6 +2296,8 @@
(map (fn [{:strs [file provides]}] file
(merge
{:file file
;; Just tag everything es6 here, add-converted-source will
;; ask the real type, CJS/ES6, from Closure.
:module-type :es6}
(when provides
{:provides provides}))))
Expand Down Expand Up @@ -2472,6 +2498,8 @@
(reduce (fn [new-opts {:keys [file module-type] :as ijs}]
(let [ijs (write-javascript opts ijs)
module-name (-> (deps/load-library (:out-file ijs)) first :provides first)]
(swap! env/*compiler*
#(assoc-in % [:js-namespaces module-name] {:module-type module-type}))
(doseq [provide (:provides ijs)]
(swap! env/*compiler*
#(update-in % [:js-module-index] assoc provide {:name module-name
Expand Down
22 changes: 18 additions & 4 deletions src/main/clojure/cljs/compiler.cljc
Original file line number Diff line number Diff line change
Expand Up @@ -378,11 +378,25 @@
;; but not standalone `default` variable names
;; as they're not valid ES5 - Antonio
(some? (namespace var-name)))
(set/difference ana/es5-allowed))]
(set/difference ana/es5-allowed))
js-module (get-in cenv [:js-namespaces (or (namespace var-name) (name var-name))])
info (cond-> info
(not= form 'js/-Infinity) (munge reserved))]
(emit-wrap env
(emits
(cond-> info
(not= form 'js/-Infinity) (munge reserved))))))))))
(case (:module-type js-module)
;; Closure exports CJS exports through default property
:commonjs
(if (namespace var-name)
(emits (munge (namespace var-name) reserved) "[\"default\"]." (munge (name var-name) reserved))
(emits (munge (name var-name) reserved) "[\"default\"]"))

;; Emit bracket notation for default prop access instead of dot notation
:es6
(if (and (namespace var-name) (= "default" (name var-name)))
(emits (munge (namespace var-name) reserved) "[\"default\"]")
(emits info))

(emits info)))))))))

(defmethod emit* :var-special
[{:keys [env var sym meta] :as arg}]
Expand Down
6 changes: 3 additions & 3 deletions src/test/cljs/cljs/printing_test.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,9 @@
(is (= (pr-str (rest (conj cljs.core.PersistentQueue.EMPTY 1 2 3))) "(2 3)"))
(is (= "\"asdf\" \"asdf\"" (pr-str "asdf" "asdf")))
;; Different hash map order on self-host
(is (#{"[1 true {:a 2, :b #\"x\\\"y\"} #js [3 4]]"
"[1 true {:b #\"x\\\"y\", :a 2} #js [3 4]]"}
(pr-str [1 true {:a 2 :b #"x\"y"} (array 3 4)]))))
(is (#{"[1 true {:a 2, :b \"x\\\"y\"} #js [3 4]]"
"[1 true {:b \"x\\\"y\", :a 2} #js [3 4]]"}
(pr-str [1 true {:a 2 :b "x\"y"} (array 3 4)]))))
(testing "Testing print-str"
(is (= (print-str "asdf") "asdf")))
(testing "Testing println-str"
Expand Down
19 changes: 17 additions & 2 deletions src/test/cljs_build/npm_deps_test/string_requires.cljs
Original file line number Diff line number Diff line change
@@ -1,9 +1,24 @@
(ns npm-deps-test.string-requires
(:require [react :refer [createElement]]
["react-dom/server" :as ReactDOMServer]
["lodash-es/toArray" :refer [default] :rename {default toArray}]
["lodash-es/toFinite" :as toFinite]
["lodash-es/array" :as array]
[npm-deps-test.string-requires-in-classpath]))

(enable-console-print!)

(println "ReactDOMServer exists:" ReactDOMServer
(.-renderToString ReactDOMServer))
;; CJS namespace access
(println ReactDOMServer)

;; CJS method call
(ReactDOMServer/renderToString nil)

;; es6 default with refer rename
(toArray nil)

;; es6 :as and default
(toFinite/default nil)

;; es6
(array/findIndex #js [1 2] 2)
62 changes: 38 additions & 24 deletions src/test/clojure/cljs/build_api_tests.clj
Original file line number Diff line number Diff line change
Expand Up @@ -226,23 +226,35 @@
(build/build (build/inputs inputs) opts)
(is (not (nil? (re-find #"foreignA[\s\S]+foreignB" (slurp (io/file out "foo.js"))))))))))

(deftest test-npm-deps-simple
(test/delete-node-modules)
(spit (io/file "package.json") "{}")
(let [out (.getPath (io/file (test/tmp-dir) "npm-deps-test-out"))
{:keys [inputs opts]} {:inputs (str (io/file "src" "test" "cljs_build"))
:opts {:main 'npm-deps-test.core
:output-dir out
:optimizations :none
:install-deps true
:npm-deps {:left-pad "1.1.3"}
:foreign-libs [{:module-type :es6
:file "src/test/cljs/es6_dep.js"
:provides ["es6_calc"]}
{:module-type :es6
:file "src/test/cljs/es6_default_hello.js"
:provides ["es6_default_hello"]}]
:closure-warnings {:check-types :off}}}
cenv (env/default-compiler-env)]
(test/delete-out-files out)
(build/build (build/inputs (io/file inputs "npm_deps_test/core.cljs")) opts cenv)
(is (.exists (io/file out "node_modules/left-pad/index.js")))
(is (contains? (:js-module-index @cenv) "left-pad")))

(.delete (io/file "package.json"))
(test/delete-node-modules))

(deftest test-npm-deps
(test/delete-node-modules)
(spit (io/file "package.json") "{}")
(testing "simplest case, require"
(let [out (.getPath (io/file (test/tmp-dir) "npm-deps-test-out"))
{:keys [inputs opts]} {:inputs (str (io/file "src" "test" "cljs_build"))
:opts {:main 'npm-deps-test.core
:output-dir out
:optimizations :none
:install-deps true
:npm-deps {:left-pad "1.1.3"}
:closure-warnings {:check-types :off}}}
cenv (env/default-compiler-env)]
(test/delete-out-files out)
(build/build (build/inputs (io/file inputs "npm_deps_test/core.cljs")) opts cenv)
(is (.exists (io/file out "node_modules/left-pad/index.js")))
(is (contains? (:js-module-index @cenv) "left-pad"))))
(let [cenv (env/default-compiler-env)
out (.getPath (io/file (test/tmp-dir) "npm-deps-test-out"))
{:keys [inputs opts]} {:inputs (str (io/file "src" "test" "cljs_build"))
Expand All @@ -252,20 +264,21 @@
:install-deps true
:npm-deps {:react "15.6.1"
:react-dom "15.6.1"
:lodash-es "4.17.4"
:lodash "4.17.4"}
:closure-warnings {:check-types :off
:non-standard-jsdoc :off}}}]
(test/delete-out-files out)
(testing "mix of symbol & string-based requires"
(test/delete-out-files out)
(test/delete-node-modules)
(build/build (build/inputs (io/file inputs "npm_deps_test/string_requires.cljs")) opts cenv)
(is (.exists (io/file out "node_modules/react/react.js")))
(is (contains? (:js-module-index @cenv) "react"))
(is (contains? (:js-module-index @cenv) "react-dom/server")))

(testing "builds with string requires are idempotent"
(build/build (build/inputs (io/file inputs "npm_deps_test/string_requires.cljs")) opts cenv)
(is (not (nil? (re-find #"\.\.[\\/]node_modules[\\/]react-dom[\\/]server\.js" (slurp (io/file out "cljs_deps.js"))))))
(test/delete-out-files out)))
(is (not (nil? (re-find #"\.\.[\\/]node_modules[\\/]react-dom[\\/]server\.js" (slurp (io/file out "cljs_deps.js"))))))))

(.delete (io/file "package.json"))
(test/delete-node-modules))

Expand Down Expand Up @@ -500,8 +513,8 @@
(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\$[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"))))))))
(is (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)
Expand All @@ -516,13 +529,14 @@
: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))]
(let [foreign-lib-file (io/file out (-> opts :foreign-libs first :file))
index-js (slurp (io/file "cljs-2334-out" "node_modules" "left-pad" "index.js"))]
(is (.exists foreign-lib-file))
(is (re-find #"module\$.*\$node_modules\$left_pad\$index=" index-js))
(is (not (re-find #"module\.exports" index-js)))
;; 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)))))
(is (re-find #"module\$.*\$node_modules\$left_pad\$index\[\"default\"\]\(42,5,0\)" (slurp foreign-lib-file))))
(test/delete-out-files out)
(test/delete-node-modules)))
Loading

0 comments on commit 72e2ab6

Please sign in to comment.