I’m testing Reagent 2.0.0-alpha1 and followed the upgrade steps in the README. Everything works fine, except for live reloading. Our render setup looks like this:
(defonce react-root (delay (rd/create-root (.getElementById js/document "app"))))
(defn ^:dev/after-load mount-root []
(rf/clear-subscription-cache!)
(rd/render @react-root [root/component]))
It renders fine initially, but after change some components, any state-updates aren’t re-rendered. So it’s essentially a static page. I can see the re-frame state changing to something new by checking internally, but this isn’t reflected in a new UI update. However, if I wrap root/component in a var like this:
(rd/render @react-root [#'root/component])
then hot reload works.
Any idea why this happens? Feels like there’s an underlying issue that shouldn’t require the var workaround.So the problem with this optimization/simplified impl is that with: • one ratom • one reaction using the ratom value • one component using both the ratom value and the reaction value changing the ratom value once, will trigger component render twice, because the ratom change has time to trigger the render before reaction is updated and then that will trigger the render again.
^ Notes about trying out removing of one of Reagent queues in this thread Performance is promising but it might break the Reagent's promise of ratom/reaction changes triggering only one render
isn't the whole deal with react19 and async rendering that you can't guarantee "one render" rendering everything anyway?
it is possible with flushSync, but might be a bad idea
they do strongly recommended not using it, but their docs aren't obviously for cases like Reagent where we are already queuing the changes ourselves
the async rendering is pretty bad for some ratom update cases, instead of the change being shown after 16ms, it will take 32ms to DOM being updated
(select-row benchmark on the first run, before I added flushSync)
adding the flushSync to ratom change handling doesn't affect async rendering with state hooks or any other state sources
And the "two component" render problems in that ratom example isn't really about React rendering stuff multiple times, but about changing the Reagent ratom queue so that it causes Reagent to tell React about the changes twice. Vs. the current 4 queues (separate ratom and component queues), where Reagent will end up telling React about the changes only once.
So the React 19 rendering possibly splitting the rendering into multiple frames isn't even relevant for this case.
Ah, so it’s not an isolated case. Good to know! I’m happy to help debugging the alpha version further. So feel free to ping me. Our reagent frontend is around 50k LOC. So it’s quite decent in size and probably covers all kinds of edge-cases which could help with debugging.
I'll check what is the deal with live reloads and update docs or the impl. I think I want the reagent API to work without any of these workarounds (react key or vars.)
I think I used some tricks on the tests to run code after React is done with rendering, I might have to add the same to the after-render queue flushing.
Though for tests my solution is just waiting for 17ms which isn't great for after-render.
Maybe I can use flushSync in afterRender to force React to flush its queue before flushing Reagent queue. Not perfect but might be the only option.
Sounds good! You could consider useEffect maybe. It runs after things are committed to the dom. There might be some possibilities there. Although, it needs to added to the component-hierarchy, and it only runs when that specific component re-renders. As far as I know that’s the only way to know react is done rendering, or by forcing it with flushSync, like you said.
There is likely no way to use hooks due to how Reagent API works
ah, check
One option is to just remove after-render completely, and provide documentation how to use hooks instead
Interestingly the live reload works for Reagent examples. Could be because those are just one namespace.
Yes
I'm not sure if this is 2.0 difference
If your root component is in different namespace than render call, shadow-cljs doesn't by default reload all the transitive dependents of the changed namespace.
You change the root ns -> root js file is loaded again and new component fn is evaluated BUT the namespace with render call isn't evaluated again so the vector in the old mount-root call still refers to the old fn value. Var fixes this by adding indirection so that the Reagent will always resolve the current value on render time.
:reload-strategy :full also fixes this, as it makes shadow-cljs eval all the transitive dependents of the changed ns.
Mmhmhmh but hot reloading works fine in Lipas master?
I don't understand your key hack. It doesn't make sense.
Did you experience the problem that came without it?
And hm lipas master works indeed with the old Reagent and without reload-strategy full.
I don't understand why my hack works either.. 😄
But that's how I got it to re-render.
Hmhmhm. Not evaluating the dependent ns (the ns with render call) should be fine because fns in Cljs just compiles the fn to JS value... so the var indirection shouldn't be necessary.
The [root/component] should just refer app.ns.root.component JS value (Closure modules etc.) so even without evaluating the core ns it should get the latest version of the component fn always.
Buuuut maybe there is some fn comparison logic in React which causes it to use the previous fn if it thinks it didn't change.
OR Reagent as-element!
which turns the [root/component] into Reagent component
The difference could be due this wrapper: https://github.com/reagent-project/reagent/blob/master/src/reagent/dom/client.cljs#L22-L43
Looks like using createElement on line 31 works
Not sure why
It kind of makes sense the "as-element wrapper component" created in render fn should be used as a react element instead of a regular fn inline in the initial-flush-render wrapper component
Maybe I can refactor it to avoid two wrapper components
https://github.com/reagent-project/reagent/commit/817456f10fc6d5d4029ee1688c96da430cee7526
This hopefully makes live reloads work better.
Available as a snapshot now:
reagent/reagent {:mvn/version "2.0.0-SNAPSHOT"}
@stex do you have some cases where it looks like after-render is called later than before, i.e. before React has mounted updated elements into DOM? I did try very quick case of adding after-render callback to the intro page "The atom click-count has value:" component where it just prints the current atom value and DOM value, and it seems the latest atom value is already in DOM here.
hmm
moving the after-render to button on-click on that component does show that atom value is updated, but dom still has the old value
I guess it makes sense
If the after-render callback is only added at the render of the reagent component, react has already the update queued, and after-render queue and react dom mount likely take the same 1 animation frame. Adding the after-render callback outside of the render does cause it really to wait for Reagent to flush updates from on-click ratom changes, but then it React will take additional time.
I have one specific case in our app. But i haven’t isolated that one yet. But it seemed obvious to me that this was due to the scheduling of react being different now. I think what you’re saying now confirms that. reagent waits, calls the functions, but than it takes some additional time by react to render.
Our use-case is a bit of hack. We call r/after-render to reset the scroll position to the top of the page when you navigate to another page. Although hacky, that has worked for us for many years now. When updating the reagent alpha you could clearly see the the browser scrolling to the top on the old page (just a flash) and then rendering the new page.
I would likely recommend useEffect hook for such cases
Yeah, we will migrate to that. We’ve been hesitant to use useEffect with react 17 and the old reagent. Still a bit confused what the performance penalty is for using functional components with reagent. Also not sure how that is for the alpha version. But we opted to avoid it until we have time to profile it.
I think this page indicated there was some penalty. https://github.com/reagent-project/reagent/blob/master/doc/ReagentCompiler.md. I guess to use useEffect you have to use :f> .
Slow part is turning the hiccup-like data to React elements on runtime
Yes I think the way ratom updates are triggered for functional components vs the class based components is a bit slower
due to functional components having to kind of re-implement class component API...
BUT that benchmark was also with React 16, it is possible the React hook performance is better now
but it’s probably fine if you just sprinkle some :f> i guess. And I’m also curious what react 19 does yeah. Alpha reagent “felt” faster with 19, not sure if it’s placebo.
I could update (at least my own branch) of the benchmark repo to compare a few reagent and react versions
That would be useful for us. But please only do so if you think it’s beneficial for others also. We need to profile anyway when we’ll be upgrading in production.
There are some interesting results here. Like why select-row is so much slower for reagent-react-19. The differences for 1.3 and 2 fn and non-fn are pretty small in this benchmark.
not sure why uncompressed size is also so much larger for reagent-react-19 versions
huh, react-dom/client is just that much larger vs react-dom
Cool to see the benchmarks. Seems like you got a ways faster laptop recently 😉. Differences (except for select row) aren’t that big indeed 👍
The repo benchmarks are 5 years old so they are 2 or 3 laptops ago 😄
The benchmark setup itself could also have changed a bit
Sidenote: Not sure where to report this.
I think reagent/after-render has different behaviour now with React 19. My guess is that this is being called after the request animation frame of Reagent, but React also does some scheduling, so it’s likely still before the React renderer is done.
Same problem, but I found a semi-ugly workaround https://github.com/lipas-liikuntapaikat/lipas/pull/160#issuecomment-3009612002 Juho is currently on holiday, but he'll look into it once he's back. 🌴
And thanks for testing the alpha! All testing and feedback is helpful.
That var trick is actually nicer than my react-key hack. Dunno why it works though and the "normal case" doesn't.
Interesting, I can get better results (especially for select-row) by adding flushSync call to where Reagent flushes its ratom change queue to React. Might be fine, it is once per animation frame, only when you are making ratom changes. It also has affect that after-render callback really sees the changes mounted to DOM.
Some batching notes: Reagent has 4 queues that are flushed on each animation frame: 1. before-flush queue 2. ratom changes (reaction change triggering dependent reactions) 3. component render 4. after-render Ratom/reaction changes are very dependent on having batching, so the requestAnimationFrame queueing can't be fully removed from Reagent. From point of view of 2. ratom queue, every component render body is also a reaction! This is how component renders catch which reactions/ratoms are used within render body. On render reactions, there is code that also adds the component to queue 3. when dependencies for that reaction changed. It is important that the ratom queue is handled first during the animation frame, before component renders, so we can't remove any one of these queues alone, even though React now queues the renders itself. Maybe. I think. This flushSync solution avoids the additional React queue for component renders, because now Reagent forces React to flush all those component changes right-away into DOM. ... Now that I'm writing this out, I'm starting to wonder if it would still be possible to remove 3. component queue, if 2. ratom queue just directly triggered the React updates. BUT then the after-render queue would be problematic.
3. component render queue is also ran in component mount order, to ensure parent components are rendered first. This ensures no component is rendered multiple times, because rendering the children from the parent render will mark them clean, and then they aren't rendered again even if the children was also added to the queue by itself. This might be why I'm seeing some tests break when testing this idea. For example: there is a ratom change that triggers both parent and child render, if child is rendered first, and then parent is rendered, also rendering the children again, the children gets rendered one extra time.
Or maybe the React itself is wise enough to handle those cases...
The test failures could be something else.
Just removing 3. queue from Reagent has the same problem then as without flushSync in flushing that queue. Then the ratom changes in 2. queue will add component renders into React queue and there is again double batching for component renders.
Adding the flushSync around 2. ratom queue flush could work.
This looks quite promising. Same perf as old reagent with react 16.