add namespacing for script tag keys (#326)

* add namespacing for script tag keys
* improve deprecation warning tests

Author: yogthos

README.md

@@ -1043,31 +1043,52 @@ Note, the escaping of variables can also be controlled through the dynamic bindi
 
 #### script
 
-The script tag will generate an HTML script tag and prepend the value of the `selmer/context` key
-to the URI. When `selmer/context` key is not present then the original URI is set.
+The script tag will generate an HTML script tag and prepend the value of the `:selmer/context` key
+to the URI. When `:selmer/context` key is not present then the original URI is set.
 
 `(render "{% script \"/js/site.js\" %}" {:selmer/context "/myapp"})` =>
 ```
-"<script async-attr defer-attr src=\"/myapp/js/site.js\" type=\"text/javascript\"></script>"
+"<script src=\"/myapp/js/site.js\" type=\"application/javascript\"></script>"
 ```
 
-When `type` key is present its value is used for the 'type' attribute.
+The tag supports the following optional arguments:
+
+* `type` - sets the `type` attribute (default: `application/javascript`)
+* `async` - when truthy, adds the `async` attribute
+* `defer` - when truthy, adds the `defer` attribute
 
-`(render "{% script \"/js/site.js\" type=\"module\" %}" ` =>
+`(render "{% script \"/js/site.js\" type=\"module\" %}" {})` =>
 ```
 "<script src=\"/js/site.js\" type=\"module\"></script>"
 ```
 
+`(render "{% script \"/js/site.js\" async=\"true\" defer=\"true\" %}" {})` =>
+```
+"<script async defer src=\"/js/site.js\" type=\"application/javascript\"></script>"
+```
+
+These attributes can also be set via the context map using namespaced keys:
+`:selmer/type`, `:selmer/async`, and `:selmer/defer`.
+
+`(render "{% script \"/js/site.js\" %}" {:selmer/type "module" :selmer/async true})` =>
+```
+"<script async src=\"/js/site.js\" type=\"module\"></script>"
+```
+
+**Note (since 1.13.0):** The `:type` context key is no longer consumed by the script tag — use
+`:selmer/type` instead. The non-namespaced `:async` and `:defer` context keys are deprecated;
+use `:selmer/async` and `:selmer/defer` to avoid conflicts with your own context data.
+
 Since 1.11.1 URI can be a name of context parameter with optional filters.
 
 `(render "{% script path %}" {:selmer/context "/myapp" :path "/js/site.js"})` =>
 ```
-"<script async-attr defer-attr src=\"/myapp/js/site.js\" type=\"text/javascript\"></script>"
+"<script src=\"/myapp/js/site.js\" type=\"application/javascript\"></script>"
 ```
 
 `(render "{% script path|upper %}" {:selmer/context "/myapp" :path "/js/site.js"})` =>
 ```
-"<script async-attr defer-attr src=\"/myapp/JS/SITE.JS\" type=\"text/javascript\"></script>"
+"<script src=\"/myapp/JS/SITE.JS\" type=\"application/javascript\"></script>"
 ```
 #### style
 

changes.md

@@ -1,3 +1,5 @@
+* 1.13.0 - [namespace script tag context keys to avoid collisions](https://github.com/yogthos/Selmer/issues/325): `script` tag now uses `:selmer/type`, `:selmer/async`, and `:selmer/defer` context keys; non-namespaced `:async` and `:defer` are deprecated with warnings, `:type` requires `:selmer/type` - **breaking change**
+  
 * 1.12.70 - [add `get` filter](https://github.com/yogthos/Selmer/issue/322)
 * 1.12.69 - fix relfection warning
 * 1.12.68 - fix handling nulls when parsing script tags

project.clj

@@ -1,4 +1,4 @@
-(defproject selmer "1.12.70"
+(defproject selmer "1.13.0"
   :description "Django style templates for Clojure"
   :url "https://github.com/yogthos/Selmer"
   :license {:name "Eclipse Public License"

src/selmer/tags.clj

@@ -315,6 +315,15 @@
   (for [[id value] (partition 2 args)]
     [(map keyword (str/split id #"\.")) (compile-filter-body value false)]))
 
+(defn- compile-args-namespaced
+  "Like compile-args but namespaces keys under selmer/ to avoid conflicts with user context."
+  [args]
+  (when-not (even? (count args))
+    (throw (ex-info (str "invalid arguments passed to tag: " args)
+                    {:args args})))
+  (for [[id value] (partition 2 args)]
+    [(map #(keyword "selmer" %) (str/split id #"\.")) (compile-filter-body value false)]))
+
 (defn with-handler [args tag-content render rdr]
   (let [content (get-in (tag-content rdr :with :endwith) [:with :content])
         args    (->> args
@@ -355,27 +364,38 @@
   number of optional arguments. Value for 'src' attribute is built accounting
   value of `selmer/context` context parameter and `uri` can be a string literal
   or name of context parameter (filters also supported). Optional arguments are:
-  * `async` - when evaluates to logical true then 'async' attribute would be
-    added to generated tag.
-  * `defer` - when evaluates to logical true then 'defer' attribute would be
-    added to generated tag.
-  * `type` - when present its value is used for the 'type' attribute."
+  * `selmer/async` (or deprecated `async`) - when evaluates to logical true
+    then 'async' attribute would be added to generated tag.
+  * `selmer/defer` (or deprecated `defer`) - when evaluates to logical true
+    then 'defer' attribute would be added to generated tag.
+  * `selmer/type` - when present its value is used for the 'type' attribute.
+
+  Note: Using non-namespaced keys `:async` and `:defer` in the context map is
+  deprecated. Please use `:selmer/async` and `:selmer/defer` instead to avoid
+  conflicts with your own context data. The `:type` key is no longer supported
+  in the context map; use `:selmer/type` instead."
   [[^String uri & args] _ _ _]
   (let [args
         (->> args
              (mapcat #(.split ^String % "="))
              (remove #{"="})
-             (compile-args))]
+             (compile-args-namespaced))]
     (fn [context-map]
       (let [args
                          (reduce
                            (fn [context-map [k v]]
                              (assoc-in context-map k (v context-map)))
                            context-map
                            args)
-            async-attr   (when (:async args) "async ")
-            defer-attr   (when (:defer args) "defer ")
-            type-attr    (or (:type args) "application/javascript")
+            async-attr   (when (if (contains? args :selmer/async)
+                                 (:selmer/async args)
+                                 (deprecated-key-lookup args :selmer/async :async))
+                           "async ")
+            defer-attr   (when (if (contains? args :selmer/defer)
+                                 (:selmer/defer args)
+                                 (deprecated-key-lookup args :selmer/defer :defer))
+                           "defer ")
+            type-attr    (or (:selmer/type args) "application/javascript")
             src-attr-val (build-uri-for-script-or-style-tag uri context-map)]
         (str "<script " async-attr defer-attr "src=\"" src-attr-val "\" type=\"" type-attr "\"></script>")))))
 

src/selmer/util.clj

@@ -294,3 +294,41 @@ so it can access vectors as well as maps."
         algo (MessageDigest/getInstance algo)
         bs (.digest algo (.getBytes s))]
     (format "%032x" (BigInteger. 1 bs))))
+
+;; Deprecation warning infrastructure
+(def ^:dynamic *warn-on-deprecated-keys* true)
+
+(defonce ^:private warned-keys (atom #{}))
+
+(defn- default-deprecation-warning-handler
+  "Default handler that tries clojure.tools.logging first, falls back to stderr."
+  [message]
+  (try
+    (require 'clojure.tools.logging)
+    (require 'clojure.tools.logging.impl)
+    (let [get-logger (resolve 'clojure.tools.logging.impl/get-logger)
+          factory    @(resolve 'clojure.tools.logging/*logger-factory*)
+          logger     (get-logger factory "selmer.util")
+          log*       (resolve 'clojure.tools.logging/log*)]
+      (log* logger :warn nil message))
+    (catch Exception _
+      (binding [*out* *err*]
+        (println "DEPRECATION WARNING:" message)))))
+
+(def ^:dynamic *deprecation-warning-handler* default-deprecation-warning-handler)
+
+(defn deprecated-key-lookup
+  "Looks up a key in context-map, preferring the namespaced version (e.g. :selmer/async)
+   but falling back to the non-namespaced version (e.g. :async) with a deprecation warning.
+   The warning is only emitted once per key per JVM session."
+  [context-map namespaced-key non-namespaced-key]
+  (if (contains? context-map namespaced-key)
+    (get context-map namespaced-key)
+    (when (contains? context-map non-namespaced-key)
+      (when (and *warn-on-deprecated-keys*
+                 (not (contains? @warned-keys non-namespaced-key)))
+        (swap! warned-keys conj non-namespaced-key)
+        (*deprecation-warning-handler*
+          (str "Using " non-namespaced-key " in context is deprecated. "
+               "Please use " namespaced-key " instead.")))
+      (get context-map non-namespaced-key))))

test/selmer/core_test.clj

@@ -479,6 +479,84 @@
     (= "<script src=\"/js/site.js\" type=\"application/javascript\"></script>"
        (render "{% script \"/js/site.js\" %}" {}))))
 
+;; Tests for namespaced keys (issue #325)
+(deftest script-namespaced-async
+  (is
+    (= "<script async src=\"/js/site.js\" type=\"application/javascript\"></script>"
+       (render "{% script \"/js/site.js\" %}" {:selmer/async true})))
+  (is
+    (= "<script src=\"/js/site.js\" type=\"application/javascript\"></script>"
+       (render "{% script \"/js/site.js\" %}" {:selmer/async false}))))
+
+(deftest script-namespaced-defer
+  (is
+    (= "<script defer src=\"/js/site.js\" type=\"application/javascript\"></script>"
+       (render "{% script \"/js/site.js\" %}" {:selmer/defer true})))
+  (is
+    (= "<script src=\"/js/site.js\" type=\"application/javascript\"></script>"
+       (render "{% script \"/js/site.js\" %}" {:selmer/defer false}))))
+
+(deftest script-namespaced-type
+  (is
+    (= "<script src=\"/js/site.js\" type=\"module\"></script>"
+       (render "{% script \"/js/site.js\" %}" {:selmer/type "module"})))
+  ;; Verify user's own :type key doesn't conflict when using namespaced key
+  (is
+    (= "<script src=\"/js/site.js\" type=\"module\"></script>"
+       (render "{% script \"/js/site.js\" %}" {:selmer/type "module" :type :my-value}))))
+
+(deftest script-namespaced-priority
+  ;; Namespaced key takes priority over non-namespaced
+  (is
+    (= "<script src=\"/js/site.js\" type=\"module\"></script>"
+       (render "{% script \"/js/site.js\" %}" {:selmer/type "module" :type "text/javascript"}))))
+
+(deftest script-falsey-namespaced-key
+  ;; When :selmer/async is false, should NOT fall back to deprecated :async
+  (is
+    (= "<script src=\"/js/site.js\" type=\"application/javascript\"></script>"
+       (render "{% script \"/js/site.js\" %}" {:selmer/async false :async true})))
+  (is
+    (= "<script src=\"/js/site.js\" type=\"application/javascript\"></script>"
+       (render "{% script \"/js/site.js\" %}" {:selmer/defer false :defer true}))))
+
+(deftest script-deprecated-key-warning
+  ;; Reset warned-keys so warnings are emitted fresh
+  (reset! @#'selmer.util/warned-keys #{})
+  (let [warnings (atom [])]
+    (binding [*deprecation-warning-handler* #(swap! warnings conj %)]
+      (render "{% script \"/js/site.js\" %}" {:async true}))
+    (is (= 1 (count @warnings)))
+    (is (str/includes? (first @warnings) ":selmer/async")))
+  ;; Second call should NOT warn again (once per key per session)
+  (let [warnings (atom [])]
+    (binding [*deprecation-warning-handler* #(swap! warnings conj %)]
+      (render "{% script \"/js/site.js\" %}" {:async true}))
+    (is (= 0 (count @warnings))))
+  ;; Reset and test defer warning
+  (reset! @#'selmer.util/warned-keys #{})
+  (let [warnings (atom [])]
+    (binding [*deprecation-warning-handler* #(swap! warnings conj %)]
+      (render "{% script \"/js/site.js\" %}" {:defer true}))
+    (is (= 1 (count @warnings)))
+    (is (str/includes? (first @warnings) ":selmer/defer")))
+  ;; No warning when *warn-on-deprecated-keys* is false
+  (reset! @#'selmer.util/warned-keys #{})
+  (let [warnings (atom [])]
+    (binding [*deprecation-warning-handler* #(swap! warnings conj %)
+              *warn-on-deprecated-keys* false]
+      (render "{% script \"/js/site.js\" %}" {:async true}))
+    (is (= 0 (count @warnings)))))
+
+(deftest script-deprecated-key-warning-default-handler
+  ;; Verify the default handler writes to stderr (the println fallback path)
+  (reset! @#'selmer.util/warned-keys #{})
+  (let [sw (java.io.StringWriter.)]
+    (binding [*err* sw]
+      (render "{% script \"/js/site.js\" %}" {:async true}))
+    (is (str/includes? (str sw) "DEPRECATION WARNING"))
+    (is (str/includes? (str sw) ":selmer/async"))))
+
 (deftest cycle-test
   (is
     (= "\"foo\"1\"bar\"2\"baz\"1\"foo\"2\"bar\"1"