firtoz / react-three-renderer

Render into a three.js canvas using React.
https://toxicfork.github.com/react-three-renderer-example/
MIT License
1.49k stars 155 forks source link

r3r appears to fire react lifecycle methods out of expected order #84

Open AndrewRayCode opened 8 years ago

AndrewRayCode commented 8 years ago

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 commented 8 years ago

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?

toxicFork commented 8 years ago

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 commented 8 years ago

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

toxicFork commented 8 years ago

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 commented 8 years ago

Alright, option 1 works!

toxicFork commented 7 years ago

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

toxicFork commented 7 years ago

Converting to bug as well to increase priority