squint-cljs / squint

Light-weight ClojureScript dialect
https://squint-cljs.github.io/squint
634 stars 38 forks source link

Using a variable as props for a jsx element doesn't work #250

Closed armincerf closed 1 year ago

armincerf commented 1 year ago
#jsx [:div props]

creates this

return <div>{props4}</div>;

rather than

return <div {...props4} />;
borkdude commented 1 year ago

Interesting!

How do we know that we want to splice props here? I mean, shouldn't we be more explicit here with [:div {:foo 1 :bar 2}]? props could also be a child element, I guess?

armincerf commented 1 year ago

well in this case the code looks like

(let [props {:foo 1 :bar 2}]
  [:div props])

the use case here is that there are several libraries that give you objects to splice into components. for example react-hook-form gives you a 'register' object that you splice into your input, containing a ref, onChange, onBlur etc. But atm instead of being able to write:

(let [{:keys [register]} (useForm)]
  [:input (merge register {:type "text"})])

I have to write

[:input {:ref ref 
         :onChange onChange
         :onBlur onBlur
         :name name
         :type "text"}]

etc

armincerf commented 1 year ago

I think if an object is at index 1 of the jsx vector it should be compiled into the {...props} format, though I'm not sure how the jsx reader macro works

borkdude commented 1 year ago

@armincerf Right... is this always the case in JSX? What if you don't want to pass props, but only children, do you always write <Foo null ....> in JSX?

armincerf commented 1 year ago

You can't use an object as children, so I guess it should error if you try. I think you need to look at the type and if the first element is an object, you assume props and spread it, else you assume its children and there aren't any props

borkdude commented 1 year ago

Note that this is a compiler, not a runtime interpreter. Have you tried writing ...props ?

borkdude commented 1 year ago

I guess we could enable that notation, so props gets resolved properly but then prepended with ... again. Or maybe an explicit (... props) notation or so where ... is a special form.

borkdude commented 1 year ago

Yeah, I think:

(let [props {}]
  #jsx [Foo (... props)])

rather than

(let [props {}]
  #jsx [Foo ...props])

is better, so we also don't need to change clj-kondo for this.

armincerf commented 1 year ago

(... props) seems more clojury, I'm good with that

borkdude commented 1 year ago

You can try this right now, as a workaround:

(js* "{...~{}}" props)
lilactown commented 1 year ago

FWIW, helix solves this by using & or :& as a key in a literal map in the first position.

(let [some-props {:foo "bar"}]
   ($ Foo {:baz 42 :& some-props}))

This allows you to pass in some props literally, and then merge those with some dynamic props.

Because it's just a key in a map, clj-kondo has no issue with it - initially I used a symbol &, but that was causing some lint issues so I also added the keyword.

It also means that you have the simple rule of "a map literal in the first position is always props, otherwise it's a child." It does catch people up when they first start using it, but because the rule is simple it's not a big barrier.

armincerf commented 1 year ago

Ohhh that's a neat idea @lilactown, that might actually be better than the js way (because having an odd number of items in a map is silly)

Couldn't get your suggestion to work though @borkdude

[:input (js* "{...~{}}" input-props)]

gives

<input>{{...input_props62}}</input>

unless I'm misunderstanding something

borkdude commented 1 year ago

@armincerf Well, that's almost what you want I guess? Can you remove the outer {}s in js*?

armincerf commented 1 year ago

It needs to be inside the input tag, like

<input {...input_props62}></input>
borkdude commented 1 year ago

because having an odd number of items in a map is silly

This doesn't even work with the Clojure reader, so we're kind of forced to choose a different solution like the Helix one.

What I meant was:

[:input (js* "...~{}" input-props)]
armincerf commented 1 year ago

Yeah that gets rid of the extra brackets but its still in-between the tags rather than treated as a prop

borkdude commented 1 year ago

Oh I see your problem, it needs to be inside the opening tag...

What about [:input {(js* "...") input-props}]?

borkdude commented 1 year ago

I see there is a bug in the JSX processing logic preventing this to work (always calling name on the key of a map)

borkdude commented 1 year ago

I guess we could allow the syntax:

(let [props {}]
  {:a 1 ... props :b 2})

clj-kondo currently doesn't complain about ... (this might be due to a bug ;-)) and still gives us an even amount of elements.

armincerf commented 1 year ago

I think keeping the :& from helix makes sense and means less to learn for people coming from that lib, also means clj-kondo shouldn't complain

(let [props {}]
  {:a 1 :& props :b 2})
borkdude commented 1 year ago

It could cause conflicts when you really want to use the keyword :& though, but maybe worth it.

borkdude commented 1 year ago

Although I can see it being useful to have the same symbol in other places, so it seems a more general solution:

(let x [1 2 3], y [0 ... x])
armincerf commented 1 year ago

I don't think reserving :& is a big deal, but I can also see the appeal to keeping the ... from the js world. So no real opinion either way I guess :D

lilactown commented 1 year ago

The helix syntax was meant to rhyme with rest args in fns, i.e. (fn [a b & args] ,,,). It's quite similar to the rest args syntax in JS, function (a, b, ...args) { }. I think it's worth keeping the rhyme going with the Clojure &, but I don't feel too strongly about it.

If we're going to go all the way, here's what I would propose, laying it all out there:

;; sequential "rest" args / destructuring today
(fn [a b & args] ,,,)
(let [[a b & rest] [1 2 3 4 5] ,,,)

;; associative destructuring `:as` today
(fn [{:keys [a b] :as more}] ,,,)
(let [{:keys [a b] :as more} x] ,,,)

;; "spread" sequential proposal
[0 1 & (range 2 10)]

;; "spread" associative proposal
{:a 1 :b 2 & more}

There's a subtle difference between the "rest" & and :as with associatives: the :as syntax does not exclude the keys that are destructured. This is also different than JS, which does that when using ... with JS objects.

An additional proposal would unify & as equitable to ... in JS:

(let [{:keys [a b] & more} x] ,,,)
borkdude commented 1 year ago

Sounds fine by me and since & isn't reserved as a function in Clojure, I guess we would avoid any conflicts...

It would be sweet if we could compile directly down to ... for performance, at least in squint, unlike we're doing currently. But we could slowly introduce this is more places, starting out with JSX. So {& more} would mean: pass down the more props, which should solve this issue.

borkdude commented 1 year ago

I went with :& since that seems safest with clj-kondo and helix users already know, will release a new squint shortly.

borkdude commented 1 year ago

Published v0.0.5

borkdude commented 1 year ago
Screenshot 2022-12-20 at 18 09 42