Skip to content
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

Document that componentDidUpdate happens before paint #8452

Closed
landabaso opened this issue Nov 29, 2016 · 12 comments
Closed

Document that componentDidUpdate happens before paint #8452

landabaso opened this issue Nov 29, 2016 · 12 comments

Comments

@landabaso
Copy link

landabaso commented Nov 29, 2016

From the docs:

componentDidUpdate() is invoked immediately after updating occurs. This method is not called for the initial render. Use this as an opportunity to operate on the DOM when the component has been updated.

Please, correct me if I'm wrong. When the the last component in a large sequence of nested component changes, then all cDU from last child to parent are called in sequence. After the last cDU is called, then flushBatchedUpdates is run which, in turn, calls runBatchedUpdates.

Read this comment from @sebmarkbage for more context:
#2659 (comment)

If this is correct, the docs should say something like: Use this as an opportunity to enqueue DOM operations in the next execution thread.

EDIT: I guess this issue is also related to new hooks proposal: componentWasMounted and componentWasUpdated ( #7678).
In the meanwhile, it would be great if the docs specified that componentDid/Mount/Update should use requestAnimationFrame/setTimeout to access the rendered DOM.

@landabaso landabaso changed the title componentDidUpdate: Use this 2 operate the DOM - But cDU is run after ReactUpdates.flushBatchedUpdates componentDidUpdate: Use this 2 operate the DOM - But cDU is run BEFORE ReactUpdates.flushBatchedUpdates Nov 29, 2016
@jftesser
Copy link

jftesser commented Apr 8, 2017

I was confused by this documentation too. Adding a description of a reliable way to access the rendered DOM would be much appreciated.

@gaearon gaearon changed the title componentDidUpdate: Use this 2 operate the DOM - But cDU is run BEFORE ReactUpdates.flushBatchedUpdates Document that componentDidUpdate happens before paint Oct 4, 2017
@bvaughn
Copy link
Contributor

bvaughn commented Oct 8, 2017

The comment referenced (#2659) is really old. I wouldn't read much into it since React has changed a lot in the last 3 years.

In the meanwhile, it would be great if the docs specified that componentDid/Mount/Update should use requestAnimationFrame/setTimeout to access the rendered DOM.

I'm not sure I understand why this is necessary. Could you elaborate? did-mount and did-update are called after changes have been flushed to the DOM, so you can synchronously access refs or their backing DOM elements in those methods.

The documentation and source code for reactjs.org now lives in a different repository: reactjs/reactjs.org. (For more info on why we made this move, see issue #11075.) Let's re-open a new issue in that repository to continue the discussion if you're still interested!

@bvaughn bvaughn closed this as completed Oct 8, 2017
@jankalfus
Copy link

@bvaughn The methods get called after the changes have been flushed to the DOM, but that's irrelevant. The problem is that they get called before the first browser painting happens, not after.

To give you an example, when you have an element with overflow set to scroll, the clientWidth/clientHeight property will report incorrect values in componentDidMount. However, correct values get reported if you wrap your code retrieving the values in setTimeout or requestAnimationFrame.

@bvaughn
Copy link
Contributor

bvaughn commented Oct 11, 2017

when you have an element with overflow set to scroll, the clientWidth/clientHeight property will report incorrect values in componentDidMount. However, correct values get reported if you wrap your code retrieving the values in setTimeout or requestAnimationFrame

Can you point me at a repro that shows this behavior, by chance?

The methods get called after the changes have been flushed to the DOM, but that's irrelevant.

As for the relevance 😄 I was just pointing out that requestAnimationFrame / setTimeout are not required to access DOM elements from cDM/cDU because those elements already exist and are accessible. Updating the docs to say that you should add async delay before accessible DOM elements might give the (incorrect) impression that those elements haven't been created and appended yet when the lifecycle hooks are called.

@jankalfus
Copy link

jankalfus commented Oct 11, 2017

Sorry, looks like a was hasty. setTimeout/requestAnimationFrame did not solve the problem for me.

In my particular case, I've nailed it down to padding causing the problem. For some reason, the 'clientWidth' that's received in cDM is smaller by the total padding amount of the surrounding component (maybe it gets painted first without the padding?). But it is accurate if obtained a bit later.

I'll attach a fiddle if I can reproduce this.

@bvaughn
Copy link
Contributor

bvaughn commented Oct 11, 2017

is this being caused by a box-sizing style by chance? (Something that may be inherited from a parent style that may be changed?)

Okay. Thanks.

@jankalfus
Copy link

Nevermind, my issue was caused by something else. Sorry.

@bvaughn
Copy link
Contributor

bvaughn commented Oct 11, 2017

It happens. 😄

@jgosmann
Copy link

jgosmann commented Dec 6, 2018

I did encounter exactly the issue that clientHeight in componentDidMount was reported incorrectly. Moving the code to a callback for requestAnimationFrame solved the problem.

@bvaughn
Copy link
Contributor

bvaughn commented Dec 10, 2018

I did encounter exactly the issue that clientHeight in componentDidMount was reported incorrectly.

Huh...that does not sound expected. Assuming stylesheets have already loaded (since they might affect layout) then reading clientHeight from within componentDidMount should force the browser to do a sync layout (not great but sometimes necessary) and return a valid clientHeight.

@jgosmann
Copy link

We also use Aphrodite and I just stumbled across this:

Aphrodite uses asap to schedule buffer flushing. If you measure DOM elements' dimensions in componentDidMount or componentDidUpdate, you can use setTimeout or flushToStyleTag to ensure all styles are injected.

While I haven't verified it yet, this might be the reason for the behavior I observed.

@bvaughn
Copy link
Contributor

bvaughn commented Dec 14, 2018

Sounds like the likely cause to me ^

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants