Skip to content

Commit

Permalink
CLJS-2592: :npm-deps using ES6 modules with .mjs extensions are not d…
Browse files Browse the repository at this point in the history 
…etected correctly

This adds a new :package-json-resolution option that can be used to
control which package.json entry fields are considered during JS module
resolution. When set, this flag will override the entries that would
otherwise be picked for the configured target.

The resulting behavior:

    {:target :nodejs}  => ("module", "main")
    {:target :browser} => ("browser", "module", "main")

    {:target <any>
     :package-json-resolution :browser} => ("browser", "main")

    {:target <any>
     :package-json-resolution :nodejs} => ("main")

This allows to import packages like graphql and iterall via :npm-deps,
which currently point their "module" entries to unsupported .mjs files.
  • Loading branch information
Jannis Pohlmann committed Mar 9, 2018
1 parent e714713 commit 43f2fac
Show file tree
Hide file tree
Showing 5 changed files with 132 additions and 8 deletions.
4 changes: 1 addition & 3 deletions src/main/cljs/cljs/module_deps.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@ let enhancedResolve = require('enhanced-resolve');

let target = 'CLJS_TARGET';
let filename = fs.realpathSync(path.resolve(__dirname, 'JS_FILE'));
let mainFields = target === 'nodejs'
? ['module', 'main']
: ['browser', 'module', 'main'];
let mainFields = MAIN_ENTRIES;
let aliasFields = target === 'nodejs' ? [] : ['browser'];

// https://github.com/egoist/konan
Expand Down
40 changes: 35 additions & 5 deletions src/main/clojure/cljs/closure.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1769,6 +1769,33 @@
(.getRequires input)))
(.toSource closure-compiler ast-root)))))

(defn- package-json-resolution->entries
"Returns a sequence of package.json entries to use for the given
:package-json-resolution mode (support: :webpack (default), :nodejs)."
[mode]
(case mode
:nodejs '("main")
:browser '("browser" "main")
'("browser" "main")))

(defn- package-json-entries
"Takes options and returns a sequence with the desired order of package.json
entries for the given :target and :package-json-resolution."
[opts]
(if-let [mode (:package-json-resolution opts)]
(package-json-resolution->entries mode)
(case (:target opts)
:browser '("browser" "module" "main")
:nodejs '("module" "main"))))

(comment
(= (package-json-entries {:target :nodejs}) '("module" "main"))
(= (package-json-entries {:target :browser}) '("browser" "module" "main"))
(= (package-json-entries {:target :nodejs :package-json-resolution :browser}) '("browser" "main"))
(= (package-json-entries {:target :nodejs :package-json-resolution :nodejs}) '("main"))
(= (package-json-entries {:target :browser :package-json-resolution :webpack}) '("browser" "main"))
(= (package-json-entries {:target :browser :package-json-resolution :nodejs}) '("main")))

(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
Expand All @@ -1781,9 +1808,8 @@
(.setLanguageIn (lang-key->lang-mode :ecmascript6))
(.setLanguageOut (lang-key->lang-mode (:language-out opts :ecmascript3)))
(.setDependencyOptions (doto (DependencyOptions.)
(.setDependencySorting true))))
_ (when (= (:target opts) :nodejs)
(.setPackageJsonEntryNames options ^List '("module", "main")))
(.setDependencySorting true)))
(.setPackageJsonEntryNames ^List (package-json-entries opts)))
closure-compiler (doto (make-closure-compiler)
(.init externs source-files options))
_ (.parse closure-compiler)
Expand Down Expand Up @@ -2322,9 +2348,13 @@
(when env/*compiler*
(:options @env/*compiler*))))
([{:keys [file]} {:keys [target] :as opts}]
(let [code (-> (slurp (io/resource "cljs/module_deps.js"))
(let [main-entries (str "[" (->> (package-json-entries opts)
(map #(str "\"" % "\""))
(string/join ",")) "]")
code (-> (slurp (io/resource "cljs/module_deps.js"))
(string/replace "JS_FILE" (string/replace file "\\" "\\\\"))
(string/replace "CLJS_TARGET" (str "" (when target (name target)))))
(string/replace "CLJS_TARGET" (str "" (when target (name target))))
(string/replace "MAIN_ENTRIES" main-entries))
proc (-> (ProcessBuilder. ["node" "--eval" code])
.start)
is (.getInputStream proc)
Expand Down
8 changes: 8 additions & 0 deletions src/test/cljs_build/package_json_resolution_test/core.cljs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
(ns package-json-resolution-test.core
(:require [iterall]
[graphql]))

(enable-console-print!)

(println "Is collection:" (iterall/isCollection #js [1 2]))
(println "GraphQL:" graphql)
53 changes: 53 additions & 0 deletions src/test/clojure/cljs/build_api_tests.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -573,3 +573,56 @@
(let [content (slurp (-> opts :modules :c :output-to))]
(testing "requires code.split.c"
(is (test/document-write? content 'code.split.c)))))))

(deftest test-cljs-2592
(test/delete-node-modules)
(spit (io/file "package.json") "{}")
(let [cenv (env/default-compiler-env)
dir (io/file "src" "test" "cljs_build" "package_json_resolution_test")
out (io/file (test/tmp-dir) "package_json_resolution_test")
opts {:main 'package-json-resolution-test.core
:output-dir (str out)
:output-to (str (io/file out "main.js"))
:optimizations :none
:install-deps true
:npm-deps {:iterall "1.2.2"
:graphql "0.13.1"}
:package-json-resolution :nodejs
:closure-warnings {:check-types :off
:non-standard-jsdoc :off}}]
(test/delete-out-files out)
(build/build (build/inputs dir) opts cenv)
(testing "processes the iterall index.js"
(let [index-js (io/file out "node_modules/iterall/index.js")]
(is (.exists index-js))
(is (contains? (:js-module-index @cenv) "iterall"))
(is (re-find #"goog\.provide\(\"module\$.*\$node_modules\$iterall\$index\"\)" (slurp index-js)))))
(testing "processes the graphql index.js"
(let [index-js (io/file out "node_modules/graphql/index.js")
execution-index-js (io/file out "node_modules/graphql/execution/index.js")
ast-from-value-js (io/file out "node_modules/grapqhl/utilities/astFromValue.js")]
(is (.exists index-js))
(is (contains? (:js-module-index @cenv) "graphql"))
(is (re-find #"goog\.provide\(\"module\$.*\$node_modules\$graphql\$index\"\)" (slurp index-js)))))
(testing "processes a nested index.js in graphql"
(let [nested-index-js (io/file out "node_modules/graphql/execution/index.js")]
(is (.exists nested-index-js))
(is (contains? (:js-module-index @cenv) "graphql/execution"))
(is (re-find #"goog\.provide\(\"module\$.*\$node_modules\$graphql\$execution\$index\"\)" (slurp nested-index-js)))))
(testing "processes cross-package imports"
(let [ast-from-value-js (io/file out "node_modules/graphql/utilities/astFromValue.js")]
(is (.exists ast-from-value-js))
(is (re-find #"goog.require\(\"module\$.*\$node_modules\$iterall\$index\"\);" (slurp ast-from-value-js)))))
(testing "adds dependencies to cljs_deps.js"
(let [deps-js (io/file out "cljs_deps.js")]
(is (re-find #"goog\.addDependency\(\"..\/node_modules\/iterall\/index.js\"" (slurp deps-js)))
(is (re-find #"goog\.addDependency\(\"..\/node_modules\/graphql\/index.js\"" (slurp deps-js)))
(is (re-find #"goog\.addDependency\(\"..\/node_modules\/graphql\/execution/index.js\"" (slurp deps-js)))))
(testing "adds the right module names to the core.cljs build output"
(let [core-js (io/file out "package_json_resolution_test/core.js")]
(is (re-find #"goog\.require\('module\$.*\$node_modules\$iterall\$index'\);" (slurp core-js)))
(is (re-find #"module\$.+\$node_modules\$iterall\$index\[\"default\"\]\.isCollection" (slurp core-js)))
(is (re-find #"goog\.require\('module\$.*\$node_modules\$graphql\$index'\);" (slurp core-js)))
(is (re-find #"module\$.+\$node_modules\$graphql\$index\[\"default\"\]" (slurp core-js))))))
(.delete (io/file "package.json"))
(test/delete-node-modules))
35 changes: 35 additions & 0 deletions src/test/clojure/cljs/closure_tests.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -442,3 +442,38 @@
(.delete (io/file "package.json"))
(test/delete-node-modules)
(test/delete-out-files out)))

(deftest test-cljs-2592
(spit (io/file "package.json") "{}")
(let [opts {:npm-deps {:iterall "1.2.2"
:graphql "0.13.1"}
:package-json-resolution :nodejs}
out (util/output-directory opts)]
(test/delete-node-modules)
(test/delete-out-files out)
(closure/maybe-install-node-deps! opts)
(let [modules (closure/index-node-modules ["iterall" "graphql"] opts)]
(is (true? (some (fn [module]
(= module {:module-type :es6
:file (.getAbsolutePath (io/file "node_modules/iterall/index.js"))
:provides ["iterall"
"iterall/index.js"
"iterall/index"]}))
modules)))
(is (true? (some (fn [module]
(= module {:module-type :es6
:file (.getAbsolutePath (io/file "node_modules/graphql/index.js"))
:provides ["graphql"
"graphql/index.js"
"graphql/index"]}))
modules)))
(is (true? (some (fn [module]
(= module {:module-type :es6
:file (.getAbsolutePath (io/file "node_modules/graphql/execution/index.js"))
:provides ["graphql/execution/index.js"
"graphql/execution/index"
"graphql/execution"]}))
modules))))
(.delete (io/file "package.json"))
(test/delete-node-modules)
(test/delete-out-files out)))

0 comments on commit 43f2fac

Please sign in to comment.