reagent-project / reagent

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

after-render fn executed before render when used in component-did-mount #241

Closed tomconnors closed 2 years ago

tomconnors commented 8 years ago

Unsure whether this is a bug or misunderstanding on my part about the purpose of after-render. When called in component-did-mount, the after-render function executes before the next render. Example:

(def after-render-bug-demonstration
  (reagent/create-class
   {:component-did-mount (fn [this]
                           (prn "component did mount")
                           (reagent/after-render
                             (fn []
                               (reagent/set-state this {:new :state})
                               (prn "component did mount - after-render"))))
    :reagent-render (fn []
                      (let [s (reagent/state (reagent/current-component))]
                        (prn "reagent-render")
                        [:div "hi"]))}))

I would expect the second render (due to the state change) to occur before after-render's fn, but "component did mount - after-render" is printed before the second "reagent-render".

holmsand commented 8 years ago

@tomconnors That is by design, I'm afraid. The after-render function is called at the next "animation frame" when Reagent has done its rendering, i.e as soon as the DOM is updated to reflect application state (i.e what you have in your Reagent atoms). It may be useful when you want to do something that requires the DOM to be in a known state.

What happens in your example is that your component doesn't actually have to be rendered again (it is already up-to-date), so the after-render function is called directly (in the next frame). Then you change state, triggering a rendering in yet another animation frame.

This could obviously be documented better... Any ideas for something clear, short and snappy? :)

tomconnors commented 8 years ago

Maybe a doc string change: "Run f using requestAnimationFrame or equivalent, after any queued renders. f is called in the next animation frame, regardless of whether any renders actually occur."

I'd also propose adding another function, after-next-render that is only executed when a render actually occurs. If you'd take a PR for that, I'll make the code change.

holmsand commented 8 years ago

@tomconnors Thanks! I've added something very similar to the doc string.

Re after-next-render: I am, of course, reluctant to add more stuff... It also seems to me that it would be a bit fragile – if you absolutely have to wait for a certain component to be rendered, it seems like it would be cleaner to just use component-did-update directly? after-next-render would be "app-wide" in scope, and so would be called if some other component suddenly needs updating (perhaps leading to bugs of the dreaded timing related kind...). You could also of course call after-render from the render function itself, if you want to avoid React's oo style.

Can you give a use case for such a thing, and a great argument why it would be such an indispensable feature to have? ;)

tomconnors commented 8 years ago

My use case is animation. The component's state atom has a :style key that is initially set to hide the component (using opacity). In component-will-mount, I measure a few relevant elements in the component's DOM, then update the state to show the component and add css transforms and transitions. Then, after the render for that state update occurs, I need to update the state once more to remove those styles. It's similar to how ReactCSSTransitionGroups work but they weren't flexible enough for my needs.

I'm currently wrapping my call to after-render in a setTimeout, which works fine. Unless my first paragraph was more convincing than I think it was, I'll happily just continue using setTimeout.

ducky427 commented 8 years ago

@tomconnors, have you seen react-motion project?

Apologies, if this is not applicable for your needs.

tomconnors commented 8 years ago

Thanks for the link, perhaps I can make use of that TransitionMotion component.

holmsand commented 8 years ago

@tomconnors I see – and I'm afraid that I agree about how convincing it was :) And if it works using setTimeout, that will probably be more flexible as well.

But perhaps after-render could be more useful for animation. As it is, the "after-render function" is called in the same animation frame if after-render is called during a normal "queued rendering" (i.e initiated by requestAnimationFrame), if after-render is invoked during that rendering (i.e in the render function, component-did-update etc). But it will be invoked in the next animation frame if e.g after-render is called in the render function during the initial render, or if one is forced using force-update.

I'm thinking that maybe it would be useful to guarantee that a function scheduled during rendering with after-render will always be called immediately after any rendering is complete (and hence before the browser has been given time to display what is rendered) – i.e for the initial render, force-update as well.

That could be useful I imagine exactly in cases like yours, where you want to render, measure something in DOM and render again in one go.

Deraen commented 2 years ago

I'm closing this old issue.

I think React has good enough tools for animations cases even without Reagent now.