Use component constructor to keep track of mount order

Previous change (35ff5d33dd) started using ComponentDidMount to keep
track of component mount order. This affected the order in which this
was called, previously ComponentWillMount was called the first for
parent components and then for children. ComponentDidMount was called
first for children etc. To work around this, the mount order was
reversed when updating components after ratom updates.

Problem with this is, that when only some components are rerendered,
they get new numbers, but their parents don't:

(given components c, b, a)

**0.8.1**

c 1 > b 2 > a 3

a rerendered
c 1 > b 2 > a 4

b rerendered
c 1 > b 5 > a 6

**35ff5d33dd**

c 3 > b 2 > a 1

a rerendered
c 3 > b 2 > a 4 (BROKEN)

b rerendered
c 3 > b 6 > a 5 (BROKEN)

Best way to fix this is to revert back to old way, where parents get the
smaller number, this was re-rendering children doesn't change the order.
To implement this the mount-order can be stored in component
constructor, which seems to work similarly to ComponentWillMount.

> The constructor for a React component is called before it is mounted.

> UNSAFE_componentWillMount()... Generally, we recommend using the constructor() instead for initializing state.
This commit is contained in:
Juho Teperi 2019-12-16 21:05:31 +02:00
parent 07bfa901ac
commit 6cb6561ba6
3 changed files with 29 additions and 9 deletions

View File

@ -29,8 +29,8 @@
;; top-most component is mounted last and gets largest
;; number. This is reverse compared to WillMount where method
;; for top component gets called first.
(- (.-cljsMountOrder c2)
(.-cljsMountOrder c1)))
(- (.-cljsMountOrder c1)
(.-cljsMountOrder c2)))
(defn run-queue [a]
;; sort components by mount order, to make sure parents

View File

@ -229,12 +229,7 @@
:componentDidMount
(fn componentDidMount []
(this-as c
;; This method is called after everything inside the
;; has been mounted. This is reverse compared to WillMount.
(set! (.-cljsMountOrder ^clj c) (batch/next-mount-count))
(when-not (nil? f)
(.call f c c))))
(this-as c (.call f c c)))
:componentWillUnmount
(fn componentWillUnmount []
@ -259,7 +254,6 @@
;; Though the value is nil here, the wrapper function will be
;; added to class to manage Reagent ratom lifecycle.
(def obligatory {:shouldComponentUpdate nil
:componentDidMount nil
:componentWillUnmount nil})
(def dash-to-method-name (util/memoize-1 util/dash-to-method-name))
@ -340,6 +334,7 @@
(construct this props))
(when get-initial-state
(set! (.-state this) (get-initial-state this)))
(set! (.-cljsMountOrder ^clj this) (batch/next-mount-count))
this))]
(gobj/extend (.-prototype cmp) (.-prototype react/Component) methods)

View File

@ -1371,3 +1371,28 @@
(r/flush)
(is (= {:height 20} @did-update))
(.removeChild js/document.body div))))))
(deftest issue-462-test
(when isClient
(let [val (r/atom 0)
render (atom 0)
a (fn issue-462-a [nr]
(js/console.log "rendering a")
(swap! render inc)
[:h1 "Value " nr])
b (fn issue-462-b []
[:div
^{:key @val}
[a @val]])
c (fn issue-462-c []
^{:key @val}
[b])]
(with-mounted-component [c]
(fn [c div]
(is (= 1 @render))
(reset! val 1)
(r/flush)
(is (= 2 @render))
(reset! val 0)
(r/flush)
(is (= 3 @render)))))))