Fork me on GitHub
#re-frame
<
2020-08-19
>
Oliver George05:08:09

@mikethompson really like where this is going

Oliver George05:08:30

Some thoughts here rather than a comment as I'm not sure I can distill it yet...

Oliver George05:08:16

Specifically regarding reg-event-fx2.

Oliver George05:08:51

The example in your comment is compatible with the current calling conventions. To compose event handlers from smaller functions the example handler prepares the initial value {:db... :fx...} and specially passes event as an arg to composable helpers (without this the helpers miss out on cofx)

Oliver George05:08:55

Here's the what if...

Oliver George05:08:28

What if the calling convention was different to streamline those tasks.

Oliver George05:08:14

Call the handler with one arg which includes :db, :fx, :event and any other :cofx keys.

Oliver George05:08:44

That can flow through all logic safely.

Oliver George05:08:49

That different calling convention would justify a new reg- I think

Oliver George05:08:20

For what it's worth I settled on the idea that "state" flows through the handler logic.

Oliver George05:08:40

:db and :fx are "state" in the sense that they represent the (1) current state of the system as input, and (2) new "state" of the system as return value.

Oliver George05:08:02

For :db the transition of state is (reset! app-db)

Oliver George05:08:25

For :fx the transition of state is (doall (map do-effect ms))

Oliver George05:08:56

It feels like we're moving closer to the core concepts of a finite state machine which might be a useful reference point for describing how it holds together.

p-himik05:08:48

I think it sounds potentially useful. But isn't it orthogonal to :fx? Sure, :fx makes composing things easier, but composing is possible without it.

Oliver George05:08:36

One thing which makes composing hard today is asymetry in the data shape in vs out

p-himik05:08:59

Not really - there's reg-event-ctx.

Oliver George05:08:31

Shooting from the hip... definitely close to what I'm talking about in terms of calling convention. Perhaps specifically different in terms of how the return value (state change) is processed.

p-himik05:08:27

The only hiccup here is the presence of the second argument, yes.

Oliver George05:08:45

Pretty sure the event-v arg is always redundant since it can be accessed via ctx as the :event arg. (I should check but presumably injected via a standard cofx.)

p-himik05:08:26

It is indeed redundant in terms of the available information. But it makes things a bit easier given that you almost always need to drill into that vector. With the second argument:

(fn [ctx [_ arg1 arg2 arg3]] ...)
Without the second argument:
(fn [{[_ arg1 arg2 arg3] :event :as ctx}] ...)

p-himik05:08:21

I'm not saying that it precludes anyone from implementing a new reg-event-* function, of course.

Oliver George05:08:37

It is very convenient for that I agree. Things like this might be common if the approach got up... (defn get-resp [s] (assoc-in s [:db :data] (get-in s [:event 1])))

Oliver George05:08:04

Today's lack of logic composability is also yuk... just more familiar 🙂

Oliver George05:08:32

I think there's interesting possibilities for using cofx to declare/type/spec/unpack/conform event args in useful ways

Oliver George05:08:43

But it's not core to the idea

p-himik05:08:34

Just so there's something concrete to talk about - have you actually encountered some problems in the wild that would definitely be easier to solve were there event handlers composition available? Can you share them?

Oliver George05:08:50

Shooting from the hip again....

Oliver George05:08:09

One issue is that I have many events in my system which should be handled in similar ways.

Oliver George05:08:45

Another is generalising something I do often.

Oliver George05:08:05

Without easy composability those helpers become heavy to hookup/use

Oliver George05:08:27

And without helpers I either duplicate logic

Oliver George05:08:33

Or dispatch more events

Oliver George05:08:46

when the eventloop becomes an api you can make quite a mess

Oliver George05:08:06

Not sure these are smoking guns but it's something I reach for often

Oliver George05:08:07

I have to head out soon but will check back later. Thanks for your thoughts @p-himik

p-himik05:08:50

With easier composability you will have to be careful about the event vector that you pass to dispatch and handle in your event handlers. Creating complex behaviors by utilizing many simple functions that operate on just db/`ctx` and a set of named arguments makes it much more flexible than using comp where each function absolutely must expect the same kind of argument and absolutely must have the event vector have specified things at a very specific position. To be more concrete. Suppose I have this function that would work in the scenarios that you describe:

(defn add-a-to-db [ctx]
  (assoc-in ctx [:db :a] (get-in ctx [:event 1])))
As you can see, it expects events like [event-id value-of-a ...]. And then I also want to have this function:
(defn add-b-to-db [ctx]
  (assoc-in ctx [:db :b] (get-in ctx [:event 1])))
Do you see the problem here? These functions look composable, but they are not! If you decide to replace 1 in add-b-to-db with 2, then suddenly even the simplest event that just wants to add b, will have to use events like [event-id nil value-of-b] for no clear reason.

Oliver George05:08:21

Yep, we're talking here about coupling your helper implementation with event structure

Oliver George05:08:25

(If I was rebooting re-frame I'd use a map for the event data.)

Oliver George05:08:59

Does that apply to all types of data sharing... I'm trying to generalise your point so it's not about event data shape specifically.

Oliver George05:08:02

Its not inherantly wrong to pass pass a second argument to your helper. Threading macro is our friend here.

Oliver George05:08:26

I'll think more about this.

p-himik05:08:19

> Threading macro is our friend here. And it's already perfectly possible with reg-event-fx. :)

(fn [ctx [_ a b]]
  (-> ctx
      (add-a-to-db ctx a)
      (add-b-to-db ctx b)))

Oliver George06:08:46

A second argument (event-v) doesn't get in the way of the core idea.

p-himik06:08:26

My point is that if you're content with not being able to use comp (which is the case with the second argument), then the core idea is already implemented by reg-event-ctx. I don't see how it can be improved to facilitate composition.

Oliver George06:08:36

I'll have to try reg-event-ctx to comment. Great if it's useful.

Oliver George06:08:57

My guess is (reg-event-ctx :x identity) would throw a heap of errors

Oliver George06:08:09

Specifically because things in ctx don't marry up with :fx handlers

Oliver George06:08:28

Happy to be wrong

p-himik06:08:13

reg-event-ctx expects a function with 2 arguments. identity is a function with 1 argument. If you mean (reg-event-ctx :x (fn [ctx _] ctx)) then it will work just fine without any errors.

Oliver George08:08:09

I stand corrected. Sounds fine then.

mikethompson06:08:09

@olivergeorge > (If I was rebooting re-frame I'd use a map for the event data.) I can remember agonizing over that long and hard. But if I was doing it all again, I probably wouldn't change that decision. I'm relatively happy with that one.

mikethompson06:08:12

(Which is not to say that maps wouldn't have some advantages ... just saying that I'm still happy with the overall balance)

p-himik06:08:43

And right now you can still use events like [:event-id {:x 1 :y 2}].

Oliver George08:08:11

Perhaps I'm drinking too much "positional args are the devil" cool aid. I do like a qualified key.

Oliver George08:08:13

But that's all an aside to the ticket I think except that if @p-himik is right then perhaps a new reg isn't necessary. (I do need to see it to believe it fully though, can't see how it'd be tolerant of cofx keys appearing in return value when they don't match up to registered fx handlers). EDIT: Yeah technically reg-event-ctx would work and is more powerful than I see need for. Ticks the "simple" box but somewhat less "easy" for common use cases. I'd still vote for the handler arg format to be state-in (a map of cofx including :db and perhaps an empty :fx) and state-out (a map of :db, :fx and other stuff which is ignored).

Oliver George09:08:39

I make it a point of contradicting myself at the earliest convenience.

👍 3
Oliver George09:08:47

Re: maps vs positional args...

Oliver George09:08:14

I think it'd be nice to open the door to multiple args for fx handlers. Accepting vectors or maps in the :fx seq would do that.

Oliver George09:08:22

implementation could be something like...

(defn do-fx [ms]
  (doseq [m ms [id & args] m]
    (apply (get-fx id) args)))

Jason17:08:11

is it fair to say that if a js component uses react hooks it cannot be used in a re-frame app? Edited to add "yes"

lilactown18:08:16

it should work fine. are you running into issues?

lilactown18:08:34

I should say, it should work fine as long as you’re using a recent version of React that includes hooks

Jason19:08:32

Thanks for replying. I would love to be proven wrong. Re: React version, I'm on react and react-dom 16.13.1. I've been trying to replicate https://material-ui.com/components/popover/#mouse-over-interaction in my re-frame app. The sample code (click "<>") uses react hooks.

Jason19:08:58

My best guess cljs is

(defn hover-sample [{:keys [^js classes]}]
  (let [[anchorEl setAnchorEl] (react/useState nil)]
    [:> Paper {:class [(.-paper classes) (.-fixedHeight classes)]}
     [:div
      [:> Typography {:id "hover-anchor"
                      :aria-haspopup "true"
                      :onMouseEnter #(setAnchorEl (-> %
                                                      .-target))
                      :onMouseLeave #(setAnchorEl nil)}
       "Hover with a popover"]
      [:> Popover {:id "mouse-over-popover"
                   :className (.-popover classes)
                   :classes {:paper (.-paper classes)}
                   :open (not= nil anchorEl)
                   :anchorEl anchorEl
                   :anchorOrigin {:vertical "bottom"
                                  :horizontal "left"}
                   :transformOrigin {:vertical "top"
                                     :horizontal "left"}
                   :onClose #(setAnchorEl nil)
                   :disableRestoreFocus true}
       [:> Typography "I use Popover."]]]]))
but that violates hook rules. If there's a way to refactor this to play nice, I am all ears

lilactown19:08:06

ah, so you need to use a react hook inside of a reagent component

lilactown19:08:41

that’s a different case than use a component, which uses hooks 😄 but we can still get there

Jason19:08:31

Please pardon my newb. I did say "in a re-frame app" so I thought I was implying the rest. I guess not!

lilactown19:08:41

well, the distinction I’m making is the difference between:

[:> SomeReactComponentThatUsesHooks {:foo "bar"}]
and actually using a hook inside of a component you are authoring using reagent. does that make sense?

lilactown19:08:57

(I’m working on an example solution for you here as well)

Jason19:08:11

I am daring to believe I'm not out of luck. It feels great

lilactown19:08:32

in this case, we don’t actually need to use useState. the material docs use a useState hook because it’s a convenient way to create some local state; the most convenient way to handle this in reagent is to use a local atom, like:

(defn hover-sample [_]
  (let [anchor-el (reagent.core/atom el)]
    (fn [{:keys [^js classes]}]
      [:> Paper {:class [(.-paper classes) (.-fixedHeight classes)]}
       [:div
        [:> Typography {:id "hover-anchor"
                        :aria-haspopup "true"
                        :onMouseEnter #(reset! anchor-el (.-target % ))
                        :onMouseLeave #(reset! anchor-el nil)}
         "Hover with a popover"]
        [:> Popover {:id "mouse-over-popover"
                     :className (.-popover classes)
                     :classes {:paper (.-paper classes)}
                     :open (not= nil @anchor-el)
                     :anchorEl @anchor-el
                     :anchorOrigin {:vertical "bottom"
                                    :horizontal "left"}
                     :transformOrigin {:vertical "top"
                                       :horizontal "left"}
                     :onClose #(reset! anchor-el nil)
                     :disableRestoreFocus true}
         [:> Typography "I use Popover."]]]])))

Jason19:08:08

I had a local ratom version which had firing hover events before I tried to do hooks, but let me try yours

lilactown19:08:23

there’s currently an alpha version of reagent which does allow you to use hooks inside of reagent components but I don’t really understand how to use it tbh

lilactown19:08:46

the docs have not been updated AFAICT

Jason19:08:26

Compiler says 'el' is not defined in the ratom init

lilactown19:08:48

oops, should be nil

Jason19:08:22

that builds and renders, but this is where i ended up with my local ratom attempt too. When I put the mouse over the anchor element and leave it stationary, I can see the mouse flicker

Jason19:08:49

when i had my local ratom set up to log events to the console, it was logging :onEnter and :onLeave in pairs, continuously

Jason19:08:57

FWIW, i can also say that if i comment out the :onLeavehandler, the popup shows up (and stays there)

lilactown19:08:10

I wouldn’t expect that to change whether you use local react state, or a local ratom

lilactown19:08:16

but I could be wrong

Jason19:08:59

I sure didn't expect it but as a newb to front-end I thought there might be some magic in using hooks so I was trying that in desperation

lilactown20:08:46

there’s a way we can test this

lilactown20:08:08

all JSX is a syntax for calling React.createElement

Jason20:08:09

yes, i've used the reagent wrapper elsewhere in this code

lilactown20:08:22

(ns my-app.some-feature
  (:require
    ["react" :as react]
    [goog.object :as gobj]
    ...))


(def $ react/createElement)


(defn hover-sample* [props]
  (let [^js classes (gobj/get props "classes")
        [anchorEl setAnchorEl] (react/useState nil)]
    ($ Paper #js {:class #js [(.-paper classes) (.-fixedHeight classes)]}
     ($ "div"
      ($ Typography #js {:id "hover-anchor"
                         :aria-haspopup "true"
                         :onMouseEnter #(setAnchorEl (-> %
                                                      .-target))
                         :onMouseLeave #(setAnchorEl nil)}
         "Hover with a popover")
      ($ Popover #js {:id "mouse-over-popover"
                      :className (.-popover classes)
                      :classes {:paper (.-paper classes)}
                      :open (not= nil anchorEl)
                      :anchorEl anchorEl
                      :anchorOrigin #js {:vertical "bottom"
                                         :horizontal "left"}
                      :transformOrigin #js {:vertical "top"
                                            :horizontal "left"}
                      :onClose #(setAnchorEl nil)
                      :disableRestoreFocus true}
       ($ Typography "I use Popover."))))))


(defn hover-sample [{:keys [classes]}]
  ($ hover-sample* #js {:classes classes}))

Jason20:08:34

...just a sec while i figure out how to integrate that....

lilactown20:08:55

☝️:skin-tone-2: the above creates a normal functional react component hover-sample* that uses hooks. hover-sample is a reagent component that handles interop between reagent & react. you could also skip that and just call [:> hover-sample* {:classes ,,,}] I just wasn’t sure if you wanted to rewrite all the callers

Jason20:08:42

me either just yet 🙂 , thinking

lilactown20:08:33

hover-sample* basically completely routes around trying to use reagent, which is what allows us to use hooks. it’s then easy to use the hover-sample* component in our reagent project

lilactown20:08:48

this is what I meant by distinguishing between: > [using] a js component [that] uses react hooks and > using hooks inside of a reagent component

Jason20:08:51

i see. hover-smaple is of course buried in a view so this will take some fiddling

Jason20:08:24

In order to cut out all callers, i'm transplanting this into my main.cljs, in mount-root. If that's a bad idea, feel free to say so

lilactown20:08:51

i don't really know what you mean but whatever we can do to test to see if the flickering goes away

Jason21:08:15

i'm still here, trying to get the classes into the props. Please bear with me

lilactown21:08:56

nw, I went and did some yoga 😄

Jason22:08:02

As mentioned, I'm new at this stuff and I am now getting errors which make me wonder whether I'm trying to do the right thing. My code is based on https://github.com/dakra/mui-templates and I'm trying to insinuate hover-sample https://github.com/dakra/mui-templates/blob/9f3a997856af199394c7dbc31a4432397590a268/src/mui_templates/main.cljs#L142 , my version of which is unchanged. Would you care to offer an opinion? I've transplanted styles and a with-styles wrapper from dashboard.cljs but am getting

Error: Element type is invalid: expected a string (for built-in components) or a class/function (for composite components) but got: object.
I hate to ask but I'm not sure which way to head

Jason22:08:54

Wait, I may be misunderstanding how far up the call stack i have to go. Can I use your hover-sample inside other material-ui components?

lilactown23:08:51

I don’t really see where you’re trying to use the example code I pasted

lilactown23:08:09

I would try and create a minimal example, instead of rewriting a huge component like the dashboard component in your project

Jason23:08:43

I'll keep trying to get more minimal. Thanks for your patience

Oliver George21:08:51

There's a functional component branch in the reagent repo with aims to improve this sort of thing (I think).

Jason21:08:50

Thanks, that's been added to my list of things to try