Skip to content

Commit

Permalink
show a compile-time warning when required under non-dev builds (#37)
Browse files Browse the repository at this point in the history
The solution with devtools.optional namespace didn't work in scenario
when cljs-devtools was required as a library via maven dependency.
In that case we cannot (goog/require "devtools.optional") because the \
file is not compiled/present.

There does not seem to be a way how to conditionally require something
based on compiler options. So I decided just emit a compile-time warning
to warn users and leave it as is.

People should not use cljs-devtools in :advanced mode. And ideally they
should include it via :preloads which has no effect in :advanced mode.
  • Loading branch information
darwin committed Apr 13, 2017
1 parent a6fccd5 commit 2c32e41
Show file tree
Hide file tree
Showing 7 changed files with 64 additions and 54 deletions.
2 changes: 1 addition & 1 deletion project.clj
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@
["test-tests"]
["test-tests-with-config"]
["test-dead-code"]
["test-dce-size"]
;["test-dce-size"]
["test-advanced-warning"]]
"test-dead-code" ["do"
["with-profile" "+testing" "cljsbuild" "once" "dead-code"]
Expand Down
39 changes: 39 additions & 0 deletions src/lib/devtools/compiler.clj
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
(ns devtools.compiler
(:require [cljs.env]
[devtools.prefs :refer [get-pref]]))

(def preloads-url "https://github.com/binaryage/cljs-devtools/blob/master/docs/installation.md#install-it-via-preloads")
(def issue-37-url "https://github.com/binaryage/cljs-devtools/issues/37")

(defn ^:dynamic make-optimizations-warning-msg [mode]
(str "WARNING: You required cljs-devtools library in a project which is currently compiled with :optimizations " mode ".\n"
" You should remove this library from non-dev builds completely because it impedes dead code elimination.\n"
" The best way is to use :preloads compiler option: " preloads-url ".\n"
" To silence this warning please set :silence-optimizations-warning config key to true.\n"
" More details: " issue-37-url "."))

(defn get-compiler-optimizations-mode []
(or (if cljs.env/*compiler*
(get-in @cljs.env/*compiler* [:options :optimizations]))
:none))

(defn compiler-in-dev-mode? []
(= (get-compiler-optimizations-mode) :none))

(defn emit-compiler-warning! [msg]
(binding [*out* *err*]
(println msg)))

(def emit-compiler-warning-once! (memoize emit-compiler-warning!))

(defn warn-if-optimizations! []
(if-not (compiler-in-dev-mode?)
(emit-compiler-warning-once! (make-optimizations-warning-msg (get-compiler-optimizations-mode)))))

(defn silence-optimizations-warnings? []
(true? (get-pref :silence-optimizations-warning)))

(defmacro check-compiler-options! []
(if-not (silence-optimizations-warnings?)
(warn-if-optimizations!))
nil)
14 changes: 6 additions & 8 deletions src/lib/devtools/hints.cljs
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
(ns devtools.hints
(:require-macros [devtools.util :refer [emit-if-compiler-in-dev-mode]])
(:require-macros [devtools.util :refer [check-compiler-options!]]
[devtools.compiler :refer [check-compiler-options!]])
(:require [devtools.prefs :refer [pref]]
[devtools.context :as context]
[goog.string] ; for cljs.stacktrace, see devtools.optional
[clojure.string])) ; for cljs.stacktrace, see devtools.optional
[cljs.stacktrace :as stacktrace]))

(emit-if-compiler-in-dev-mode (goog/require "devtools.optional"))
; cljs.stacktrace does not play well in :advanced mode optimizations, see https://github.com/binaryage/cljs-devtools/issues/37
(check-compiler-options!)

(defn ^:dynamic available? []
true)
Expand Down Expand Up @@ -97,10 +98,7 @@
:else nil))

(defn parse-stacktrace [native-stack-trace]
; note that we have to do dynamic lookup here, cljs.stacktrace is present only in dev mode
; or when explicitly required by the user of cljs-devtools
(if (exists? js/devtools.optional.parse-stacktrace)
(js/devtools.optional.parse-stacktrace {} native-stack-trace {:ua-product :chrome} {:asset-root ""})))
(stacktrace/parse-stacktrace {} native-stack-trace {:ua-product :chrome} {:asset-root ""}))

(defn error-object-sense [error]
(try
Expand Down
25 changes: 0 additions & 25 deletions src/lib/devtools/optional.cljs

This file was deleted.

10 changes: 10 additions & 0 deletions src/lib/devtools/prefs.clj
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,13 @@

(defmacro emit-env-config []
`'~(or (memoized-read-env-config) {}))

; -- macro config api -------------------------------------------------------------------------------------------------------

(defn read-config []
(merge (memoized-read-env-config) (read-external-config)))

(def memoized-read-config (memoize read-config))

(defn get-pref [key]
(key (memoized-read-config)))
9 changes: 0 additions & 9 deletions src/lib/devtools/util.clj
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,3 @@
(~f ~@args)
(catch :default e#
~exceptional-result)))

(defn compiler-in-dev-mode? []
(if cljs.env/*compiler*
(let [mode (get-in @cljs.env/*compiler* [:options :optimizations])]
(or (= mode :none) (nil? mode))))) ; I'm not sure if mode must be always specified, defaults to :none if not

(defmacro emit-if-compiler-in-dev-mode [body]
(if (compiler-in-dev-mode?)
body))
19 changes: 8 additions & 11 deletions src/lib/devtools/util.cljs
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
(ns devtools.util
(:require-macros [devtools.util :refer [oget ocall oset emit-if-compiler-in-dev-mode]])
(:require-macros [devtools.util :refer [oget ocall oset]]
[devtools.compiler :refer [check-compiler-options!]])
(:require [goog.userAgent :as ua]
[clojure.data :as data]
[devtools.version :refer [get-current-version]]
[devtools.context :as context]
[goog.string] ; for cljs.pprint, see devtools.optional
[clojure.string] ; for cljs.pprint, see devtools.optional
[cljs.pprint :as cljs-pprint]
[devtools.prefs :as prefs]))

(emit-if-compiler-in-dev-mode (goog/require "devtools.optional"))
; cljs.pprint does not play well in advanced mode :optimizations, see https://github.com/binaryage/cljs-devtools/issues/37
(check-compiler-options!)

(def lib-info-style "color:black;font-weight:bold;")
(def reset-style "color:black")
Expand All @@ -22,13 +23,9 @@
; -- general helpers --------------------------------------------------------------------------------------------------------

(defn pprint-str [& args]
; note that we have to do dynamic lookup here, cljs.pprint is present only in dev mode
; or when explicitly required by the user of cljs-devtools
(if (exists? js/devtools.optional.pprint)
(with-out-str
(binding [*print-level* 300]
(apply js/devtools.optional.pprint args)))
(apply pr-str args)))
(with-out-str
(binding [*print-level* 300]
(apply cljs-pprint/pprint args))))

; -- version helpers --------------------------------------------------------------------------------------------------------

Expand Down

0 comments on commit 2c32e41

Please sign in to comment.