Performance issue when using r/atom as a function parameter

[markokocic]

I posted this first in Stackoverflow, but it could actually be a reagent bug.

I work on the react-native application using Clojurescript, latest re-frame and latest reagent. I have one text input component, and have two versions of the code:

Version 1: input text is a separate component, and state atom is passed as an argument, the same as the recommended in the reagent library docs and examples.

(defn todo-input [value]
  [rn/text-input
   {:style          (styles :textInput) :multiline true
    :placeholder    "What do you want to do today?" :placeholder-text-color "#abbabb"
    :value          @value
    :on-change-text #(reset! value %)}]
  )

(defn todo-screen []
  (let [value (r/atom nil)]
    [rn/view {:style (styles :container)}
     [rn/text {:style (styles :header)} "Todo List"]
     [rn/view {:style (styles :textInputContainer)}
      [todo-input value]
      [rn/touchable-opacity
       {:on-press (fn [] (rf/dispatch [:add-todo @value]) (reset! value nil))}
       [icon {:name "plus" :size 30 :color "blue" :style {:margin-left 15}}]]]
     [todos]
     ]))

Version 2: everything in one component.

(defn todo-screen []
  (let [value (r/atom nil)]
    [rn/view {:style (styles :container)}
     [rn/text {:style (styles :header)} "Todo List"]
     [rn/view {:style (styles :textInputContainer)}
      [rn/text-input
       {:style          (styles :textInput) :multiline true
        :placeholder    "What do you want to do today?" :placeholder-text-color "#abbabb"
        :value          @value
        :on-change-text #(reset! value %)}]
      [rn/touchable-opacity
       {:on-press (fn [] (rf/dispatch [:add-todo @value]) (reset! value nil))}
       [icon {:name "plus" :size 30 :color "blue" :style {:margin-left 15}}]
       ]]
     [todos]]))

The issue is that first version has a performance issue while typing, since there's a big delay and flickering when trying to type fast. Version 2 doesn't have any issues, and I can type as fast as I can.

According to the reagent documentation, passing r/atom as a parameter should not incur any performance issues.

Am I doing something wrong here? What would be the best way to avoid performance penalty here.

This is a small example, and having one component instead of two is not a big deal, but splitting one big to multiple smaller components in a good praxis. State here is local to the component, and I don't want to use re-frame for it.

[lucywang000]

Regarding, your example code,

(defn todo-screen []
    (let [value (r/atom nil)]

I think you should be using with-let here, right? Otherwise the ratom would be recreated every time the component is re-rendered. So I suggest you update your code, otherwise it's hard to tell where the performance issue comes from.

State here is local to the component, and I don't want to use re-frame for it.

That make sense, but without storing the state outside the component it won't work well with hot-reload, unless you use with-let.

[markokocic]

You are right, my code is wrong. But when I switch to with-let I get the same flickering issue like in version 1.

For example I changed my version 2 and tried both with-let and form-2 and in both cases I get issues when typing.

(defn todo-screen []
  (r/with-let [value (r/atom nil)]
  ...

(defn todo-screen []
  (let [value (r/atom nil)]
    (fn []
      ...

(defn todo-screen []
  (r/with-let [value (r/atom nil)]
    (fn []
      ...

The original version 2 as posted above is the only one that doesn't have this issue.

[markokocic]

Just found out #444 where it mentions that reagent doesn't play well with controlled components where the value is controlled by passing value to the :value of textInput component.

[markokocic]

OK, to summary, it looks like not using :value to update the text on the component solves flickering issues, but it makes it impossible to control or set the value of the component from the outside. As soon as I add :value @value text input component starts behaving.

Javascript react native version doesn't have any issues.

However, the following code, although incorrect, appears to "work", but that may be just a coincidence.

(defn todo-screen []
  (let [value (r/atom nil)]
    [rn/view {:style (styles :container)}
     [rn/text {:style (styles :header)} "Todo List"]
     [rn/view {:style (styles :textInputContainer)}
      [rn/text-input
       {:style          (styles :textInput) :multiline true
        :placeholder    "What do you want to do today?" :placeholder-text-color "#abbabb"
        :value          @value
        :on-change-text #(reset! value %)}]
      [rn/touchable-opacity
       {:on-press (fn [] (rf/dispatch [:add-todo @value]) (reset! value nil))}
       [icon {:name "plus" :size 30 :color "blue" :style {:margin-left 15}}]
       ]]
     [todos]]))

So the main issue is that reagent doesn't work well with react-native text input components in controlled mode, where input is after updated set to the new value using :value property.

[markokocic]

After further investigation and reading the comments on re-natal issue 124 and Stackowervflow question it looks like the issue is that reagent delays re-rendering of the text-input component.

Forcing the rendering using r/flush in change handler seems to solve the issue:

(defn todo-input-container []
  (let [value (r/atom "")]
    (fn []
      [rn/view {:style (styles :textInputContainer)}
       [rn/text-input
        {:style          (styles :textInput) :multiline true
         :placeholder    "What do you want to do today?" :placeholder-text-color "#abbabb"
         :value          @value
         :on-change-text (fn [v]
                           (reset! value v)
                           (r/flush)
                           )}]
       [rn/touchable-opacity
        {:on-press (fn [] (rf/dispatch [:add-todo @value]) (reset! value ""))}
        [icon {:name "plus" :size 30 :color "blue" :style {:margin-left 15}}]
        ]]
      )
    ))

This also explains why the code from the previous comments "works". It's because the element gets re-rendered on every invocation.

At this point, I'm not sure if this is a reagent bug, react-native bug or simply a quirk one has to learn to work around. Javascript version of the code doesn't have this issue.

[lucywang000]

Great findings!

This also explains why the code from the previous comments "works". It's because the element gets re-rendered on every invocation.

This still can not explain ... when the ratom is a local variable, on reach re-render it's created again and its value is empty string, so how could the input be remembered?

[Deraen]

There is too much unrelated information in this issue to really say what is happening.

Problems with controlled inputs with Reagent are known "feature": https://github.com/reagent-project/reagent/blob/master/doc/ControlledInputs.md. The workaround that is applied to DOM inputs can't be used with React-native, so pretty much the best option is currently to use uncontrolled inputs (default-value & on-change) in RN. Not sure if similar workaround could be created for RN inputs, but probably not, because RN doesn't provide API to control the input selection.

Option to disable async rendering could be useful, but would require big refactoring of Reagent implementation.

The problems aren't really connected to how RAtoms are created or used. Same problem would happen even if with React component state or any other source of state, when using controlled inputs.

[lucywang000]

Option to disable async rendering could be useful, but would require big refactoring of Reagent implementation.

Two noob questions

1. does reagent really need to add a batch/async re-rendering layer, given that the the forceUpdate calls are batched by react reconciler anyway?
2. Since react itself also renders in an async/batch manner, why it doesn't have a problem with controlled inputs? Does it have some special handling for it like reagent?

Update: I can answer the second question myself: it seems react does have special handling for controlled inputs:

• https://github.com/facebook/react/blob/v16.13.1/packages/react-dom/src/client/ReactDOMComponent.js#L1246-L1261
• Use setImmediate to defer value restoration facebook/react#1759
• Use explicit pass for restoring controlled state instead of asap facebook/react#8176

[Deraen]

By the way, there is more discussion about RN TextInput in this old issue: #119

[StarScape]

This is really a shame. This basically kills any possibility of using Reagent with RN. Even the r/flush trick doesn't seem to solve the flickering for me. Actually, after reading the fine print, it looks like r/flush will work, but only if I name the prop :on-change-text, not :onChangeText. That may or may not be obvious to someone with more Reagent experience than me 🙂.

[Deraen]

Closing this. There are several other issues about the controlled inputs and there is some work being done to see if it is possible to remove Reagent batching.