reagent-project / reagent

A minimalistic ClojureScript interface to React.js
http://reagent-project.github.io/
MIT License
4.75k stars 415 forks source link

Improve JS value detection #335

Open jeluard opened 6 years ago

jeluard commented 6 years ago

When calling adapt-react-class props of type JS array are not recognized as JS value: js-val? should also consider array type. This forces Clojure data serialization via clj->js that might not be wanted in some cases.

Deraen commented 6 years ago

Can you provide some example cases? Is array as props even valid in React?

IIRC, Clojure data serialization will always happen with adapt-react-class, but in that case user shouldn't use that function anyway, but use r/create-element to pass the Clojure data into component.

mattly commented 6 years ago

ReactNative's FlatList component's data prop requires an array. I found a workaround here https://github.com/status-im/status-react/blob/d05f50db6238117f1cee7bdff8973ac1b7802d14/src/status_im/ui/components/list/views.cljs#L110 , but it would be nice to not have to do this.

jeluard commented 6 years ago

Right this is indeed my use case.

Deraen commented 6 years ago

Looking at this, I don't understand what is the problem.

js-val? checks if the value is something that is already js-value, and if it is, like number of string (or array), it is not converted from Clojure to JS.

reagent.impl.template.js_val_QMARK_([1, 2]);
true
reagent.impl.template.js_val_QMARK_({a: 1});
false
reagent.impl.template.js_val_QMARK_("1");
true

I tested this with a component taking props map containing property with array value, and single child value which is array. The values are correctly passed into the React component as is.

(def c (create-react-class ...))
(def x (adapt-react-class c))

(r/render [x {:data #js [1 2 3]} #js [4 5]])
solidfox commented 5 years ago

Not sure if I'm seeing the same problem but I'm having a similar issue with a flat list where each item is taking an admittedly crazy huge props map (megabytes of data if printed). For some reason I can't quite pinpoint reagent deems it necessary to convert this insane props map resulting in the render taking seconds.

This is extra delicate since flat-list is often used to render thousands of items making it really important for components to be able to quickly say "don't re-render me!"

Here's an example of how I use flat-list

[rn/flat-list
 {:key-extractor (fn [item _index] (str (get-in item [:title-metadata :id])))
  :renderItem    (fn [js-kwargs] (reagent/as-element [continue-reading-list-item (aget js-kwargs "item")])) ; Item is crazy huge.
  :data          (apply array
                        (for [title titles]
                          {:title-metadata       title
                           :on-press             on-press
                           :bookmarked-props     (core/create-bookmarked-props state module-context {:item-data title})
                           :adjusted-client-time (response/adjust-client-time response (time/now))}))}]