Clojurians Log v2

@tony.kay are you aware of this?

yeah, it is certainly believable that the new inputs for react 18+ are misbehaving. That’s why the RC has been running so long. I don’t feel super safe rewriting something that was so well-tested and is also pretty difficult to get right.

I’ve been waiting for all the bugs with inputs to crop up. I thought we’d gotten them all

thanks for the minimal repro. I’ll try to get to it soon

try rc11. I rewrote how the DOM input works. It tests out for me now.

I think I was actually able to simplify it, and it seems to work for the cases I can think of to test it against.

I suspect part of the simplification was enabled by the fact that I got the top-level integration with react concurrent rendering right.

of course I am. That’s how closures work. Deal with it all the time all over the place in front and back end. Nothing specific to React hooks here. It can be a little annoying to track down when/why, but it’s actually quite a common thing people confuse themselves about.

but when not using hooks it doesn't happen as far as I know? the function just gets updated

Also remember that clj had vars, which help somewhat, but cljs does not. A lot of the values are just, well, values. So closing over them is much more common in the front-end

what is ui-drag-and-drop???

My guess is that it is somehow telling react not to re-render (so it can re-capture the onFileSelected fn), which is leaving it with the prior version.

useMemo?

but yes, if you specifically re-grab the timeframe data from props at the higher level (as you say works), then if doesn’t matter if the lambda is closed over, because the props on this are a mutable thing from one render to the next. That’s my best guess.

it's a component of ours

Personally this style of programming is not something I prefer unless the dynamism of the app actually requires it. Hooks are “convenient”, but I don’t think they are necessarily the best way to structure an app you want to reason about. You end up with all sorts of mess. I support them because they are sometimes quite necessary to avoid other complexities and are the lesser of two evils. But I can count the components that use hooks in my 250k line code base on my hands.

anyway, hooks just trigger re-renders of components in complex ways, but they don’t rewrite your code (like core async does). So, IF the hook triggered an update, THEN you got new props and your let ran. the onFileSelected would have be regenerated and closed over the new value, and the body would have been called. Thus my guess that this code isn’t the problem, but instead the closure happened in the drag and drop component

AH, your DnD component is query-less…and you’r e passing two args to it’s factory? But not using computed props?

yes I guess that's what I was saying, that hooks mixed with closure seem to have the unexpected effect of staling values

hooks really don’t figure into this case AFAICT

> AH, your DnD component is query-less…and you’r e passing two args to it’s factory? But not using computed props? I thought that was the issue too but I tried with computed props as well, same staling issue

put a println at the top of your DnD component, and put one at the top of your hooks-based component. See if the child ALWAYS re-renders when the parent does.

the only way I see this going wrong is that your parent is re-rendering, but your factory is preventing the child (e.g. :shouldComponentUpdate is returning false in the child)

Try adding :shouldComponentUpdate true to the child

or you otherwise made some weird custom factory for the child that is causing the skip of render

it’s possible that when there is no query my pre-generated shouldComponentUpdate is malfunctioning

so you could also switch the child to hooks, which then stops using the classic component callbacks.

as mentioned I have a working solution already but happy to experiment, I was just sharing this in the hope we may find a way to fix/document this to make fulcro more consistent/user friendly

> Personally this style of programming is not something I prefer unless the dynamism of the app actually requires it. Hooks are “convenient”, but I don’t think they are necessarily the best way to structure an app you want to reason about. You end up with all sorts of mess. I support them because they are sometimes quite necessary to avoid other complexities and are the lesser of two evils. But I can count the components that use hooks in my 250k line code base on my hands. I totally agree on that, it's cargo culting in the codebase, we only had forms that were part of the routing around the time hooks came out, and they opened up the possibility to put forms anywhere without hving to deal with the setup (like starting the state machine), so from there they got overused and I don't think we have a single instance of a RAD form being started "manually" (it's either used directly in a router or with the hook)

I don’t know that there’s anything to document. This has nothing really to do with Fulcro, unless it turns out that the queryless component is preventing rendering in a way that’s a bug. That’s why I asked to see the sub-component, because I wanted to know if you’d accidentally prevented child renders there.

@tony.kay what is your approach for these kind of generic components, where the props aren't really something you are going to find in the app db, do you always pass them as computed props?

Computed props are part of rendering optimization for stateful components with a query.

https://book.fulcrologic.com/#_passing_callbacks_and_other_parent_computed_data

I'm talking generic components like ui-drag-and-drop above, you often have props such as :supported-mime-types there that are known even at compile time since they are local config of the generic component.

@tony.kay so I had time to go back to that component to experiment here are the result: • I added a print at the top of ImportStudentModal and DragAndDrop and they both print everytime I type something, but when I drop a file:

[{(brian.model.organization/import-users-from-csv
   {:organization-id 1653,
    :timeframe-data {},
    :com.fulcrologic.fulcro.networking.file-upload/uploads
    [{:file/name "students.csv", :file/content {}}]})
  [:tempids *]}]

I also rewrote ImportStudentModal with no hooks: and it still called onFileSelected with stale data

I think I finally figured out whats wrong

I added a button in the drag and drop. you can see 2 mutations one with stale data and one without

the one without is by clicking the button and after that dropping a file calls the muitation again with stale data

And here is a minimal repro:

(defsc PrintingButton [_ {:keys [onClick]}]
  {}
  (dom/div (dom/input
            {:id "file-input"
             :type "file"
             :accept "csv"
             :multiple false
             :onChange (fn [evt]
                         (when onClick
                           #?(:cljs
                              (onClick)
                              :clj evt)))})
           (dom/button {:onClick (fn [_]
                                   (println "before click")
                                   (onClick)
                                   (println "after click"))}
                       "Click to print")))

(def printing-button-ui (comp/factory PrintingButton))

(defsc TopChrome [this {:ui/keys [value]}]
  {:query [:ui/value]
   :ident (fn [] [:component/id :top-chrome])
   :initial-state (fn [_] {:ui/value ""})}
  (dom/div
   (dom/span "Enter something here:")
   (dom/div
    (dom/input {:onChange (fn [e] (m/set-string! this :ui/value :event e))}))
   (dom/div
    (printing-button-ui {:onClick (fn [] (println (str "" value ""))) :data value})
    (dom/span "You entered: " value))))

(def top-chrome-ui (comp/factory TopChrome))

(defsc Root [this {:root/keys [top-chrome]}]
  {:query [{:root/top-chrome (comp/get-query TopChrome)}]
   :initial-state (fn [_] {:root/top-chrome (comp/get-initial-state TopChrome {})})}
  (top-chrome-ui top-chrome))

@tony.kay I think there is definitely a bug here I think it is in dom/input that closures get stalled

ah, are you using the latest RCs?

it is definitely possible that the new input wrapper isn’t updating properly

I didn’t think it had to do with hooks, and I couldn’t see how the other theory was true either, so I’m glad you came up with something that seems more plausible 😄

I'm on 813679f235252fe801f7a87a4b0a3b49f4a1336a

3.9.0-rc11-SNAPSHOT