Clojurians Log v2

hi, I started using helix, switching from uix -> hx -> helix, but I still have problem with rules when camel-case conversion is applied, let’s say I’m getting some native react component (coming from a library) nd then using this element in place ($ myEl {:styles …}), helix decides to not treat myEl as native component and does not convert props to native js. I cannot really apply ^:native metadata to myEl because it is a symbol bound to a native js ReactElement, and I cannot attach cljs metadata to it. is there any solution?

ah, ^:native override works with $ macro, but it does not with $$

and my react element type is react.forward_ref, so this is maybe similar issue like my recent post in #hx channel

just looking at https://reactjs.org/docs/react-api.html#createelement, it sound like type can be a “react component type”, which is probably my case forwardRef: https://reactjs.org/docs/react-api.html#reactforwardref pointing to native react component and that wants to consume native props

but it could be another simlar type, like memo or lazy, there would be the same issue I guess

and this maybe brings another more general question, I would expect that all react components created by my helix code convert props to native js props and all defnc components would give me a bean from them. it looks to me that you have some heuristics here[1] and skip this conversion when you think the consuming component is a helix component - I understand that this could be an optimization, but it leads to theses issues with more dynamic code, I’d like to strive for correctness first, I would explore a way how to do a conversion to native props lazily (e.g. via lazy js getters and let consuming helix component access cached cljs data if it finds them) - this way no heuristics would be needed [1] https://github.com/Lokeh/helix/blob/732453738a19c7c97d471c995f7471f3527b8917/src/helix/core.clj#L23-L27

hm, I should probably open some issues on github, this line looks outdated compared to the link above: https://github.com/Lokeh/helix/blob/732453738a19c7c97d471c995f7471f3527b8917/src/helix/core.cljs#L46

that last one is a bug, yeah

help me understand what you mean by forwardRef. you have to pass forwardRef a component (not a native element), I thought?

I think seeing a slice of your code would help

the heuristic that we are talking about is purely about whether to do camel->kebab conversion and deep conversion of :style prop when passing props in. it doesn’t have anything to do with how you consume props

if you create your forwardRef component like so:

(defnc ForwardedDiv [props ref]
  {:wrap [(react/forwardRef)]}
  (d/div {:ref ref & props}))

does that match what you’re doing?

@lilactown thanks for help, I’m going to push my project and show you the problems exactly

👍

needed this: https://github.com/Lokeh/helix/pull/7, please consider merging or drop my PR and write your own version

I agree with this change, but I’d also like to understand why using $$ is necessary. it should be an escape hatch; if there’s some way I can make the $ fit your case then I’d like to do that as well

@lilactown I have just pushed my current version to https://github.com/binaryage/cljs-react-three-fiber/tree/helix/examples

great

this is first issue: https://github.com/binaryage/cljs-react-three-fiber/blob/helix/examples/src/app/react_three_fiber/examples/demos/font.cljs#L146

I like the <component> naming 😄

it would be nice for helix.dom macros as well, <div> etc. would minimize clashes with existing stuff and one could refer it directly

where is react-three-fiber.core?

in the repo root

is that defined in code? or an extern?

https://github.com/binaryage/cljs-react-three-fiber/blob/helix/src/lib/react_three_fiber/core.cljs#L9

👍

I think that the ^:native metadata here is fine. either that, or you can hide it behind a component:

(defnc <canvas> [props]
  ($ ^:native Canvas {& props}))

sounds good!

but it would be even better if I didn’t have to think about this :native flag

interop should be something seamless

ideally

btw. only first 4 demos are ported to helix so far

will finish it today and then hopefully refine it

still using use-state from uix, I quite like it

https://github.com/binaryage/cljs-react-three-fiber/blob/helix/examples/src/app/react_three_fiber/examples/lib/ui.cljs#L46-L87

I agree with you on interop, but maybe not in the way you’re think. in general, I would expect interoping with external components to involve: • using :camelCase props • passing the appropriate type (e.g. JS objects instead of maps, arrays instead of vectors) as props my philosophy is that helix should make these explicit. for props, it increases grepability. and for both, there aren’t a bunch of rules you have to remember or problems you get into when you’re serializing things to JS objects and then deserializing them in event handlers

btw. also something like this could be part of helix, react is explicit about props.children to be opaque data structure https://github.com/binaryage/cljs-react-three-fiber/blob/helix/examples/src/app/react_three_fiber/examples/lib/ui.cljs#L17-L36

if you pass only one child, children is not an array for example, but the raw child

why would you even had to think about interop? all components are react component, only helix-originated components have a wrapper on “input” and “output”

this wrapper should be smart enough to do no conversion when you nest multiple helix components

helix components aren’t wrapped. they receive JS props just like any other React component

it’s only inside the body of a defnc that they get shallowly converted to a map-like object for destructuring etc.

so why is ^:native needed?

$ is some sugar to let you write (react/createElement #js {:my-prop-one foo :my-prop-two bar})

the ^:native metadata is an escape hatch I added. in react-dom, because of the ubiquity of div span etc. I opted to allow those to be written using kebab-case

the ^:native metadata can also be added to do the kebab-case conversion

it came up when someone was trying to write a wrapper for React Native’s components similar to helix.dom

but I want all my cljs code to use kebab by convention, no matter if using native or helix components

and I want all components to use camelCase when used from js code

and I want escape hatch on individual keys see https://github.com/roman01la/uix/pull/41

:man-shrugging: that’s not how helix works. the props you give a component is what it expects. If you have a component you’re going to expose to the outside world, it should take the correctly cased props as it’s API

if you want to use kebab casing everywhere in your app code, it’s not too much work to create a few smart wrappers that will do the casing for you. that’s where the ^:native escape hatch can come in

hmm, I would say, then there should be no magical camel case conversion and user really needs to be consistent

in general, there isn’t any magical camel case conversion other than for helix.dom

hmm, all my three-js components got converted when used as keywords in $

and that is what I actually wanted 🙂

yeah, that’s correct. your case seems far more strange because of the usage of a different renderer

anyways for a newcomer, I think it is quite big mental overhead to wrap his head around these rules

now I understand it, but using it as keyword works differently than importing it as a symbol, which could be quite puzzling for someone

are there cases where something could be used as both a keyword or a component?

I don’t know, but if you do it, then it behaves differently

@darwin I do appreciate your feedback. this has given me a lot to think about. I feel like the story when using helix.dom is very nice because this is all opaque, but branching out into these other renderers is starting to show chinks in the logic

have to go, will be back in 30mins

some quick reviews: https://github.com/binaryage/cljs-react-three-fiber/blob/helix/examples/src/app/react_three_fiber/examples/demos/font.cljs#L110 this $$ can be converted to ($ :scene {:name "scene" & props})

same here https://github.com/binaryage/cljs-react-three-fiber/blob/helix/examples/src/app/react_three_fiber/examples/demos/font.cljs#L116

($ :bufferGeometry {:attach "geometry" & (bean (get-gltf-geometry gltf 1)))

the line below that can similarly be converted using the & prop

documented here: https://github.com/Lokeh/helix/blob/master/docs/creating-elements.md#dynamic-props

I’m back, yep good points, have to learn about & option, thanks!

the only problem is that cursive highlights & as unresolved symbol

would you consider adding the same functionality with :& keyword?

ah that sucks

maybe. I didn’t want to remove people’s potential component API 😄

alternatively, I could define dummy & and refer it, it would be worse

I strive to have my main code without unresolved symbols, and move all problematic interop-y stuff under some kitchen-sink namespace where I don’t look that often

but I don’t want to disable unresolved symbols highlighting, that is very useful in day-to-day work

could even include that as part of helix.core

but personally I would go with my own wrapper macro over defnc and convert those on my side

with the idea (above) that conversion is happening all the time, :& prop is invalid anyways, because it has no good conversion in js world

it stringifies it. so it would be an "&" prop which is valid (although not valid JSX)

I see

or make the macro more sophisticated and interpret some (merge ...) cases

it would read well and could be copy&pasted outside

and throw error when you encounter too-dificult to understand form for static analysis

so user would then use $$ or rewrite it into supported form

you could support stuff like (merge props1 {:my “map”} props2 {:another “map”}) etc.

which is probably not supported now with &

I think I’d rather continue to use the & key and export it from helix.core like:

(def &
  "The & key can be used in helix create element macros ($, helix.dom) to merge map-like objects into static props. This symbol is exported to help with warnings about undefined code."
  '&)

you can do:

($ foo {& (merge props1 {:my "map"} props2 {:another "map"})}) 

ok, I’m not too opinionated about it, that is a solution, but thinking about refering it still feels like an unnecessary dance

Ideally cursive would have some way of adding these symbols to a list of global ignores. or respecting macros in some other way

also & or :& does not look very well when I put it on a new line and reformat my code, it gets aligned with other map keys

it looks nice as oneliner though

it does look a little different but I don’t hate it. It could be familiarity at this point.

hmm, use-effect should have a dynamic version as well, or at least warn in deps-macro-body that it was passed something else than expeced

I was passing deps via function argument (local symbol) and it silently compiled into no call

it can be ambiguous what the caller wants in some cases, but I might check for the truly meaningless case of:

(use-effect
  [foo]
  bar)

(use-effect deps …) and deps was a local symbol

I was passing it into my function as a param

interesting, yes that should be an error

the cond in deps-macro-body emits nothing

you should probably add an :else branch with some kind of exception

or emit dynamic code in that case

the hooks dependencies should be static every render so I think it should be written literally

I’m trying to remember how to emit a warning

I agree that this is really an edge case, depends how far you want to go with supporting this deps stuff in js I can pass deps as parameters, they don’t have to be in local scope

I believe

I’m not sure what you mean

I agree that you can do:

let deps = [foo, bar];
useEffect(() => doThing(foo, bar), deps);

this is the piece I was porting: https://github.com/react-spring/react-three-fiber/blob/master/examples/src/demos/Physics.js#L36

ahh interesting

I hadn’t thought about custom hooks that would want to forward dependencies

I think correct solution for your impl of use-effect would be to have a separate dynamic version, and fallback on it when you encounter non-static deps case

the problem here is that if someone passes a vector in, then it will be an actual vector and not get converted to a JS array

in your dynamic version you should handle all the semantics you have in the macro, e.g. :only :always etc

you can detect static vectors and everything else will be converted into-array if it is dynamic vector (in that dynamic branch)

yeah

I think adding an :else (to-array deps)` branch would fix it

well, it would work only partially, what if deps is bound to :only?

you don’t want your users to undertand impl details of your use-effect

it must work the same for both cases dynamic and static

if you don’t want to deal with dynamic case, then throw and call it a day

better solution than doing inconsitent partial work

yes, I see what you mean. unfortunately, I cannot infer the :auto-deps case in a custom hook if dynamic deps are passed in

ah, good point

so maybe throw on cases you don’t want to impl or cannot support

I’m still not sold on this auto-deps stuff

I’m sort of confused by the useCannon example. I wonder why it doesn’t simply add the props and fn as deps

I would rather see it as a warning if your deps vector is not in agreement with static analysis

with an option to silence it

that’s fair

for code reader it is better to see the deps explicitely and be aware that something important changed when touching the code

not sure, this is my first react app with hooks and deps

I like that you swapped order of deps and the fn itself

what I don’t like is that auto-deps forces mi to put fn body inline

sometimes I tend to define fns separately and then reference them for better code shape

hm I see

you can always do:

(defn do-thing! [foo bar]
   ...)

...
  (use-effect
   :auto-deps
   (do-thing! foo bar))

but it can get hard to parameterize over lots of complex values

I will think about adding the linting as well. In practice, I have begun to agree that I like seeing the explicit deps

I’m thinking if this form wouldn’t be possible: (use-effect (fn [foo bar] ...)) which would be rewritten into code similar to above, foo and bar could be new names for fn scope, the use-effect macro would use that fn param list as deps and it would look natural

you would not have this handling of deps argument because important info can be inferred from the fn param list

it still wouldn’t fix the current problem which is forwarding deps in custom hooks. it might even making it.. weirder

true

frankly I think this useCannon hook is bugged

hmm

I guess it’s routing around the need for users to aggressively useCallback their fn they pass in

I wonder if this would be helped by a defhook macro

I can’t really say, don’t understand it that well, I just rewrote it hopefully 1:1

FYI you can always use react/useEffect for custom hooks or helix.hooks/use-effect* which is just a proxy that does the conversion from vectors to arrays for deps, and returning js/undefined appropriately

another topic: I think I can force helix do what I wanted with naming props, 1) use ^:native everywhere, and then hack extract-cljs-props to give me a bean with custom :key->prop and :prop->key which will be mapping names from camecase back to dashes

so my components can stay in dash-land but underlying react components talk camelcase

yes, but I want to stick only to one method, if I adopt your wrapper I want to use it everywhere

you could do that, yep

I have all the demos working, just need one extra patch to allow quoted keys

currently there is no easy way how to prevent camelCase conversion selectively on individual keys

btw. impl.utils don’t seem to be used anywhere

helix.impl.utils

thanks, its usage was removed in a refactoring

what’s your use case where you want to opt out of camelCase conversion sometimes?

it should handle aria- and data- fields already

https://github.com/react-spring/react-three-fiber/blob/534fe1aaa63d27dcf65f43d05566977df759eef1/examples/src/demos/Montage.js#L54-L55

dashes are used to go deeper into objects in three.js, I guess

😵

btw. using replace with function should be more perfomant than orig impl

if you wanted I could provide tailored :cljs branches for going directly via interop without clojure core

sure, that would be helpful

something like this: https://gist.github.com/darwin/e1d990dea50d153b4a7474f59653cc80

I’m slowly being convinced camelCasing all the time

I think this would be a backwards compatible change, and it would make the life of everyone not using react-dom a lot easier

I think there would still need to be some determination of whether a value is “native” or not, when deciding whether to recursively convert the :style prop to JS. but maybe I could just implement that in helix.dom instead of concerning the $ / $$ with that

I would vote for doing nothing by default, but providing hooks for people to pick different policy or own functions

not sure about performance, because this will be a hot path

yes, that is the reason I de-opted for converting to camelCase at one point

but if people are using $, the performance should be negligible because props will be converted at macro time if they’re written statically

yes that’s what I like the most on helix, that static conversion

also that cljs keywords will become strings in rewritten code

I looked at uix results in advanced mode, it optimizes well, but there stayed a bunch of cljs keyword objects, which should ideally go away

I still think I will be able to get the best of both worlds, and convert only when dynamically crossing boundary between js and cljs land

or maybe only in dynamic cases which are rare

btw. your current “& props” idiom expects native js objects? didn’t see ->js conversion there

it expects a map. I want to extend it to work with JS objs too

when it detects & it emits a call to -native-props on the CLJS side, which does a shallow conversion of the map-like into a JS object using the same rules

ah, good, thanks for explanation

I have just updated the pull request

some tests would be nice, I didn’t try the clj path, but its generic code worked under cljs for me

wait, the PR is broken I think

I misplaced close paren of that reader tag

fixed

I’m not a fan of cljc files, I’d rather write it twice 🙂

hehehe. in these instances where I’m trying to mirror the same behavior I feel like it helps me sync changes to both

I’ll need to circle back on this later today / early tomorrow, my day job needs my attention for a bit

sure. no worries, thanks for all your help

helix looks very promising

I really appreciate you trying out helix and all of the feedback you’ve given me. PRs are the cherry on top. it’s given me a lot to think about! and will help a lot

FYI, I have just compiled in release mode (via shadow-cljs) and the keywords related to hiccup really went away (nice!)

but the code preparing and setting displayName is still there, I would expect it to be elided in release mode

I would expect that too. it should rely on goog/DEBUG https://github.com/Lokeh/helix/blob/master/src/helix/core.clj#L145

i wonder if the output of cond-> is messing with it. I could pull it out into a separate expression from the def

it would probably need some help: https://gist.github.com/darwin/1bdf46da01b16ecc19410091fca7d325#file-box-js-L60

also I compared the shadow-cljs report abou sizes with uix and sometimes the js files are a tiny bit bigger

no big deal, but there will be probably some room for improvements

that line above is $APP.$cljs$core$truth_$$(!1) && ($G__19540$$.displayName = "react-three-fiber.examples.demos.box/\x3cdemo\x3e"); with pseudo names

I bet uix and reagent will be tiny bit smaller since it’s actually emitting less code

tradeoff of emitting less code and doing more abstraction at runtime

but yes, probably work to be done there for helix. I think we can get close enough