Fork me on GitHub
#reagent
<
2018-04-06
>
justinlee01:04:15

doesn’t react guarantee that the ref callback is done before did-mount? I have a normal kind of ref call back :ref #(swap! state assoc :div-node %) but sometimes in my component-did-mount that :div-node is empty

justinlee01:04:20

is this a reagent oddity?

justinlee01:04:31

okay i guess an empty chat channel can operate as a rubber ducky. i figured it out. 🦆

mikethompson01:04:26

When the silence quacks back

justinlee01:04:02

if only i’d used re-frame this would never have happened… 😛

mikethompson01:04:21

I'd like to disagree. But I can't :-)

justinlee01:04:45

that’s actually true… i got bit by the ol’ load-stuff-in-your-component anti-pattern but i swear i had a good reason

justinlee01:04:06

though to be fair dealing with canvases is just hard in react

pesterhazy08:04:59

@lee.justin.m what ended up being the problem?

pesterhazy08:04:33

(not that the ref callback is called with nil on unmount)

justinlee15:04:25

@pesterhazy oh I had a placeholder div while the data was loading. the ref was attached to the non-placeholder div. so something like (if (nil? @thing) [:div placeholder] [:div {:ref ref-cb}]. in component-did-mount I was rendering to a canvas if the data was loaded, other wise i was loading the data from server and then rendering. but occasionally, the data would come back and the render func would be called before the divs would get re-rendered. i didn’t think that was possible because i assumed the loading of the data would cause an atom-based re-render of the divs first. apparently you can’t rely on that. maybe due to the batching logic?

justinlee15:04:36

or maybe due to the fact that i was already in component-did-mount and it can’t be re-entered. not sure.

lgessler17:04:15

hi again all--I think this must be a basic question but some googling around didn't help me. I'm writing a form-2 and this is what I tried first:

(let [state (r/atom default-form-state)]
    (fn []
      [:form {:on-submit #(js/console.log "Submit")}
       [ui/text-field {:on-change #(swap! state assoc :name %2)}]]))
This causes visible UI lag, though--the rendering of the entered char is delayed until the atom is updated, I think, which causes input lag of ~100ms. I tried debouncing the function, which allowed for a better result, but this code smells:
(let [state (r/atom default-form-state)]
    (fn []
      [:form {:on-submit #(js/console.log "Submit")}
       [ui/text-field {:on-change (goog.functions.debounce #(swap! state assoc :name %2) 100)}]]))
Any suggestions?

justinlee17:04:18

@lgessler is that let at the top-level or is it wrapped in a defn?

justinlee17:04:33

second question: how are you calling this component?

lgessler17:04:00

@lee.justin.m 1) wrapped in a defn:

(defn new-project-dialog-form
  [submit-callback]
  (let [state (r/atom default-form-state)]
    (fn []
      [:form {:on-submit #(js/console.log "Submit")}
       [ui/text-field {:floating-label-text "Project name"
                       :error-text (name-error-text (:name @state))
                       :on-change (goog.functions.debounce #(swap! state assoc :name %2) 100)}]


       ])))
2) within another component:
(defn- new-project-dialog []
  (let [open (rf/subscribe [::new-project-dialog-open])]
    [ui/dialog {:title "Create a new project"
                :modal false
                :open @open
                :actions (r/as-element new-project-dialog-actions)
                :on-request-close close-dialog}
     [new-project-dialog-form]]))

justinlee17:04:23

hm okay that looks right (though I don’t know about reframe). you should definitely not be seeing 100ms delays. my instinct is that something is causing re-renders. try this

(defn new-project-dialog-form
  [submit-callback]
  (let [state (r/atom default-form-state)]
    (js/console.log "new-project-dialog-form: new component")
    (fn []
      (js/console.log "new-project-dialog-form: render")
      [:form {:on-submit #(js/console.log "Submit")}
       [ui/text-field {:floating-label-text "Project name"
                       :error-text (name-error-text (:name @state))
                       :on-change (goog.functions.debounce #(swap! state assoc :name %2) 100)}]])))

justinlee17:04:30

okay that. see if you’re getting some weird rendering

justinlee17:04:50

you should only get a new component once, and then a single render for each keypress

justinlee17:04:10

oh remove the debounce

lgessler17:04:03

That was the output for typing in "qweqweqweqwe" for the code you gave me (minus debounce). Still saw the input lag

justinlee17:04:25

so just got one render per key press?

lgessler17:04:28

that's right

justinlee17:04:52

okay so what is ui/text-field

justinlee17:04:39

because it pretty much has to be something going on in there

lgessler17:04:08

yeah... I guess that's all that remains

justinlee17:04:13

man all of a sudden everybody is using material-ui

👍 4
lgessler17:04:27

oh duh I should have tried this with a plain old <input>

justinlee17:04:41

yea try that for process of elimination

lgessler17:04:14

interesting, I'm still getting it

lgessler17:04:48

with this:

(defn new-project-dialog-form
  [submit-callback]
  (let [state (r/atom default-form-state)]
    (js/console.log "new-project-dialog-form: new component")
    (fn []
      (js/console.log "new-project-dialog-form: render")
      [:form {:on-submit #(js/console.log "Submit")}
       [:input {:type "text" :on-change #(swap! state assoc :name (-> % .-target .-value))}]
       ])))

lgessler17:04:42

wait nevermind--i removed a <p> under it that rendered it

lgessler17:04:46

and it doesn't lag anymore

lgessler17:04:03

it must be something about TextField

justinlee17:04:19

you’re beyond my expertise now

lgessler17:04:09

haha, well thanks for the help regardless. a bit surprised that TextField is apparently so slow

lgessler17:04:52

I guess I'll just write a macro that will debounce all my validator functions unless the material-ui crowd has a suggestion :man-shrugging:

justinlee17:04:06

yea that doesn’t make sense. there’s got to be more to it

pesterhazy17:04:00

material-ui wraps an input element, masking it to reagent so it can't work its ratom-to-input magic

pesterhazy17:04:05

could that be it?

pesterhazy17:04:44

the topic was discussed a few days/weeks ago, it might be worth checking the logs

lgessler17:04:06

@pesterhazy perhaps--FWIW I'm using the 0.x material-ui (nobody's made a clojurescript wrapper for 1.x yet). I found this issue and they claim to have fixed perf in the 1.x branch https://github.com/mui-org/material-ui/issues/5121

pesterhazy17:04:38

there are known issues with reagent and 3rd party input fields

pesterhazy18:04:23

do you need a controlled component? If not you might just switch to an uncontrolled component (for <Input> that would be using defaultValue rather than value)

justinlee18:04:53

i don’t think he is controlling the input

justinlee18:04:04

he has an on-change handler but isn’t setting the value

justinlee18:04:13

unless i misread the code

justinlee18:04:45

maybe material-ui is controlled internally?

pesterhazy18:04:45

oh, then it can't be the issue I'm thinking of

pesterhazy18:04:06

doesn't it have to be an issue with material-ui then?

justinlee18:04:20

the reagent controlled input issue should cause things like jumping cursors and dropped characters. it really shouldn’t cause a 100ms delay

pesterhazy18:04:21

unless you're hogging the event loop with computation

justinlee18:04:35

it’s baffling what could be doing that

pesterhazy18:04:49

i'd boil it down to a minimal repro case. just keep cutting stuff until the problem goes away

lgessler18:04:26

@pesterhazy not sure if you saw it up there but i tried replacing the mui component with a regular <input> element and the problem disappeared. i could probably dig in deeper but it seems likely that there's just a problem with how the component implements on-change

pesterhazy18:04:03

if you replace the on-change handler with (fn []) does the problem go away as well?

pesterhazy18:04:41

it might be that the input element gets unmounted and remounted on every keystroke — that might explain the problem

justinlee18:04:59

no that was my first idea: we checked that

pesterhazy18:04:39

oh, sorry 🙂

justinlee18:04:06

good to know we both had similar ideas

lgessler18:04:02

something that might be interesting: TextField contains an element that holds the value of the errorText proprety. I was feeding that a function of my state, so it was updating with every call to on-change event. Removing that label got rid of the lag I was seeing

lgessler18:04:35

I probably ought to just make that change with on-blur or something

pesterhazy20:04:46

Sounds like you found the solution

juhoteperi20:04:15

My current recommendation for custom inputs on Reagent (and even just built-in inputs) is to debounce on-change/or disable on-change if the input has focus or something, so that the input is uncontrolled when input is focused, and controlled else (so input value updates if state is updated). Based on tests I've done with Material-UI 0 and 1, the workaround Reagent uses for native inputs can't be implemented for custom inputs.

juhoteperi20:04:21

This is currently the only thing blocking 0.8 release, I'm trying to understand if I should revert the synthetic-input thing that was introduced but which I think doesn't solve anything. If I find a solution I'll add MaterialUI 0/1 and React-bootstrap examples to Reagent examples folder and write a doc about this.

juhoteperi20:04:32

I'm also waiting to see what will be official React solution for these cases when they start supporting async rendering.

pesterhazy20:04:36

Very interesting

pesterhazy20:04:17

Removing code, even hacks of course has the potential to break existing code

juhoteperi21:04:21

The code was added in 0.8-alpha2 and I doubt it is really used elsewhere than one work project where I tried to use it.

pesterhazy21:04:51

Oh I thought you meant something older than that

juhoteperi21:04:53

Removing the feature will change function arity so it will be compile time error if it is removed, so no silent breakage.

pesterhazy21:04:27

Changing from an alpha is totally fine

juhoteperi21:04:39

I'm not changing how the workaround that used for native inputs, just the option that tried to make it available for custom inputs.

pesterhazy21:04:44

It’s a fundamental problem that ratoms don’t work well with input fields

juhoteperi21:04:18

Brain dump about controlled inputs & the problem: https://gist.github.com/Deraen/762351365ac04b2057ef5f726342fd1a

👍 12
pesterhazy21:04:01

I guess it works in React because people use this.state

pesterhazy21:04:23

I thought this comment by Dustin Getz was interesting: https://www.reddit.com/r/Clojure/comments/87psf9/comment/dwez2u9

pesterhazy21:04:02

The comparison to redux is useful

juhoteperi21:04:30

My understanding is that it works with React because it doesn't use RAF, and it doesn't work there either if they would use RAF

juhoteperi21:04:07

But I still don't understand RAF/async rendering well enough to describe what it does (or how it works, and why it is good idea/or I'm not sure if it is a good idea)

juhoteperi21:04:31

But seeing that React team is looking into it, there is probably some value on implementing it correctly

juhoteperi21:04:06

But React's solution could be "you are handling the updates incorrectly (updating on each key-press), you shouldn't do it like that"

pesterhazy21:04:10

The fact remains that input value needs to update in the same tick as the change event

pesterhazy21:04:25

I feel I don’t understand the requirements 100%

justinlee21:04:50

until recently, I thought that was it (updates to value in same tick as change event), but then there is this IE bug where the change event actually comes late. to me, that would seem to be impossible to deal with, but somehow it is possible to deal with that.