react-three-renderer icon indicating copy to clipboard operation
react-three-renderer copied to clipboard

r3r appears to fire react lifecycle methods out of expected order

Open AndrewRayCode opened this issue 9 years ago • 7 comments

PROBLEM

It appears that r3r renders components in an unexpected order, firing React lifecycle hook methods in an unexpected order. Right now this is what I believe I'm seeing:

  1. Parent render is called
  2. Parent componentDidUpdate is called
  3. Child componentWillReceiveProps is called
  4. Child render is called

This is the order I expect:

  1. Parent render is called
  2. Child componentWillReceiveProps is called
  3. Child render is called
  4. Parent componentDidUpdate is called

I'm not 100% sure about the order of willReceiveProps with regards to children, but I believe that the parent componentDidUpdate firing before the child render completes is an error.

HOW TO REPRODUCE

  1. Clone my fork of the examples
  2. npm install
  3. npm run serve
  4. Open http://localhost:8080/
  5. Click on the "Simple" example on the sidebar
  6. Open the Javascrit debugger in your browser
  7. Run window.debuggg = true
  8. Note the order of the logs:
wrapper render
index.js componentDidUpdate called
Cube.js componentWillReceiveProps
Cube.js render

This is a crude repro case but I think it demonstrates the problem. I've modified the "Simple" example to have a child object called Cube.js. I've added lines such as this one:

    if( window.debuggg ) {
      console.log('index.js wrapper render');
    }

...at 4 points throughout the code. Only when the child Cube.js renders is the debug flag turned off:

    if( window.debuggg ) {
      window.debuggg = false;
      console.log('Cube.js render');
    }

You can see in the order of what's printed (copied above), it appears that the component in Simple/index.js renders, then its componentDidUpdate is fires, then after that Cube.jss debugging starts. This appears to be out of order compared to what I would expect from the component lifecycle. As far as I understand it a parent's componentDidUpdate doesn't fire unless all of this children have finished rendering.

AndrewRayCode avatar Jun 25 '16 01:06 AndrewRayCode

Maybe this is related?

https://github.com/toxicFork/react-three-renderer/blob/staging/2.1/src/lib/React3.js#L71-L73

  componentDidUpdate() {
    this._render();
  }

Child components aren't rendered until after the parent component has updated?

AndrewRayCode avatar Jun 25 '16 01:06 AndrewRayCode

Thanks for the report :)

For that component yes, I can make that render happen earlier, I will now be looking at the provided test as well to see if it is the same issue or do I need to fix something further in the internals

toxicFork avatar Jun 26 '16 05:06 toxicFork

@DelvarWorld does this happen for components deeper inside <React3> as well, or only for the root?

toxicFork avatar Jun 26 '16 07:06 toxicFork

The reason for doing the internal rendering within componentDidUpdate is that updating some properties in the <canvas/> clears it. For example if you rendered something into a canvas and then change its width, it will be blank.

When the canvas element is created, and subsequently whenever the width and height attributes are set (whether to a new value or to the previous value), the bitmap and any associated contexts must be cleared back to their initial state and reinitialized with the newly specified coordinate space dimensions.

So we need to render after react-dom action is complete for at least the canvas.

Technical notes

Potential fix thoughts: render a <div/> for React3#render and within componentWillUpdate:

  1. if it's possible to nest react-dom render calls:
    • use react-dom to manually render the canvas into the div and then immediately render into the canvas before componentWillUpdate even finishes
  2. otherwise:
    • create a new internal <canvas/> component for react-three-renderer to render

I will try these in order.

toxicFork avatar Jun 26 '16 07:06 toxicFork

Alright, option 1 works!

toxicFork avatar Jun 29 '16 16:06 toxicFork

Linking http://stackoverflow.com/questions/39626921/react-three-renderer-refs-not-current-in-componentdidupdate-mvce-included

toxicFork avatar Sep 27 '16 20:09 toxicFork

Converting to bug as well to increase priority

toxicFork avatar Sep 27 '16 21:09 toxicFork