From fe2f82d032afa93266012f51e3baca235af3f38e Mon Sep 17 00:00:00 2001 From: Juho Teperi Date: Fri, 27 Apr 2018 22:21:22 +0300 Subject: [PATCH 1/5] Switch back to :cljsbuild :builds vector, for figwheel Figwheel will default to first dev build. --- project.clj | 46 +++++++++++++++++++++++----------------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/project.clj b/project.clj index b15116e..3a09997 100644 --- a/project.clj +++ b/project.clj @@ -47,8 +47,8 @@ ;; In future :main alone should be enough to find entry file. :cljsbuild {:builds - {:client - {:source-paths ["demo"] + [{:id "client" + :source-paths ["demo"] :watch-paths ["src" "demo" "test"] :figwheel true :compiler {:parallel-build true @@ -59,8 +59,8 @@ :asset-path "js/out" :npm-deps false}} - :client-npm - {:source-paths ["demo"] + {:id "client-npm" + :source-paths ["demo"] :watch-paths ["src" "demo" "test"] :figwheel true :compiler {:parallel-build true @@ -70,8 +70,8 @@ :output-to "target/cljsbuild/client-npm/public/js/main.js" :asset-path "js/out"}} - :test - {:source-paths ["test"] + {:id "test" + :source-paths ["test"] :compiler {:parallel-build true :optimizations :none :main "reagenttest.runtests" @@ -81,8 +81,8 @@ :npm-deps false :aot-cache true}} - :test-npm - {:source-paths ["test"] + {:id "test-npm" + :source-paths ["test"] :compiler {:parallel-build true :optimizations :none :main "reagenttest.runtests" @@ -93,16 +93,16 @@ ;; Separate source-path as this namespace uses Node built-in modules which ;; aren't available for other targets, and would break other builds. - :prerender - {:source-paths ["prerender"] + {:id "prerender" + :source-paths ["prerender"] :compiler {:main "sitetools.prerender" :target :nodejs :output-dir "target/cljsbuild/prerender/out" :output-to "target/cljsbuild/prerender/main.js" :aot-cache true}} - :node-test - {:source-paths ["test/reagenttest/runtests.cljs"] + {:id "node-test" + :source-paths ["test/reagenttest/runtests.cljs"] :watch-paths ["src" "test"] :compiler {:main "reagenttest.runtests" :target :nodejs @@ -113,8 +113,8 @@ :npm-deps false :aot-cache true}} - :node-test-npm - {:source-paths ["test/reagenttest/runtests.cljs"] + {:id "node-test-npm" + :source-paths ["test/reagenttest/runtests.cljs"] :watch-paths ["src" "test"] :compiler {:main "reagenttest.runtests" :target :nodejs @@ -126,8 +126,8 @@ ;; With :advanched source-paths doesn't matter that much as ;; Cljs compiler will only read :main file. - :prod - {:source-paths ["demo"] + {:id "prod" + :source-paths ["demo"] :compiler {:main "reagentdemo.prod" :optimizations :advanced :elide-asserts true @@ -139,8 +139,8 @@ :npm-deps false :aot-cache true}} - :prod-npm - {:source-paths ["demo"] + {:id "prod-npm" + :source-paths ["demo"] :compiler {:main "reagentdemo.prod" :optimizations :advanced :elide-asserts true @@ -151,8 +151,8 @@ :closure-warnings {:global-this :off} :aot-cache true}} - :prod-test - {:source-paths ["test"] + {:id "prod-test" + :source-paths ["test"] :compiler {:main "reagenttest.runtests" :optimizations :advanced :elide-asserts true @@ -163,8 +163,8 @@ :npm-deps false :aot-cache true}} - :prod-test-npm - {:source-paths ["test"] + {:id "prod-test-npm" + :source-paths ["test"] :compiler {:main "reagenttest.runtests" :optimizations :advanced :elide-asserts true @@ -173,4 +173,4 @@ :output-to "target/cljsbuild/prod-test-npm/main.js" :output-dir "target/cljsbuild/prod-test-npm/out" :closure-warnings {:global-this :off} - :aot-cache true}}}}) + :aot-cache true}}]}) From 4a8ac5cd83bda15fffa0a9d7416a204c4dc6475e Mon Sep 17 00:00:00 2001 From: Juho Teperi Date: Fri, 27 Apr 2018 23:17:25 +0300 Subject: [PATCH 2/5] Fix problem with custom HTML element property name code Custom HTML element property name code accidentally modified cache object for normal HTML elements, which can cause Reagent to lose correct mappings for properties like className, htmlFor and charSet. --- src/reagent/impl/template.cljs | 2 +- test/reagent/impl/template_test.cljs | 20 ++++++++++++++++++++ test/reagenttest/runtests.cljs | 6 +++--- 3 files changed, 24 insertions(+), 4 deletions(-) create mode 100644 test/reagent/impl/template_test.cljs diff --git a/src/reagent/impl/template.cljs b/src/reagent/impl/template.cljs index 02e61aa..73cebef 100644 --- a/src/reagent/impl/template.cljs +++ b/src/reagent/impl/template.cljs @@ -82,7 +82,7 @@ (if (named? k) (if-some [k' (cache-get custom-prop-name-cache (name k))] k' - (aset prop-name-cache (name k) + (aset custom-prop-name-cache (name k) (util/dash-to-camel k))) k)) diff --git a/test/reagent/impl/template_test.cljs b/test/reagent/impl/template_test.cljs new file mode 100644 index 0000000..92d51f0 --- /dev/null +++ b/test/reagent/impl/template_test.cljs @@ -0,0 +1,20 @@ +(ns reagent.impl.template-test + (:require [clojure.test :as t :refer [deftest is testing]] + [reagent.impl.template :as tmpl] + [goog.object :as gobj])) + +(deftest cached-prop-name + (is (= "className" + (tmpl/cached-prop-name :class)))) + +(deftest cached-custom-prop-name + (is (= "class" + (tmpl/cached-custom-prop-name :class)))) + +(deftest convert-props-test + (is (gobj/equals #js {:className "a"} + (tmpl/convert-props {:class "a"} #js {:id nil :custom false}))) + (is (gobj/equals #js {:class "a"} + (tmpl/convert-props {:class "a"} #js {:id nil :custom true}))) + (is (gobj/equals #js {:className "a b" :id "a"} + (tmpl/convert-props {:class "b"} #js {:id "a" :class "a" :custom false})))) diff --git a/test/reagenttest/runtests.cljs b/test/reagenttest/runtests.cljs index 92e087a..b32d68a 100644 --- a/test/reagenttest/runtests.cljs +++ b/test/reagenttest/runtests.cljs @@ -7,6 +7,7 @@ [reagenttest.testtrack] [reagenttest.testwithlet] [reagenttest.testwrap] + [reagent.impl.template-test] [cljs.test :as test] [doo.runner :as doo :include-macros true] [reagent.core :as r] @@ -21,8 +22,7 @@ :color :#aaa}) (defn all-tests [] - #_(test/run-tests 'reagenttest.testratomasync) - (test/run-all-tests #"reagenttest.test.*")) + (test/run-all-tests #"(reagenttest\.test.*|reagent\..*-test)")) (defmethod test/report [::test/default :summary] [m] ;; ClojureScript 2814 doesn't return anything from run-tests @@ -56,4 +56,4 @@ (run-tests) [#'test-output-mini])) -(doo/doo-all-tests #"reagenttest.test.*") +(doo/doo-all-tests #"(reagenttest\.test.*|reagent\..*-test)") From 606b321d2fa31fb8b63efa57ad16b8b6d22a7bae Mon Sep 17 00:00:00 2001 From: Juho Teperi Date: Fri, 27 Apr 2018 23:19:59 +0300 Subject: [PATCH 3/5] Fix keywords and symbols in :class Fixes #367 --- src/reagent/impl/template.cljs | 4 +++- test/reagenttest/testreagent.cljs | 16 ++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/reagent/impl/template.cljs b/src/reagent/impl/template.cljs index 73cebef..214830c 100644 --- a/src/reagent/impl/template.cljs +++ b/src/reagent/impl/template.cljs @@ -122,12 +122,14 @@ ;; Merge classes class (assoc :class (let [old-class (:class props)] - (if (nil? old-class) class (str class " " old-class))))))) + (if (nil? old-class) class (str class " " (name old-class)))))))) (defn stringify-class [{:keys [class] :as props}] + ;; (keep name) doesn't work because class vector could contain false, which is not Named (if (coll? class) (->> class (filter identity) + (map name) (string/join " ") (assoc props :class)) props)) diff --git a/test/reagenttest/testreagent.cljs b/test/reagenttest/testreagent.cljs index 12d69c1..e1b8736 100644 --- a/test/reagenttest/testreagent.cljs +++ b/test/reagenttest/testreagent.cljs @@ -574,6 +574,22 @@ (is (= (rstr [:p {:class #{"a" "b" "c"}}]) (rstr [:p {:class "a b c"}])))) +(deftest class-named-values + (is (= (rstr [:p {:class :a}]) + (rstr [:p {:class "a"}]))) + (is (= (rstr [:p.a {:class :b}]) + (rstr [:p {:class "a b"}]))) + (is (= (rstr [:p.a {:class 'b}]) + (rstr [:p {:class "a b"}]))) + (is (= (rstr [:p {:class [:a :b]}]) + (rstr [:p {:class "a b"}]))) + (is (= (rstr [:p {:class ['a :b]}]) + (rstr [:p {:class "a b"}]))) + + (testing "class collection can contain false value" + (is (= (rstr [:p {:class [:a :b false nil]}]) + (rstr [:p {:class "a b"}])))) ) + (deftest test-force-update (let [v (atom {:v1 0 :v2 0}) From d5923356125a1081ca21254e3193b948b2c6d93e Mon Sep 17 00:00:00 2001 From: Juho Teperi Date: Fri, 27 Apr 2018 23:20:06 +0300 Subject: [PATCH 4/5] Update CHANGELOG.md --- CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 05a5340..3075b96 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,13 @@ # Changelog +## Unreleased + +**[compare](https://github.com/reagent-project/reagent/compare/v0.8.0...master)** + +- Fix problem which caused using e.g. `:class` property with custom HTML element to break normal elements +- Fix problem using keyword or symbol as `:class` together with element tag class shorthand, e.g. `[:p.a {:class :b}]` ([#367](https://github.com/reagent-project/reagent/issues/367)) +- Added support for using keywords and symbols in `:class` collection + ## 0.8.0 (2018-04-19) **[compare](https://github.com/reagent-project/reagent/compare/v0.8.0-rc1...v0.8.0)** From e1019a1c9c5104b500d0b42bf724eeaaa519c259 Mon Sep 17 00:00:00 2001 From: Juho Teperi Date: Fri, 4 May 2018 16:10:41 +0300 Subject: [PATCH 5/5] Check if value is named before calling name --- src/reagent/impl/template.cljs | 12 ++++++++---- test/reagenttest/testreagent.cljs | 29 +++++++++++++++++------------ 2 files changed, 25 insertions(+), 16 deletions(-) diff --git a/src/reagent/impl/template.cljs b/src/reagent/impl/template.cljs index 214830c..416f27d 100644 --- a/src/reagent/impl/template.cljs +++ b/src/reagent/impl/template.cljs @@ -122,14 +122,18 @@ ;; Merge classes class (assoc :class (let [old-class (:class props)] - (if (nil? old-class) class (str class " " (name old-class)))))))) + (if (nil? old-class) class (str class " " (if (named? old-class) + (name old-class) + old-class)))))))) (defn stringify-class [{:keys [class] :as props}] - ;; (keep name) doesn't work because class vector could contain false, which is not Named (if (coll? class) (->> class - (filter identity) - (map name) + (keep (fn [c] + (if c + (if (named? c) + (name c) + c)))) (string/join " ") (assoc props :class)) props)) diff --git a/test/reagenttest/testreagent.cljs b/test/reagenttest/testreagent.cljs index e1b8736..d965935 100644 --- a/test/reagenttest/testreagent.cljs +++ b/test/reagenttest/testreagent.cljs @@ -574,19 +574,24 @@ (is (= (rstr [:p {:class #{"a" "b" "c"}}]) (rstr [:p {:class "a b c"}])))) -(deftest class-named-values - (is (= (rstr [:p {:class :a}]) - (rstr [:p {:class "a"}]))) - (is (= (rstr [:p.a {:class :b}]) - (rstr [:p {:class "a b"}]))) - (is (= (rstr [:p.a {:class 'b}]) - (rstr [:p {:class "a b"}]))) - (is (= (rstr [:p {:class [:a :b]}]) - (rstr [:p {:class "a b"}]))) - (is (= (rstr [:p {:class ['a :b]}]) - (rstr [:p {:class "a b"}]))) +(deftest class-different-types + (testing "named values are supported" + (is (= (rstr [:p {:class :a}]) + (rstr [:p {:class "a"}]))) + (is (= (rstr [:p.a {:class :b}]) + (rstr [:p {:class "a b"}]))) + (is (= (rstr [:p.a {:class 'b}]) + (rstr [:p {:class "a b"}]))) + (is (= (rstr [:p {:class [:a :b]}]) + (rstr [:p {:class "a b"}]))) + (is (= (rstr [:p {:class ['a :b]}]) + (rstr [:p {:class "a b"}])))) - (testing "class collection can contain false value" + (testing "non-named values like numbers" + (is (= (rstr [:p {:class [1 :b]}]) + (rstr [:p {:class "1 b"}])))) + + (testing "falsey values are filtered from collections" (is (= (rstr [:p {:class [:a :b false nil]}]) (rstr [:p {:class "a b"}])))) )