search

madvas / cljs-react-material-ui

Clojurescript library for using material-ui.com
Eclipse Public License 1.0
205 stars 32 forks source link

`Can only update a mounted or mounting component` with slider in Rum #25

Closed alvatar closed 7 years ago

alvatar commented 7 years ago

This code fails:

[:div [:p (:min (rum/react ui-values))] ; <-- removing this will avoid the error
(ui/slider {:min 200 :max 20000
                :value (:min (rum/react ui-values))
                :on-change #(swap! ui-values assoc :min %2)})]

The full error is:

Can only update a mounted or mounting component. This usually means you called setState() on an unmounted component. This is a no-op. Please check the code for the Slider component.

I'm using the latest Rum

edannenberg commented 7 years ago

Not familiar with Rum but looks like a typo to me:

[:p (:min (rum/react ui-values))]

(misssing : for min)

alvatar commented 7 years ago

Yeah, sorry for the typo, I wrote it here, didn't copy-paste it :)

alvatar commented 7 years ago

Full example:

(def ui-values (atom {:val 500}))

(rum/defc mycomp < rum/reactive []
  (ui/mui-theme-provider
   {:mui-theme (get-mui-theme {:palette {:text-color (color :blue900)}})}
   [:div [:p (:val (rum/react ui-values))] ; <-- actually now fails even with this removed
    (ui/slider {:min 200 :max 20000
                :value (:val (rum/react ui-values))
                :on-change #(swap! ui-values assoc :val %2)})]))

(rum/mount (mycomp) (js/document.getElementById "app"))

<!DOCTYPE html>
<html>
  <head>
    <meta charset="UTF-8">
    <meta name="viewport" content="width=device-width, initial-scale=1">
    <link href="css/style.css" rel="stylesheet" type="text/css">

  </head>
  <body>
    <div id="app"></div>
    <script src="js/compiled/main.js" type="text/javascript"></script>
  </body>
</html>
alvatar commented 7 years ago

This is the full error:

Uncaught TypeError: Cannot read property 'getBoundingClientRect' of null
    at Slider.getTrackOffset (material-ui.inc.js:53885)
    at material-ui.inc.js:53918
getTrackOffset @ material-ui.inc.js:53885
(anonymous) @ material-ui.inc.js:53918

Warning: setState(...): Can only update a mounted or mounting component. This usually means you called setState() on an unmounted component. This is a no-op. Please check the code for the Slider component.
printWarning @ material-ui.inc.js:1297
warning @ material-ui.inc.js:1321
getInternalInstanceReadyForUpdate @ material-ui.inc.js:17264
enqueueSetState @ material-ui.inc.js:17416
ReactComponent.setState @ material-ui.inc.js:1780
onDragStop @ material-ui.inc.js:53929
Slider._this.handleMouseEnd @ material-ui.inc.js:53745
tonsky commented 7 years ago

Looks to me that every time component created through adapt-rum-class (which is pretty much every component in this lib if you’re importing Rum version, new React class is created:

(defmacro adapt-rum-class [react-class]
  `(fn [& args#]
     (let [[opts# children#] (if (map? (first args#))
                               [(first args#) (rest args#)]
                               [{} args#])
           type# (first children#)]
       (let [new-children# (if (vector? type#)
                             [((rum.core/defc ~(gensym) [] (last children#)))]
                             children#)]
         (cljs-react-material-ui.core/create-mui-cmp ~react-class (cons opts# new-children#))))))

(note the call to ((rum.core/defc ~(gensym) [] (last children#))))

This is very-very wrong thing to do. defc should be called once, you can get a reference to generated class from it by doing something like:

(defc x [] ...)
(:rum/class (meta x))
madvas commented 7 years ago

Thanks for the tip! I admit it may be wrong since I never actually used Rum, so I'm not too familiar with it. Will try to fix it, any help appreciated :)

alvatar commented 7 years ago

Thank you @tonsky and @madvas. It works.