ctrlplusb / react-async-bootstrapper

Execute a bootstrap method on your React/Preact components. Useful for data prefetching and other activities.
MIT License
117 stars 12 forks source link

The application calls render twice using react-async-bootstrapper #3

Open cloud-walker opened 7 years ago

cloud-walker commented 7 years ago

HI, i'm trying to upgrade my universal application from react-router 3 to 4, and the only almost functioning async component its yours (I've tried react-loadable, but it flickers on first render...).

But I've noticed that calling asyncBootstrapper(content).then(...), trigger the rendering twice (4 times if we consider the ssr).

There is no solution to that problem? I've made something wrong?

My project experiment: https://github.com/cloud-walker/rr4-ssr-code-split-experiment

jamesjjk commented 7 years ago

Indeed it calls a second rendor.

cloud-walker commented 7 years ago

I dont think its a good thing 😕

threehams commented 7 years ago

Walking the render tree as a collection step is common - react-async-component, react-apollo, loadable-components, and isomorphic-relay all take this approach. In the case of this library, it's capable of doing other work (collecting data fetch queries, for instance) in addition to bundle splitting, just like the Relay clients.

There's a performance tradeoff to this, so you'll have to measure the impact and decide if the combination of walking the tree + rendering to string is worth the cost. It's not double the time, but it's still more than rendering alone.

crazyx13th commented 7 years ago

would be cool to have an intendifier to know the render "phase". At the moment I use a variable

private async asyncBootstrap()
{
    this.asyncBootstrapPhase = true;
    ...
}
public render(): JSX.Element
{
    if (this.asyncBootstrapPhase) return null;
...
}

because I only use async things at my router... ne tree check needed for me. thx!

ctrlplusb commented 6 years ago

Clever little trick @crazyx13th 👍

jnath commented 6 years ago

sorry not working for my, i use code splitting and react-async-component.

jeroenwienk commented 6 years ago

It is never calling componentWillUnmount in my application. Since we are using style-loader with the useable styles it will never call styles.unuse on the Component causing my document head to be filled with unused styles causes all sorts of problems. Is there any way arround this? Or should it call componentWillUnmount?

My solution for now is to do this:

toggleStyleLoader(false);
  asyncBootstrapper(nextApp).then(() => {
    toggleStyleLoader(true);
    hydrate(nextApp, element);
});

The toggleStyleLoader will add some property to the window that my wrapped component checks for.

ctrlplusb commented 6 years ago

@crazyx13th the API has been updated to allow you to inject a custom context that will be available to all your components during bootstrapping. This can be helpful for cases where you want to make the distinction, like in your example.

class Foo extends Component {
  bootstrap() {
     console.log(this.context.asyncBootstrapPhase)
  }

  render() {
    return <div>foo</div>
  }
}

bootstrapper(<Foo />, null, { asyncBootstrapPhase: true })
  .then(() => console.log('done'))

As you can see it's a new third parameter to the bootstrapper.

That being said, I would like to remind you that this is just an implementation of react-tree-walker and therefore obeys all of it's rules. So therefore if you only want to execute the bottom level instances that have a bootstrap method on them, like in your routes, you can simply have your bootstrap functions resolve a false value. This indicates that the bootstrapper should not render the route or attempt to traverse its children. i.e. an early bail out clause much like you are trying to achieve in your example.


class Foo extends Component {
  bootstrap() {
     return fetch('/data')
       .then(doSomethingWithData)
       // resolve a "false" value to stop bootstrapper from traversing this component
       .then(() => false)
  }

You may have noticed I am using `bootstrap` instead of `asyncBootstrap`.  Both work, however, `asyncBootstrap` will be removed in next major release.