odysseyscience / react-router-proxy-loader

Dynamically load react-router components on-demand, based on react-proxy-loader
145 stars 12 forks source link

[Discussion] renderUnavailable is unreachable code #5

Open liouh opened 9 years ago

liouh commented 9 years ago

Say I'm navigating between two react-router routes, A --> B.

Order of execution is roughly as follows:

  1. A's willTransitionFrom
  2. B's willTransitionTo (async load, go to step 3 via callback when complete)
  3. A's componentWillUnmount
  4. B's componentWillMount, render, ...etc

The proxy render logic calls renderUnavailable if the component has not yet loaded. In react-proxy-loader, this works because render can happen before the component is downloaded. However, with the new requirement of waiting for willTransitionTo, the component will always be loaded by the time render is called.

On slower connections, this results in a period of time where the page appears to hang while the new component is downloaded (no visual feedback to confirm user interaction).

Any thoughts on the best way to communicate this loading process?

liouh commented 9 years ago

I tried exposing callbacks in index.js to signal the start and end of loading, and implemented those callbacks at the same place renderUnavailable would be defined:

statics: {
    componentLoading: function() {
        console.log('loading...');
    },
    componentLoaded: function() {
        console.log('loaded');
    }
}

However, the things I could do in them was severely limited because there's no instance available at that point in time.

I ended up ripping out willTransitionTo altogether, and moving all logic from willTransitionTo into componentWillMount, allowing me to use renderAvailable as before. I lose the ability to hook into willTransitionTo, but I think that's a reasonable tradeoff to get the full async loading behavior.

seanadkinson commented 9 years ago

When I wrote this code the first time, before open-sourcing it, I had a dependency on jquery, and was adding a class to the body element, which would show a spinner in the top left corner when the script was loading. I've been meaning to add that back, so I might look at that again soon.

Not sure if the whole renderUnavailable approach is going to work, since you can't render anything in willTransitionTo, and react-router is always going to wait until the callback is called before continuing the transition. Even if it did support rendering something before the script was loaded, it would be a pretty bad experience for nested Routes (you'd call renderUnavailable at each level, as the scripts were loaded).

More thinking needed.