Skip to content

Commit 233b423

Browse files
Jannis Pohlmannswannodette
authored andcommitted
CLJS-2592: :npm-deps using ES6 modules with .mjs extensions are not detected 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. As of this commit, the option accepts three values: :webpack (for ["browser" "module" "main"]) :nodejs (for ["main"]) a custom vector of entries (e.g. ["main" "module"]) If the :package-json-resolution option is not set, we fall back to the two default modes based on the build target: No target => :webpack Target :nodejs => :nodejs The resulting behavior: {:target :nodejs} => ["main"] {} => ["browser", "module", "main"] {:target <any> :package-json-resolution :webpack} => ["browser", "module", "main"] {:target <any> :package-json-resolution :nodejs} => ["main"] {:target <any> :package-json-resolution ["foo", "bar"]} => ["foo", "bar"] Using the new :nodejs mode, this allows to import packages like graphql and iterall via :npm-deps, which currently point their "module" entries to unsupported .mjs files.
1 parent 92cbedd commit 233b423

5 files changed

Lines changed: 134 additions & 8 deletions

File tree

src/main/cljs/cljs/module_deps.js

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,7 @@ let enhancedResolve = require('enhanced-resolve');
88

99
let target = 'CLJS_TARGET';
1010
let filename = fs.realpathSync(path.resolve(__dirname, 'JS_FILE'));
11-
let mainFields = target === 'nodejs'
12-
? ['module', 'main']
13-
: ['browser', 'module', 'main'];
11+
let mainFields = MAIN_ENTRIES;
1412
let aliasFields = target === 'nodejs' ? [] : ['browser'];
1513

1614
// https://github.com/egoist/konan

src/main/clojure/cljs/closure.clj

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1769,6 +1769,35 @@
17691769
(.getRequires input)))
17701770
(.toSource closure-compiler ast-root)))))
17711771

1772+
(defn- package-json-entries
1773+
"Takes options and returns a sequence with the desired order of package.json
1774+
entries for the given :package-json-resolution mode. If no mode is provided,
1775+
defaults to :webpack (if no target is set) and :nodejs (if the target is
1776+
:nodejs)."
1777+
[opts]
1778+
{:pre [(or (= (:package-json-resolution opts) :webpack)
1779+
(= (:package-json-resolution opts) :nodejs)
1780+
(and (sequential? (:package-json-resolution opts))
1781+
(every? string? (:package-json-resolution opts)))
1782+
(not (contains? opts :package-json-resolution)))]}
1783+
(let [modes {:nodejs ["main"]
1784+
:webpack ["browser" "module" "main"]}]
1785+
(if-let [mode (:package-json-resolution opts)]
1786+
(if (sequential? mode) mode (get modes mode))
1787+
(case (:target opts)
1788+
:nodejs (:nodejs modes)
1789+
(:webpack modes)))))
1790+
1791+
(comment
1792+
(= (package-json-entries {}) ["browser" "module" "main"])
1793+
(= (package-json-entries {:package-json-resolution :nodejs}) ["main"])
1794+
(= (package-json-entries {:package-json-resolution :webpack}) ["browser" "module" "main"])
1795+
(= (package-json-entries {:package-json-resolution ["foo" "bar" "baz"]}) ["foo" "bar" "baz"])
1796+
(= (package-json-entries {:target :nodejs}) ["main"])
1797+
(= (package-json-entries {:target :nodejs :package-json-resolution :nodejs}) ["main"])
1798+
(= (package-json-entries {:target :nodejs :package-json-resolution :webpack}) ["browser" "module" "main"])
1799+
(= (package-json-entries {:target :nodejs :package-json-resolution ["foo" "bar"]}) ["foo" "bar"]))
1800+
17721801
(defn convert-js-modules
17731802
"Takes a list JavaScript modules as an IJavaScript and rewrites them into a Google
17741803
Closure-compatible form. Returns list IJavaScript with the converted module
@@ -1781,9 +1810,8 @@
17811810
(.setLanguageIn (lang-key->lang-mode :ecmascript6))
17821811
(.setLanguageOut (lang-key->lang-mode (:language-out opts :ecmascript3)))
17831812
(.setDependencyOptions (doto (DependencyOptions.)
1784-
(.setDependencySorting true))))
1785-
_ (when (= (:target opts) :nodejs)
1786-
(.setPackageJsonEntryNames options ^List '("module", "main")))
1813+
(.setDependencySorting true)))
1814+
(.setPackageJsonEntryNames ^List (package-json-entries opts)))
17871815
closure-compiler (doto (make-closure-compiler)
17881816
(.init externs source-files options))
17891817
_ (.parse closure-compiler)
@@ -2322,9 +2350,13 @@
23222350
(when env/*compiler*
23232351
(:options @env/*compiler*))))
23242352
([{:keys [file]} {:keys [target] :as opts}]
2325-
(let [code (-> (slurp (io/resource "cljs/module_deps.js"))
2353+
(let [main-entries (str "[" (->> (package-json-entries opts)
2354+
(map #(str "\"" % "\""))
2355+
(string/join ",")) "]")
2356+
code (-> (slurp (io/resource "cljs/module_deps.js"))
23262357
(string/replace "JS_FILE" (string/replace file "\\" "\\\\"))
2327-
(string/replace "CLJS_TARGET" (str "" (when target (name target)))))
2358+
(string/replace "CLJS_TARGET" (str "" (when target (name target))))
2359+
(string/replace "MAIN_ENTRIES" main-entries))
23282360
proc (-> (ProcessBuilder. ["node" "--eval" code])
23292361
.start)
23302362
is (.getInputStream proc)
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
(ns package-json-resolution-test.core
2+
(:require [iterall]
3+
[graphql]))
4+
5+
(enable-console-print!)
6+
7+
(println "Is collection:" (iterall/isCollection #js [1 2]))
8+
(println "GraphQL:" graphql)

src/test/clojure/cljs/build_api_tests.clj

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -573,3 +573,56 @@
573573
(let [content (slurp (-> opts :modules :c :output-to))]
574574
(testing "requires code.split.c"
575575
(is (test/document-write? content 'code.split.c)))))))
576+
577+
(deftest test-cljs-2592
578+
(test/delete-node-modules)
579+
(spit (io/file "package.json") "{}")
580+
(let [cenv (env/default-compiler-env)
581+
dir (io/file "src" "test" "cljs_build" "package_json_resolution_test")
582+
out (io/file (test/tmp-dir) "package_json_resolution_test")
583+
opts {:main 'package-json-resolution-test.core
584+
:output-dir (str out)
585+
:output-to (str (io/file out "main.js"))
586+
:optimizations :none
587+
:install-deps true
588+
:npm-deps {:iterall "1.2.2"
589+
:graphql "0.13.1"}
590+
:package-json-resolution :nodejs
591+
:closure-warnings {:check-types :off
592+
:non-standard-jsdoc :off}}]
593+
(test/delete-out-files out)
594+
(build/build (build/inputs dir) opts cenv)
595+
(testing "processes the iterall index.js"
596+
(let [index-js (io/file out "node_modules/iterall/index.js")]
597+
(is (.exists index-js))
598+
(is (contains? (:js-module-index @cenv) "iterall"))
599+
(is (re-find #"goog\.provide\(\"module\$.*\$node_modules\$iterall\$index\"\)" (slurp index-js)))))
600+
(testing "processes the graphql index.js"
601+
(let [index-js (io/file out "node_modules/graphql/index.js")
602+
execution-index-js (io/file out "node_modules/graphql/execution/index.js")
603+
ast-from-value-js (io/file out "node_modules/grapqhl/utilities/astFromValue.js")]
604+
(is (.exists index-js))
605+
(is (contains? (:js-module-index @cenv) "graphql"))
606+
(is (re-find #"goog\.provide\(\"module\$.*\$node_modules\$graphql\$index\"\)" (slurp index-js)))))
607+
(testing "processes a nested index.js in graphql"
608+
(let [nested-index-js (io/file out "node_modules/graphql/execution/index.js")]
609+
(is (.exists nested-index-js))
610+
(is (contains? (:js-module-index @cenv) "graphql/execution"))
611+
(is (re-find #"goog\.provide\(\"module\$.*\$node_modules\$graphql\$execution\$index\"\)" (slurp nested-index-js)))))
612+
(testing "processes cross-package imports"
613+
(let [ast-from-value-js (io/file out "node_modules/graphql/utilities/astFromValue.js")]
614+
(is (.exists ast-from-value-js))
615+
(is (re-find #"goog.require\(\"module\$.*\$node_modules\$iterall\$index\"\);" (slurp ast-from-value-js)))))
616+
(testing "adds dependencies to cljs_deps.js"
617+
(let [deps-js (io/file out "cljs_deps.js")]
618+
(is (re-find #"goog\.addDependency\(\"..\/node_modules\/iterall\/index.js\"" (slurp deps-js)))
619+
(is (re-find #"goog\.addDependency\(\"..\/node_modules\/graphql\/index.js\"" (slurp deps-js)))
620+
(is (re-find #"goog\.addDependency\(\"..\/node_modules\/graphql\/execution/index.js\"" (slurp deps-js)))))
621+
(testing "adds the right module names to the core.cljs build output"
622+
(let [core-js (io/file out "package_json_resolution_test/core.js")]
623+
(is (re-find #"goog\.require\('module\$.*\$node_modules\$iterall\$index'\);" (slurp core-js)))
624+
(is (re-find #"module\$.+\$node_modules\$iterall\$index\[\"default\"\]\.isCollection" (slurp core-js)))
625+
(is (re-find #"goog\.require\('module\$.*\$node_modules\$graphql\$index'\);" (slurp core-js)))
626+
(is (re-find #"module\$.+\$node_modules\$graphql\$index\[\"default\"\]" (slurp core-js))))))
627+
(.delete (io/file "package.json"))
628+
(test/delete-node-modules))

src/test/clojure/cljs/closure_tests.clj

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -442,3 +442,38 @@
442442
(.delete (io/file "package.json"))
443443
(test/delete-node-modules)
444444
(test/delete-out-files out)))
445+
446+
(deftest test-cljs-2592
447+
(spit (io/file "package.json") "{}")
448+
(let [opts {:npm-deps {:iterall "1.2.2"
449+
:graphql "0.13.1"}
450+
:package-json-resolution :nodejs}
451+
out (util/output-directory opts)]
452+
(test/delete-node-modules)
453+
(test/delete-out-files out)
454+
(closure/maybe-install-node-deps! opts)
455+
(let [modules (closure/index-node-modules ["iterall" "graphql"] opts)]
456+
(is (true? (some (fn [module]
457+
(= module {:module-type :es6
458+
:file (.getAbsolutePath (io/file "node_modules/iterall/index.js"))
459+
:provides ["iterall"
460+
"iterall/index.js"
461+
"iterall/index"]}))
462+
modules)))
463+
(is (true? (some (fn [module]
464+
(= module {:module-type :es6
465+
:file (.getAbsolutePath (io/file "node_modules/graphql/index.js"))
466+
:provides ["graphql"
467+
"graphql/index.js"
468+
"graphql/index"]}))
469+
modules)))
470+
(is (true? (some (fn [module]
471+
(= module {:module-type :es6
472+
:file (.getAbsolutePath (io/file "node_modules/graphql/execution/index.js"))
473+
:provides ["graphql/execution/index.js"
474+
"graphql/execution/index"
475+
"graphql/execution"]}))
476+
modules))))
477+
(.delete (io/file "package.json"))
478+
(test/delete-node-modules)
479+
(test/delete-out-files out)))

0 commit comments

Comments
 (0)