Skip to content

Commit

Permalink
Use the modern JSX transform (closes #123) (#129)
Browse files Browse the repository at this point in the history 
* Use the modern JSX transform

With the new transform, the $ macro is technically no longer dependent
on React. This possibly opens a path to Preact support in Helix.

More importantly, we obtain marginally faster performance. React is
able to make optimizations that it otherwise can't with
React.createElement().

* Update src/helix/core.cljs

* Update src/helix/core.cljs

* Update src/helix/dom.cljc

* Update src/helix/core.clj

---------

Co-authored-by: Will Acton <[email protected]>
  • Loading branch information
@rome-user @lilactown
rome-user and lilactown authored May 23, 2023
1 parent 9c38a29 commit 34811a9
Show file tree
Hide file tree
Showing 7 changed files with 168 additions and 48 deletions.
10 changes: 5 additions & 5 deletions dev/workshop/core.cljs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -358,11 +358,11 @@
(simple-benchmark
[]
(rds/renderToString
(r/createElement
react-children-benchmark
#js {:foo "bar"}
(r/createElement "div" #js {:style #js {:backgroundColor "green"}} "foo")
(r/createElement "div" nil "bar")))
(helix/jsxs react-children-benchmark
#js {:foo "bar"
:children #js [(helix/jsx "div" #js {:style #js {:backgroundColor "green"}
:children "foo"})
(helix/jsx "div" #js {:children "bar"})]}))
iterations)))

helix-time (hooks/use-memo
Expand Down
42 changes: 29 additions & 13 deletions src/helix/core.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,14 @@
(:require
[helix.impl.analyzer :as hana]
[helix.impl.props :as impl.props]
[cljs.tagged-literals :as tl]
[clojure.string :as string]))

(defn- jsx-children [coll]
(let [s (seq coll)]
(if (and s (next s))
(tl/->JSValue coll)
(first coll))))

(defmacro $
"Create a new React element from a valid React type.
Expand Down Expand Up @@ -44,19 +50,29 @@
(:native (meta type)))
type (if (keyword? type)
(name type)
type)]
(cond
(map? (first args))
`^js/React.Element (.createElement
(get-react)
~type
~(if native?
`(impl.props/dom-props ~(first args))
`(impl.props/props ~(first args)))
~@(rest args))

:else `^js/React.Element (.createElement (get-react) ~type nil ~@args))))

type)
has-props? (or (map? (first args))
(nil? (first args)))
children (if has-props?
(rest args)
args)
props (if (map? (first args))
(if native?
`(impl.props/dom-props ~(first args) ~(jsx-children children))
`(impl.props/props ~(first args) ~(jsx-children children)))
(tl/->JSValue (cond-> {}
(not-empty children)
(assoc :children (jsx-children children)))))
has-key? (when has-props?
(contains? (first args) :key))
the-key (when has-key?
(:key (first args)))
emit-fn (if (next children)
`jsxs
`jsx)]
(if has-key?
`^js/React.Element (~emit-fn ~type ~props ~the-key)
`^js/React.Element (~emit-fn ~type ~props))))

(defmacro <>
"Creates a new React Fragment Element"
Expand Down
37 changes: 25 additions & 12 deletions src/helix/core.cljs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
[helix.impl.props :as impl.props]
[helix.impl.classes :as helix.class]
[cljs-bean.core :as bean]
["react" :as react])
["react" :as react]
["react/jsx-runtime" :as jsx-runtime])
(:require-macros [helix.core]))


Expand Down Expand Up @@ -37,6 +38,8 @@
;; a dynamic arity dispatch. See https://github.com/Lokeh/helix/issues/20
(defn ^js/React get-react [] react)

(def jsx jsx-runtime/jsx)
(def jsxs jsx-runtime/jsxs)

(defn $
"Create a new React element from a valid React type.
Expand All @@ -55,20 +58,30 @@
native? (or (keyword? type)
(string? type)
(:native (meta type)))
has-props? ^boolean (or (map? ?p)
(nil? ?p))
children* ^seq (if has-props?
?c
args)
children (if (next children*)
(into-array children*)
(first children*))
props* (cond-> {}
has-props? (conj ?p)
(some? children) (assoc :children children))
props (if native?
(impl.props/-dom-props props*)
(impl.props/-props props*))
key (:key props*)
emit-fn (if (next children*)
jsxs
jsx)
type' (if (keyword? type)
(name type)
type)]
(if (map? ?p)
(apply create-element
type'
(if native?
(impl.props/-dom-props ?p)
(impl.props/-props ?p))
?c)
(apply create-element
type'
nil
args))))
(if (some? key)
(emit-fn type' props key)
(emit-fn type' props))))


(def ^:deprecated $$
Expand Down
30 changes: 18 additions & 12 deletions src/helix/dom.cljc
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
(ns helix.dom
(:refer-clojure :exclude [map meta time])
(:require
[cljs.tagged-literals :as tl]
[helix.core :as hx]
[helix.impl.props :as impl.props])
#?(:cljs (:require-macros [helix.dom])))
Expand Down Expand Up @@ -161,18 +162,23 @@
Use the special & or :& prop to merge dynamic props in."
[type & args]
(if (map? (first args))
`^js/React.Element (.createElement
(hx/get-react)
~type
(impl.props/dom-props ~(first args))
~@(rest args))
`^js/React.Element (.createElement
(hx/get-react)
~type
nil
~@args)))

(let [?p (first args)
has-props? (map? ?p)
children* (if has-props?
(rest args)
args)
multiple-children (next children*)
children (if multiple-children
(tl/->JSValue children*)
(first children*))
props* (when has-props? ?p)
key (:key props*)
emit-fn (if multiple-children
`hx/jsxs
`hx/jsx)]
(if (some? key)
`^js/React.Element (~emit-fn ~type (impl.props/dom-props ~props* ~children) ~key)
`^js/React.Element (~emit-fn ~type (impl.props/dom-props ~props* ~children)))))

#?(:clj (defn gen-tag
[tag]
Expand Down
14 changes: 10 additions & 4 deletions src/helix/impl/props.cljc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -172,8 +172,11 @@
(-dom-props {:style ["fs1"]})
)

(defmacro dom-props [m]
(-dom-props m))
(defmacro dom-props
([m] `(dom-props ~m nil))
([m c] (-dom-props (cond-> m
c (assoc :children c)
true (dissoc :key)))))


(defn -props
Expand Down Expand Up @@ -205,5 +208,8 @@
(-props {:foo-bar "baz"})
)

(defmacro props [m]
(-props m))
(defmacro props
([m] `(props ~m nil))
([m c] (-props (cond-> m
(some? c) (assoc :children c)
true (dissoc :key)))))
57 changes: 57 additions & 0 deletions test/helix/core_test.cljs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,3 +90,60 @@
(t/testing "can be used with IReset"
(reset! ref "well done")
(t/is (= "well done" @ref))))))

(t/deftest jsx-test
(t/testing "jsx transform"
;; In this test, you will see comments of the equivalent JSX. You can
;; compare the output of the JSX transform by copy-pasting the provided JSX
;; and running it through @babel/plugin-transform-react-jsx
(t/testing "with no props or children"
(let [component-1 (macroexpand '($ :div)) ; <div></div>
component-2 (macroexpand '($ "div"))
component-3 (macroexpand '($ "div" nil))
component-4 (macroexpand '($ "div" nil nil)) ; <div>{null}</div>
expected-1 '(helix.core/jsx "div" {})
expected-2 '(helix.core/jsx "div" {"children" nil})]
(t/are [x y] (= x (js->clj y))
expected-1 component-1
expected-1 component-2
expected-1 component-3
;; This ensures we are matching the behavior of react/createElement,
;; which is also reflected in the modern JSX transform.
expected-2 component-4)))
(t/testing "with no props and a single child"
(let [component-1 (macroexpand '($ :div "Hello")) ; <div>Hello</div>
component-2 (macroexpand '($ "div" nil "Hello"))
component-3 (macroexpand '($ "div" "Hello"))
expected '(helix.core/jsx "div" {"children" "Hello"})]
(t/are [x] (= expected (js->clj x))
component-1
component-2
component-3)))
(t/testing "with props and children"
(let [component (macroexpand
;; <Stack direction={row}>
;; <Item />
;; <Item />
;; </Stack>
'($ Stack {:direction "row"}
($ Item)
($ Item)))
;; Ideally, we'd check the full macroexpansion, but this suffices.
expected '(helix.core/jsxs Stack
(helix.impl.props/props {:direction "row"}
[($ Item)
($ Item)]))]
(t/is (= expected (js->clj component)))))
(t/testing "provides keys as expected"
(let [props {:key "key"}
component (macroexpand
;; <div key={kee} {...props}></div>
'($ :div {:key "kee" :& props}))
expected '(helix.core/jsx "div"
(helix.impl.props/dom-props {:key "kee"
:& props}
nil)
"kee")
element ($ :div {:key "kee" :& props})]
(t/is (= expected (js->clj component)))
(t/is (= "key" (gobj/get element "key")))))))
26 changes: 24 additions & 2 deletions test/helix/impl/props_test.cljc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,18 @@
#js {:foo "bar"}))

(t/is (eq (impl/dom-props {:style ["bar"]})
#js {:style #js ["bar"]}))))
#js {:style #js ["bar"]}))

(t/testing "children are added to props"
(t/is (eq (impl/dom-props nil "are the best")
#js {:children "are the best"}))
(t/is (eq (impl/dom-props {} "are the best")
#js {:children "are the best"}))
;; Should we convert to JS???
(t/is (eq (impl/dom-props nil ["one" "two"])
#js {:children ["one" "two"]}))
(t/is (eq (impl/dom-props {} ["one" "two"])
#js {:children ["one" "two"]})))))


#?(:cljs
Expand Down Expand Up @@ -183,7 +194,18 @@

(t/is (eq (impl/props {:foo "bar"
& nil})
#js {:foo "bar"}))))
#js {:foo "bar"}))

(t/testing "children are added to props"
(t/is (eq (impl/props nil "are the best")
#js {:children "are the best"}))
(t/is (eq (impl/props {} "are the best")
#js {:children "are the best"}))
;; Should we convert to JS???
(t/is (eq (impl/props nil ["one" "two"])
#js {:children ["one" "two"]}))
(t/is (eq (impl/props {} ["one" "two"])
#js {:children ["one" "two"]})))))


(t/deftest test-normalize-class
Expand Down

0 comments on commit 34811a9

Please sign in to comment.