-
Notifications
You must be signed in to change notification settings - Fork 47.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Issue: useEffect
executed before browser can paint DOM update
#20863
Comments
https://codesandbox.io/s/effectrendering-forked-svcqr?file=/src/App.js Hopefully I fixed your issue. The useCallback hooks required dependencies and are unnecessary. When loading is true then the value of loading and setLoading are passed to the wrapper and used there. useEffect will now only be called when the value of loading is changed. |
@PolybiusPro You fixed the issue by actually removing the blocking Also, you are of course right that the |
I noticed the same behavior here: |
I noticed the same thing on react-native. I am able to update view position with a call to the native side in use effect before the changes are drawn to the screen. |
Any updates regarding this issue? I am currently working around this by having a timeout so that changes can be drawn to the screen, but that is no really satisfying solution for obvious reasons. |
Time Slice addresses this maybe ? |
This isn't true AIUI. If you change the state during any unrelated
You can read this bottom up as: React caught the event, is doing a sync render, and then it flushes the passive effect (because not seen on the stack, the change has changed and its preparing to re-render).
You shouldn't run blocking code from a useEffect, particularly due what I mentioned above. If you must, then at least run it outside of the effect to avoid blocking the paint. |
Could you check this example: https://stackblitz.com/edit/react-3jsceq ? It doesn't look to block the paint to me, since the button gets greyed out after the first click, it keeps getting the click events though . What's the explanation of this behavior ? |
In this case this works as expected right? Your event handler triggers a render, which has a passive effect and nothing special causes that effect to run before the paint. This work as you intent it. Now if you have something else in your application that causes the effect to flush before the paint then suddenly it doesn't work as you expected. Like this: https://stackblitz.com/edit/react-8zxitw Note that it's line 17 that's responsible for |
This is an alternative to setTimeout you can use 'requestAnimationFrame' `export const LoadingWrapper = ({ callback, children }) => { return <React.Fragment> {children} </React.Fragment>; Why I have used requestAnimationFrame twice, I have seen it in a video presented by google chrome team. |
If you're in an event handler, the first |
I thought so too, but if I am not mistaken here this is still experimental, is it not?
Then I find the documentation quite misleading, since it states:
So this does not mean the updates should be visible within the browser? Also, in the given example, there is neither a
In this case, what would be a good practice / pattern for this usecase (rendering a loading indicator before executing a blocking function call) without using effects or experimental features? |
I was expecting that too, but apparently it's not the case. I was trying to use it for CSS animations which ended up not working as expected in this way. As far as I understand it, the situation is like this:
From my research, a (incomplete) hook that always triggers after a repaint has been completed could look like this: function useAfterPaintEffect(effect, dependencies) {
useEffect(() => {
requestAnimationFrame(() => {
setTimeout(() => {
effect();
}, 0);
});
}, dependencies);
} Explanation (source: archived page "MozAfterPaint" on MDN):
I think nesting either requestAnimationFrame + setTimout or 2x requestAnimationFrame as per the comment of @pervezalam777 fits our use cases. I've also seen this in libraries, e.g. in react-animate-mount I agree with @limekiln that the documentation is wrong - the documentation states that an effect runs after the browser has painted |
Here's another small examle that demos the actual behavior (a fade-in animation): https://codesandbox.io/s/react-effect-vs-paint-qkt1c?file=/src/App.tsx As I said, I think the actual behavior and the documentation don't match. |
I could reproduce the issue of having For example, this means that we cannot use The solution by @marko-knoebl with |
Hm, I can't confirm this observation. Setting a breakpoint will actually change / "fix" the behavior in the codesandbox that I posted. If I set a breakpoint on 34 that code will start working- it will render the transparent div in that instant. |
Found an iteresting comment here: According to this comment, React should be using some combination of Edit: The answer points to some old code, the exact file apparently doesn't exist anymore. |
thanks @marko-knoebl, I've used this technique to fix my scroll restoration issues (previously had a less elegant solution with a short setTimout) |
I think you are confusing how useEffect works, here is the correct flow for hiding and showing a fade element (it will animate in and out and remove from DOM once done): https://codesandbox.io/s/react-effect-vs-paint-forked-4ie9z?file=/src/App.tsx From my experience, useEffect does run correctly as stated in docs:
If this was not true, my example above would not work as intended. If we study your example on codesandbox, you can see that the order of operations is correct, but the opacity is always 1 because you return a empty node when the opacity is 0 - this means that transitioning from 0 to 1 is impossible because the node that uses opacity is never in the DOM when the opacity is 0. The reason it works with a request animation frame or set timeout is very simply because it is being executed after the effect in the call stack. |
@abacaj thanks for taking the time to respond - very much appreciated. You're saying about my code: "you return a empty node when the opacity is 0" - I think you meant "when phase is 'unmounted' ". It's true that in that phase I'm returning an empty node - but then I'm also rendering something in phase "beforeEnter". Thanks for your code - I'll take a deeper look into it and would use it as a reference in the future. But I still think it's true that an effect can run before the paint - depending on how the component is coded. |
https://thoughtspile.github.io/2021/11/15/unintentional-layout-effect/ This is an interesting article that explains a specific case where "useEffect" fires before paint. Again, I think this is a documentation issue. |
tl;dr for people only reading the last few comments: Any state updates scheduled from I agree that the documentation could be better/clearer in this regard. (cc @gaearon, @rachelnabors). My advice:
|
I was trying to point out that your example did not make use of The reason that there is an issue with It was also pointed out by @thoughtspile: reactjs/react.dev#4107 (comment)
Agreed, it is not well documented and I imagine could cause unwanted behavior like blocking the painting of the browser. |
Behavior in React 18 should be more consistent. Updates caused by discrete user input events (like clicks or typing) should make useEffect run synchronously. Otherwise, it should be asynchronous. Would somebody like to check whether the original repro has become more consistent in 18? Thanks! |
Yes, the behaviour looks fixed now with react@18 stackblitz . The button click gets disabled immediately now, even though it doesn't get visually greyed out. There's also another discussion on SO, with the weird effect showed here: codesanbox. Not sure if related though, but react@18 does not seem to change anything in the behaviour ( tested with both @17 and @18 ). |
What is the "weird effect" here, could you please clarify? |
I think we can close this because 18 behavior is consistent. Unless I'm missing something. |
The CSS transition looks to work only if the |
Is that different from the browser behavior when the change is synchronous? Meaning, is there anything React-specific about this question? |
I tried to change the opacity imperatively inside the ref callback and it has the same issue with transition, so I think you are right, this is not React related. |
I indeed encoutered the same problem: useEffect delaying the painting. This problem occured after a click event meant to trigger some state update. I did not try to observe useEffect behavior for other use cases yet.
|
After expressing my best regards @gaearon , I'm bringing to your attention that I've modified the sandbox you referred to earlier, by adding a delay of 2 seconds ( |
Sorry I don't understand which sandbox you're referring to. Can you rephrase what the question is? |
Sorry, here is the sandbox link
And to let you know that
|
To my understanding,
useEffect
should always run after the browser acutally did paint any updates. This way, I tried to ensure that a loading indicator is rendered after a user submits data but before a blocking function execution (e.g. a fetch operation where the data is waited on) is executed. The code looks somewhat like the following:Wrapper component to which a loading indicator can be passed:
Usage:
I would expect from the documentation that the
callback
function is executed after the "Loading" div is rendered on the screen, but that is actually not the case. The new div appears after the blocking call is completely resolved.To get it working, I acutally have to use a local state variable in addition to trigger the effect:
Using this, I also observed some strange browser dependent behaviour. If input changes (which lead
loading
being set totrue
) are submittedonBlur
of the input, it works fine in Chrome as well as in Firefox. Submitting these changesonKeyPress
(while checking if the pressed key was theEnter
key), it works in Firefox, but not in Chrome.Does anyone have an idea what exactly is going on here or do I miss something important?
UPDATE:
I added a code sandbox: https://codesandbox.io/s/effectrendering-umtwo
The behaviour is quite strange:
In Chrome, triggering
onBlur
works (almost) always, sometimes it seems to bug out. TriggeringonKeyPress
will never trigger the spinner, except whenonBlur
was triggered before with the exact same input string (even though this string should have no influence at all here).In Firefox, neither
onBlur
noronKeyPress
will trigger the spinner for me.Tested versions:
Chrome 88.0.4324.150 (64-Bit)
Firefox 85.0.2 (64-Bit)
The text was updated successfully, but these errors were encountered: