Use the modern JSX transform (closes #123) #129
Conversation
|
I'll review this week. Thank you so much! |
|
Still on my list. @rome-user could you resolve the conflicts on this branch? |
|
Sure. I'll be able to do that over the next few days |
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().
There was a problem hiding this comment.
A few very mechanical changes I suggest here, and one food for thought comment. If it's alright with you, I'll apply the suggestions and then merge this, and I'll go test it on our code before doing a release.
Thank you for this work! It's important to keep helix up to date with the latest practices.
| has-props? ^boolean (or (map? ?p) | ||
| (nil? ?p)) | ||
| children* ^seq (if has-props? | ||
| ?c | ||
| args) |
There was a problem hiding this comment.
what are the type hints here for? it's not clear to me if they're doing anything. also, in the past I've typically seen them attached to the binding in a let rather than the expr, e.g.
| has-props? ^boolean (or (map? ?p) | |
| (nil? ?p)) | |
| children* ^seq (if has-props? | |
| ?c | |
| args) | |
| ^boolean has-props? (or (map? ?p) | |
| (nil? ?p)) | |
| ^seq children* (if has-props? | |
| ?c | |
| args) |
There was a problem hiding this comment.
The type hints ensure efficient code is emitted when you test for nil or want to call next on them. Here is a demonstration.
cljs.user> (str (fn [s] (and s (next s))))
"function (s){\nvar and__5043__auto__ = s;\nif(cljs.core.truth_(and__5043__auto__)){\nreturn cljs.core.next.call(null,s);\n} else {\nreturn and__5043__auto__;\n}\n}"
cljs.user> (str (fn [^seq s] (and s (next s))))
"function (s){\nreturn ((s) && (cljs.core.next.call(null,s)));\n}"
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().Backwards compatibility
While the new JSX transform was introduced in React 17, it has been backported to React 16.14.0, React 15.7.0, and React 0.14.10. This PR should not introduce any surprises to current users of Helix, unless they use outdated versions of React 16, 15, or 0.14.
Pros
Cons
$and$dmacros are slightly more complicated. The logic of the$function is also more complicated. Unit tests are provided to ensure the output of$matches the modern JSX transform, but these tests may need maintenance.