Skip to content

Commit 82fda4a

Browse files
committed
CLJS-3144: NPM Modules should have all their vars marked to avoid .call invokes
add new var information to vars resolved from Node or global foreign modules - mark them as :foreign true. In the compiler if a f is :foreign, then do not emit the .call convention. By doing this we avoid issues when the library itself is a singleton instance as .call convention breaks `this`.
1 parent f021e99 commit 82fda4a

File tree

3 files changed

+32
-7
lines changed

3 files changed

+32
-7
lines changed

src/main/clojure/cljs/analyzer.cljc

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1071,21 +1071,22 @@
10711071

10721072
(defmethod resolve* :node
10731073
[env sym full-ns current-ns]
1074-
{:name (symbol (str current-ns) (str (munge-node-lib full-ns) "." (name sym)))
1074+
{:ns current-ns
1075+
:name (symbol (str current-ns) (str (munge-node-lib full-ns) "." (name sym)))
10751076
:op :js-var
1076-
:ns current-ns
1077-
:tag 'js})
1077+
:foreign true})
10781078

10791079
(defmethod resolve* :global
10801080
[env sym full-ns current-ns]
10811081
(let [pre (into '[Object] (->> (string/split (name sym) #"\.") (map symbol) vec))]
10821082
(when-not (has-extern? pre)
10831083
(swap! env/*compiler* update-in
10841084
(into [::namespaces current-ns :externs] pre) merge {}))
1085-
{:name (symbol (str current-ns) (str (munge-global-export full-ns) "." (name sym)))
1085+
{:ns current-ns
1086+
:name (symbol (str current-ns) (str (munge-global-export full-ns) "." (name sym)))
10861087
:op :js-var
1087-
:ns current-ns
1088-
:tag (with-meta 'js {:prefix pre})}))
1088+
:tag (with-meta 'js {:prefix pre})
1089+
:foreign true}))
10891090

10901091
(def ^:private private-var-access-exceptions
10911092
"Specially-treated symbols for which we don't trigger :private-var-access warnings."

src/main/clojure/cljs/compiler.cljc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1142,7 +1142,7 @@
11421142
opt-count? (and (= (:name info) 'cljs.core/count)
11431143
(boolean ('#{string array} first-arg-tag)))
11441144
ns (:ns info)
1145-
js? (or (= ns 'js) (= ns 'Math) (= tag 'js))
1145+
js? (or (= ns 'js) (= ns 'Math) (:foreign info)) ;; foreign - i.e. global / Node.js library
11461146
goog? (when ns
11471147
(or (= ns 'goog)
11481148
(when-let [ns-str (str ns)]

src/test/clojure/cljs/build_api_tests.clj

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,30 @@
286286
(.delete (io/file "package.json"))
287287
(test/delete-node-modules))
288288

289+
(deftest test-npm-deps-invoke-cljs-3144
290+
(test/delete-node-modules)
291+
(spit (io/file "package.json") "{}")
292+
(let [cenv (env/default-compiler-env)
293+
out (.getPath (io/file "test-out" #_(test/tmp-dir) "npm-deps-test-out"))
294+
{:keys [inputs opts]} {:inputs (str (io/file "src" "test" "cljs_build"))
295+
:opts {:main 'npm-deps-test.invoke
296+
:output-dir out
297+
:optimizations :none
298+
:install-deps true
299+
:npm-deps {:react "15.6.1"
300+
:react-dom "15.6.1"
301+
:lodash-es "4.17.4"
302+
:lodash "4.17.4"}
303+
:closure-warnings {:check-types :off
304+
:non-standard-jsdoc :off}}}]
305+
(test/delete-out-files out)
306+
(testing "invoking fns from Node.js libraries should not emit .call convention"
307+
(build/build (build/inputs (io/file inputs "npm_deps_test/invoke.cljs")) opts cenv)
308+
(is (.exists (io/file out "node_modules/react/react.js")))
309+
(is (not (string/includes? (slurp (io/file out "npm_deps_test/invoke.cljs")) "call")))))
310+
(.delete (io/file "package.json"))
311+
(test/delete-node-modules))
312+
289313
(deftest test-preloads
290314
(let [out (.getPath (io/file (test/tmp-dir) "preloads-test-out"))
291315
{:keys [inputs opts]} {:inputs (str (io/file "src" "test" "cljs"))

0 commit comments

Comments
 (0)