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

Add example for preloading data before navigate #4407

Closed
lanwin opened this issue Jan 31, 2017 · 61 comments

Comments

@lanwin
Copy link

commented Jan 31, 2017

While playing with react router (even earlier versions) I struggled mostly with the problem how I could preload my data before the navigation is finished (both for initial request and navigation request).

I am sure I am not the only one with that problem. So I would suggest it could be a great addition for the (quiet good) examples on the new v4 page.

@goshakkk

This comment has been minimized.

Copy link

commented Jan 31, 2017

The way I'd do it is by making a custom Link component which would handle the click event by starting data fetching (and maybe displaying a spinner next to the link or something — so that the user receives immediate feedback and doesn't think the click wasn't handled), and preventing the navigation until the data is loaded.

Then, after the data is fetched, the custom link component would then trigger the navigation.

Fwiw, you can wrap Link and pass onClick (https://github.com/ReactTraining/react-router/blob/v4/packages/react-router-dom/modules/Link.js#L32) with preventDefault() to stop the navigation. You'd then get the router from context (or withRouter) and do router.push yourself.

It is not perfect, of course, as it shifts this kind of logic to the individual link, but there doesn't appear to be any other place to prevent navigation.

@lanwin

This comment has been minimized.

Copy link
Author

commented Jan 31, 2017

From my understanding it should be something like a mixture of Route and Prompt. Something like a

<RouteHook path="/news" beforeNavigate={(match,callback)=>{...}}>

This way it could be decoratively declared like routes and prompt. But I am not that deep into the new stuff for now.

@lanwin

This comment has been minimized.

Copy link
Author

commented Jan 31, 2017

But this is why I think an official example could be very helpful.

@goshakkk

This comment has been minimized.

Copy link

commented Jan 31, 2017

There used to be route hooks like that before v4, and it did in fact have a callback to block the transition, but the RR team found out that they were basically re-implementing what React already has: lifecycle hooks. In fact, there's a talk by Ryan and Michael on that exactly: https://www.youtube.com/watch?v=Vur2dAFZ4GE

One side-effect of that simplification is that blocking a transition now has to be done someplace else (Link). Route is simply conditionally rendering a component, it can't prevent a transition anymore.

@wolfadex

This comment has been minimized.

Copy link

commented Jan 31, 2017

What about starting your data load in componentWillMount()?

@pshrmn

This comment has been minimized.

Copy link
Collaborator

commented Jan 31, 2017

@goshakkk basically described exactly what I would do.

@wolfadex What you suggest is what I imagine most people will do, but being able to load data before the navigation allows you to do things like the loading topbar that github uses.

@goshakkk

This comment has been minimized.

Copy link

commented Jan 31, 2017

@wolfadex componentWillMount is ok if you want to load after changing the route (and hide the previous route), but the OP's question was about loading data before transitioning.

@mjackson

This comment has been minimized.

Copy link
Member

commented Jan 31, 2017

@goshakkk Thank you for responding here! Care to work up an example of what you're describing so future generations can share your vision? :)

@goshakkk

This comment has been minimized.

Copy link

commented Jan 31, 2017

@mjackson sure!

@goshakkk

This comment has been minimized.

Copy link

commented Jan 31, 2017

So I just threw a quick example together: https://rr4-preload-szmnvdgody.now.sh

The code: https://gist.github.com/goshakkk/a9df8238f43e4ad71f6ae4034482d509

I'm not super-happy about pushing the fetching into a link, but it's the simplest I can think of.

A different approach might be to keep the fetching inside a route's lifecycle and somehow make it so that the old vdom is still rendered while the fetching is in progress (possibly by making a smarter version of Route that, even when the route no longer matches, would keep its component in the tree while the new route is loading. Not sure how'd each Route will get that loading state of the now-current route though)

@lanwin

This comment has been minimized.

Copy link
Author

commented Feb 1, 2017

@wolfadex componentWillMount requires you to ensure that each view can work without any data. From users perspective, you see a empty view and a second later the data gets into that view and makes it flicker on every navigate. There are ways to show fake data and fit in the right data so that it looks more natural...but that is a lot work which dont allways be makes sens.

@goshakkk I like the simplicity of your solution. But this will not work when initial navigating to the page, witch I would happy when its would be supported, cause of the reasons above.

Wouldnt it be possible to get the router and register getUserConfirmation for that?

@goshakkk

This comment has been minimized.

Copy link

commented Feb 1, 2017

@lanwin initial navigation would be work in my example. I'm passing the load function into the Products route as well:

<Products fetch={this.loadProducts} ... />

class Products extends Component {
  componentDidMount() {
    if (this.props.list == null) {
      this.props.fetch();
    }
  }
}

Just go to https://rr4-preload-szmnvdgody.now.sh/products directly

@lanwin

This comment has been minimized.

Copy link
Author

commented Feb 1, 2017

Ah sorry that was not what I meant. I would like to ensure that I can kick in some process (like fetching) and ensure that the page is only rendered when this process is done at any case. Even when we navigate to that page the first time.

@mjackson

This comment has been minimized.

Copy link
Member

commented Feb 1, 2017

Thanks for the example, @goshakkk! Would you like to include it as part of our official examples in the repo? I think it would go a long way toward showing people how to compose using <Link>.

@goshakkk

This comment has been minimized.

Copy link

commented Feb 1, 2017

@mjackson can do. Actually, I just realized that the way I'm making a custom Link can effectively be made with a alone, since handling the click is overridden anyway.

Looking at the Link component sources right now, I see it also has checks for modifier keys and so on, which my custom component has no way of making use of, which means it will not handle cmd+clicks correctly.

One possible way forward might be to make a custom link that does not build on top of Link, and effectively copies the existing Link code, adding async data fetching to it. Simple, but it would duplicate Link functionality.

Another might be to have Link accept another prop, onNavigate or something, that gets called after the appropriate checks inside Link, changing into something like this instead:

  handleClick = (event) => {
    if (this.props.onClick)
      this.props.onClick(event)

    if (
      !event.defaultPrevented && // onClick prevented default
      event.button === 0 && // ignore right clicks
      !this.props.target && // let browser handle "target=_blank" etc.
      !isModifiedEvent(event) // ignore clicks with modifier keys
    ) {
      // new code — start
      if (this.props.onNavigate)
        this.props.onNavigate(event)

      if (event.defaultPrevented)
        return;
      // new code — end

      event.preventDefault()

      const { router } = this.context
      const { replace, to } = this.props

      if (replace) {
        router.replace(to)
      } else {
        router.push(to)
      }
    }
  }

https://github.com/ReactTraining/react-router/blob/v4/packages/react-router-dom/modules/Link.js#L41

Do you have any thoughts regarding this?

@timdorr timdorr added the question label Feb 1, 2017

@Velenir

This comment has been minimized.

Copy link

commented Feb 2, 2017

Excellent work on preloading example, but I see a couple problems:

  • Late transition -- if preloading promise resolves after the user has grown tired of waiting and clicked onto a different route. Then this late promise forcibly pushes to history and transitions to that route contrary to user's expectations.
  • Always retriggering preload -- for a use case where we need not request data each time (if for example, once loaded, though stale by this point, data would suffice), it could be a waste to wait on preload to complete.

I've put together an example of how I would solve that: https://rr4-async-transition.surge.sh
Relevant code here.

Please give it a look.

@timdorr timdorr changed the title v4 add example for preloading data before navigate Add example for preloading data before navigate Feb 2, 2017

@strues strues referenced this issue Feb 2, 2017

Closed

v13 approaches #345

@mjackson

This comment has been minimized.

Copy link
Member

commented Feb 3, 2017

@goshakkk Interesting... I'm wondering if we could formalize this pattern a bit. What if onNavigate received both the event object and a callback to call when it's done working. That way, people who need to do some async work in between when the link is clicked and when they need to actually perform the navigation won't have to do it themselves.

const DataLink = ({ action, ...props }) => (
  <Link {...props} onNavigate={(event, callback) => {
    fetchData(event, action).then(data => {
      storeTheDataSomewhere(data)
      callback()
    }, error => {
      callback(error)
    })
  }}/>
)

Probably still needs more refining, but if we take care of actually executing the transition then we have the opportunity to do what @Velenir's example does and actually prevent the navigation in cases where the user gets tired of waiting and clicks another link (or e.g. the back button).

@goshakkk

This comment has been minimized.

Copy link

commented Feb 3, 2017

@mjackson this is a great idea.

Implementation-wise, tracking other transitions while waiting like @Velenir might not be the way forward... because transitions can happen outside Link. What I think might be a way around that is having the link listen to route changes, and if a change occurs while state.loading = true, abort this link's transition. Dunno if it's optimal to do it like that. Thoughts?

@Velenir

This comment has been minimized.

Copy link

commented Feb 4, 2017

@goshakkk, you are absolutely correct. My implementation doesn't account for history.back/forward and probably some other cases. I agree that it is better to keep the decision of going forward or not closer to the metal -- that is in the Link itself.

For example, setting a blockAsyncTransition flag right after any history change.
I gave it another go. Please, have a look. This would allow blocking async on history.back/forward.

@manspaniel

This comment has been minimized.

Copy link

commented Feb 5, 2017

Hmm, but wouldn't this mean you now have to set an onNavigation prop on every link to a certain page? Sure, you could store the fetcher functions in a separate file, or create a component which wraps <Link onNavigation={getCoolStuff}>, but it feels a bit roundabout.

Surely the data fetching function should be attached to the <Route /> somehow? That way the route is pulling the data it needs, and passing it to render/component/children as props. Maybe this would only work for always-rendered/non-nested <Switch/Route> components though, since the Route would need to be mounted both before and after the URL transition.

I'm not familiar enough with React Router yet to know if this is possible with the current API.

@ryanflorence

This comment has been minimized.

Copy link
Contributor

commented Feb 20, 2017

I've got a note to make a demo of this stuff. There are two major things here:

  1. "initial data" - basically needed for server rendering, you want all the initial request data loaded up before render, and then "slurped up" on the client.

  2. "Pending Nav" - this is when an already rendered app pauses to wait for the data for the next page before transitioning. The motivation here is to avoid flickery apps. If your data requests are snappy, you'll get old screen -> flash of empty screen -> filled in screen. By loading the data first, you have a much better transition old screen -> new screen. It's kind of how browsers work without any fancy javascript*.

Both of these require similar touch points with the router and I'll eventually have a demo app that does both (along with server rendering). Also, there are two approaches for Pending Nav: data deps are on routes (or route components) or data deps are on Links. There are subjective trade-offs to both approaches that turn it into a product question so the demo will showcase both.

(*) a browser usually stays on the current page while it negotiates a request with the next page, but if it takes long enough you often end up with an empty white screen anyway.

@Ephem

This comment has been minimized.

Copy link

commented Feb 21, 2017

@ryanflorence Very good summary!

I'd argue that for initial fetching there are also a couple of different considerations:

  • Data dependencies can either be declared on routes or they can be handled separately (something like a separate url->data map outside of react-router).
  • If data dependencies are declared on routes, the only practical way I see of solving that is to have routes declared outside of render so that they can be matched beforehand.
    • One solution to this is using a centralised route-config, like react-router-addons-routes.
    • Another way would be to use something like a HOC to wrap the components that contain routes, something like:
const Comp = ({ routes }) => <div>{renderRoutes(routes)}</div>;
const Wrapped = withRoutes([ { path: '/', component: Child }, ... ])(Comp);

Wrapped.getMatchedComponents('/'); => [Child]
ReactDOM.render(...);

Calling getMatchedComponents(url) on the wrapper would return an array of all matched components recursively and is static so could be used before rendering. This would mean you can co-localize data-dependencies as statics on the component themselves.

I really like how react-router v4 lets you co-localize routes with components, but I also need to prefetch stuff, so I toyed around with a HOC like the above to solve this. An early proof of concept can be found here for reference: https://github.com/Ephem/react-router-wrapper

The best way to get a quick overview of the idea is probably the reimplementation of the react-router v4 basic-example, which can be found here: https://github.com/Ephem/react-router-wrapper/blob/master/examples/basic/basic.js

Just a rough idea, but I wanted to throw it in here. :)

@ryanflorence

This comment has been minimized.

Copy link
Contributor

commented Feb 22, 2017

Just a rough idea, but I wanted to throw it in here. :)

Thanks, and that's exactly why we are not involved in static route configs and prescribing ways to 1) declare data dependencies and 2) actually fetch them.

By being Just Components™ (+ a matchPath) you can make your own route config (whether with HOCs, or an array/tree of routes, or whatever) and use that config to render our components and then use matchPath to match like we do wherever/whenever you want.

There are just too many ways to do this w/o a clear winner.

@Ephem

This comment has been minimized.

Copy link

commented Feb 22, 2017

Yup, and it's a beautiful thing indeed. It was lovely throwing together the POC, I experienced firsthand how easy it is to reason about v4 when everything is Just Components™. :) This is still a pain-point in the migration path for many though so having a demo will be great!

@alexeybondarenko

This comment has been minimized.

Copy link

commented Feb 22, 2017

@timdorr

This comment has been minimized.

Copy link
Collaborator

commented Feb 22, 2017

@alexeybondarenko We're talking React Router 4.0, which that library doesn't support.

@jguddas

This comment has been minimized.

Copy link

commented Feb 23, 2017

A simple link component based on react-router-dom/modules/Link.js where you wrap Line 47-51 in a callback function is the simplest way i can think of doing this.

This for example would delay the route transmission for one second.

setTimeout(() => {
  if (replace) {
    router.replace(to)
  } else {
    router.push(to)
  }
}, 1000)
@oleggromov

This comment has been minimized.

Copy link

commented May 26, 2017

True, I'm also in search for a neat solution to solve a pretty similar issue. I'd like to prevent navigation until all fetching/posting data in the current "screen" finishes. One of the consequences of allowing transitions between "screens" (i.e. routes) before data sync is finished is that the UX becomes ambiguous and the user has no idea whether or not the action he made has been finished correctly before switching to another "screen".

Any suggestions on how to make it without Link modifications would be highly appreciated! Thanks :-)

@gajus

This comment has been minimized.

Copy link

commented May 28, 2017

Probably https://github.com/ctrlplusb/react-async-component would be one possible solution?

For the record, this worked great.

@cloutiertyler

This comment has been minimized.

Copy link

commented May 30, 2017

Just a quick question about this. Let's assume you put the logic in the Link. What happens when the user navigates directly to a webpage?

You'll have to have the fetching logic in at least two places for this to work. Intuitively, it seems like the Route (or the component prop of the Route) is where this should live. That way my components can be just pure render functions. Am I missing something?

@arempe93

This comment has been minimized.

Copy link

commented Jun 9, 2017

@ryanflorence any demo updates? this is the only blocker for v3 => v4, but it's a pretty substantial one

@builtbyedgar

This comment has been minimized.

Copy link

commented Jul 12, 2017

Some time ago I worked on a component that handles data loading for the react-router (v3) routes. I'm sure you will find a very dirty code, but I think the idea is good..

https://github.com/builtbyedgar/async-route-manager/blob/master/src/index.js

@kellyrmilligan

This comment has been minimized.

Copy link
Contributor

commented Aug 27, 2017

For version 2 and now version 4 I have been using the pattern describe in react router config component essentially. I baked it into a simple module that only requires that the data dependencies return a promise.

https://www.npmjs.com/package/react-router-fetch

See the examples in the readme

@Merri

This comment has been minimized.

Copy link

commented Aug 28, 2017

Since I didn't find a similar solution I'll add one that works well if you're managing most of your state in Redux store or other state management solution.

This one is different in that it only preloads data based on specific routes, and as such you don't need to provide components for the routes. Also, I don't see a need for a recursive list, so instead the routes are in a flat array and each of the items is checked against the current pathname. To avoid fetching too often you either have to rely on caching or implement additional logic in Redux action creators.

If you want to show loading indicators, transitions, or do progressive rendering, you can handle that via Redux store's state; and as a reminder the implementation allows use of something else than Redux since preloadLocation is agnostic of the second parameter, only expecting it to be an object.

routes.js

import Promise from 'bluebird'
import matchPath from 'react-router/matchPath'
import Router from 'react-router/Router'

// used for default root match
const { computeMatch } = Router.prototype

const routes = []

export function preloadLocation(location, props) {
    return Promise.all(
        routes.reduce(function(promises, { preload, ...route }) {
            const match = route.path ? matchPath(location.pathname, route) : computeMatch(location.pathname)

            if (match) {
                promises.push(
                    preload({ ...props, location, match, route })
                )
            }

            return promises
        }, [])
    )
}

export function preloadOnRoute(route) {
    (Array.isArray(route) ? route : [route]).forEach(function(route) {
        if (route && typeof route.preload === 'function') {
            routes.push(route)
        }
    })
}

Sample component

function Places({ places }) {
    ...
}

Places.propTypes = {
    places: PropTypes.array,
}

import { connect } from 'react-redux'
// getPlaces returns a Promise (redux-thunk is also in use)
import { getPlaces } from '../../../store/actions/places'
import { preloadOnRoute } from '../../lib/routes'

function mapStateToProps(state) {
    return { places: state.places }
}

preloadOnRoute({
    path: '/places',
    exact: true,
    preload: ({ dispatch }) => dispatch(getPlaces()),
})

export default connect(mapStateToProps)(Places)

Sample root component (handles loading on client side)

import { preloadLocation } from './lib/routes'

class App extends React.Component {
    constructor(props) {
        super(props)
    }

    componentWillReceiveProps(nextProps) {
        if (nextProps.location !== this.props.location) {
            preloadLocation(nextProps.location, this.context.store)
        }
    }

    render() {
        return (
            ... your regular React Router v4 stuff here ...
        )
    }
}

App.contextTypes = {
    store: PropTypes.object,
}

App.propTypes = {
    location: PropTypes.object,
}

export default withRouter(App)

Sample hapi.js server render

import { createMemoryHistory } from 'history'

const history = createMemoryHistory()
            
// initialState here is for handling cases like rendering form errors server side
// (useful if you make an app that can also work without client side JS)
export function renderReactWithState(request, reply, initialState) {
    const location = history.createLocation(request.url.href)
    const store = makeStore(initialState)

    preloadLocation(location, store).then(function() {
        const state = store.getState()
        // renders and sends HTML with initial state
        renderReply(
            { reply, state },
            <Provider store={store}>
                <Router context={{}} location={location}>
                    <App />
                </Router>
            </Provider>
        )
    })
}

You could also just have a separate file with all the preloaders as you can simply do:

preloadOnRoute([{
    path: '/people/:id',
    exact: true,
    preload: ({ dispatch, match }) => dispatch(getPerson(match.params.id)),
}, {
    path: '/places',
    exact: true,
    preload: ({ dispatch }) => dispatch(getPlaces()),
}])

Although this is troublesome as when components are removed you would have to separately remember to remove the preloader.

Hopefully someone else is looking for a similar solution :)

@ReactTraining ReactTraining deleted a comment from kelp404 Nov 23, 2017

@ReactTraining ReactTraining deleted a comment from crazyyi Nov 23, 2017

@kellyrmilligan

This comment has been minimized.

Copy link
Contributor

commented Nov 23, 2017

@Merri, react-router-fetch essentially does what you’re suggesting, and let’s you write a static method co located with the route handler. I’ve also implemented hapi-react-redux, which uses react router fetch under the hood to do the same thing, just at a different point in the request lifecycle

@Merri

This comment has been minimized.

Copy link

commented Nov 23, 2017

@kellyrmilligan react-router-config used by react-router-fetch forces into re-declaring a nested tree of routes with the components, which is something I don't like as an idea, because it seems like something that would prevent going for loading the app in smaller pieces (which is something that I might be wrong about). Another thing is that - if I recall correctly - the matching logic in react-router-config prevents some use cases for matching routes.

This is why I wrote my solution to be a flat array (instead of nested) and each route is matched and checked against. To me this seems easier to reason about. Also, there is nothing that actually binds a preloader to a component, which leaves open the question where to put the preloaders, but there is less forced abstraction. In the main project I work with I've found nice possibilities for code re-use in the flat and independent implementation, but I can't compare to using react-router-config since I never tested it in production-like codebase.

@kellyrmilligan

This comment has been minimized.

Copy link
Contributor

commented Nov 24, 2017

You can still use something like react-loadable from what I understand. In practice I find it convenient to have a central route definition using react router config, and having the static method defined on the rout handler seems more convenient to me in terms of co-location. To each their own :)

@chbdetta

This comment has been minimized.

Copy link

commented Nov 29, 2017

I don't think this is possible with v4 and they know it. The react lifecycle approach is clean but lack of some essential hooks.

@kellyrmilligan

This comment has been minimized.

Copy link
Contributor

commented Nov 29, 2017

It is totally possible. I’m doing it in production currently

@LastDreamer

This comment has been minimized.

Copy link

commented Feb 6, 2018

I've wrote react-router-prefetch
I'm not sure about SSR and Code splitting, because not use it now, but issue reports are welcome)

@mjw56

This comment has been minimized.

Copy link
Contributor

commented Mar 4, 2018

Will this be solved with suspense?

@timdorr

This comment has been minimized.

Copy link
Collaborator

commented Mar 5, 2018

Not really. That just provides one possible pattern for doing so.

@rayronvictor

This comment has been minimized.

Copy link

commented Apr 1, 2018

Best way is fetch data in componentDidMount and show a loading state while fetch isn't finished.
With this approach you can navigate to this route by putting url directly.

class TargetRouteComponent extends Component {
  state = {
    data: null
  }

  componentDidMount () {
    this.props.fetchData().then((res) => {
      this.setState({data: res})
    }
  }

  render () {
    const { data } = this.state

    if( !data ) {
      return <LoadingComponent />
    }

    return (
      <ComponentUsingData />
    )
  }
}

Using recompose it's even better:

Loader.js

import { branch, renderComponent } from 'recompose'

export const withLoadingTest = (test) => branch(
  test,
  renderComponent(() => <Loader active />)
)

export const withLoading = withLoadingTest(({ loading }) => loading)

TargetRouteComponent.js

const TargetRouteComponent = ({ data }) => (
  <h1>{data}</h1>
)

const enhance = withLoadingTest(({ data }) => !data)

export default enhance(TargetRouteComponent)

Alternative TargetRouteComponent.js

export const TargetRouteComponent = withLoadingTest(({ data }) => !data)(
({ data }) => (
  <h1>{data}</h1>
))

Sorry for my bad english!

@raunaqss

This comment has been minimized.

Copy link

commented Apr 12, 2018

@cloutiertyler your answer / suggestion seemed obvious to me as well. Did you trying doing so (using the render method for all your routes)?

If yes, have you found any potential downsides?

@BenMagyar

This comment has been minimized.

Copy link

commented May 9, 2018

I ran into this problem as well and somewhat forked after.js to solve it (found here: later.js).

Basically I did four things:

  • Have some resolveRoute function that takes the loadData attributes on the route and converts them to promises.
  • Have a component that wraps all routes and will manage changes between the routes that go unhandled (fetching data/components).
  • Have another HoC that allows for components (Links, etc.) to fetch data and show progress indicators on their own.
  • Have some context that is provided to prevent calls that the HoC handles to be re-fetched by the outer wrapped component.

If you don't use the wrapper component and just attempt to use the HoC method, it's possible for some weird unhandled back behavior/pushed routes that will cause it to be come out of sync.

@taviroquai

This comment has been minimized.

Copy link

commented Jun 6, 2018

I don't think that actions to be run after route is matched and before the component is going to be instantiated, should go on componentDidMount. The old events onEnter makes more sense to me.

I created a wrapper to run sync/async code before the component will be instantiated, and it works going directly to the URL.

NOTE: for the reasons that @Merri pointed out, don't use this.

import React, { Component } from 'react';

class Loader extends Component {
  state = { status: 'loading' }
  async onLoad() {
    let result = await this.props.onLoad()
    this.setState({ status: result ? 'loaded' : 'error' });
  }
  render() {
    if (this.state.status === 'loading') {
      this.onLoad();
      return <div>Loading...</div>
    }
    if (this.state.status === 'error') {
      return <div>Error...</div>
    }
    const { history, location, match, Target } = this.props;
    return <Target history={history} location={location} match={match} />;
  }
}

export default Loader;

And used like this (one liner):

<Route path="/" render={(props) => <Loader {...props} Target={Home} onLoad={Fetch} />} />

If there is nothing wrong with this approach, I don't see a reason why this is not supported by React Router with the old onEnter:

<Route path="/" component={Home} onEnter={Fetch} />

Cheers

@Merri

This comment has been minimized.

Copy link

commented Jun 6, 2018

Render's only concern should be rendering. Doing side-effects there is quite evil.

The above code bugs on client if there would be something that would cause render multiple times, thus causing onLoad to be called multiple times as well. You could of course prevent this by having additional state management and see if already loading, but doing that would be working around a problem, not solving a problem.

Another problem with the above is that it triggers loading on server side as well, but the result will never be used for anything. The solution works only on client side.

To solve these issues one should use a lifecycle method that is run client-side but not server-side... and we're back to componentDidMount.

@taviroquai

This comment has been minimized.

Copy link

commented Jun 6, 2018

@Merri thanks for the explanation. I did not thought about server-side (as I did not use it yet). I left a note in bold in the previous comment.
Just a question: what can cause multiple renders thus multiple onLoad calls?

@Merri

This comment has been minimized.

Copy link

commented Jun 6, 2018

Basically anything having a need to re-render in the tree above. Say, you have a top-level component that reacts to user scrolling the page down, making a header element that stays on top to a certain point. This can then cause a waterfall effect where a lot of inner components are re-rendered as well. This can be optimized out of course (with PureComponent for example), but it doesn't take away the fact that the render may be called multiple times while this.state.status === 'loading' holds true.

@taviroquai

This comment has been minimized.

Copy link

commented Jun 6, 2018

Thanks @Merri. I ended up creating a better solution based on the official React docs:
https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html#fetching-external-data-when-props-change

@jun-thong

This comment has been minimized.

Copy link

commented Jun 12, 2018

Tried to solve this problem too, and wasn't really enjoying using a custom Link component.
The following is a basic sample, without caching, transition, loadingbar, or even error handling, just reproducing the browser behavior.

This sample use react 16.3 getDerivatedStateFromProps() new lifecycle method. Once the local state is prepared, i use shouldComponentUpdate() to check if the prefetcher should update the route or not. Quite simple in fact.

App component

export default class AppContainer extends React.Component {
    render(){
        return (
                <StaticRouter context={this.props.context} location={this.props.location} basename="/">
                            <Prefetcher routes={routes} /> // routes contain routes configuration.
                </StaticRouter>
        );
    }
}

Prefetcher component

@withRouter
export default class Prefetcher extends React.Component {
    constructor(props) {
        super(props);
        this.state = {
            location: this.props.location,
            nextLocation: null,
            needPrefetch: false,
            data: null
        };
    }

    static getDerivedStateFromProps(nextProps, prevState) {
        const navigated = nextProps.location.key !== prevState.location.key;
        let nextState = Object.assign({}, prevState);

        if (navigated){
            nextState.nextLocation = nextProps.location;
            nextState.needPrefetch = true;
        }

        return nextState;
    }

    shouldComponentUpdate(nextProps, nextState){
        if(nextState.needPrefetch) {
            this.getInitialProps(nextState.nextLocation);
            return false;
        }else{
            return true;
        }
    }

    getInitialProps = (nextLocation) => {
        this.props.routes.find((route) => {
            const match = matchPath(nextLocation.pathname, route);
            if(match && route.component && route.component.getInitialProps){
                route.component.getInitialProps().then((data) => {
                    this.setState({
                        location: nextLocation,
                        nextLocation: null,
                        needPrefetch: false,
                        data
                    });
                }).catch((e) => {});
            }
            return match;
        });
    };

    render(){
        const { data } = this.state;
        const initialData = data;

        return (
            <Switch location={this.state.location}>
                {this.props.routes.map((route, i) => (
                    <Route
                        key={`route--${i}`}
                        path={route.path}
                        exact={route.exact}
                        render={props =>
                            React.createElement(route.component, {
                                ...initialData,
                                history: props.history,
                                match: props.match
                            })
                        }
                    />
                ))}
            </Switch>
        );
    }
}

Route component used in route.render() must contain a static method called getInitialProps that return a promise, basically, fetching data.

I'm don't have hardcore knowledges on react-router. It's maybe not the best way to solve this problem. For example, i'm not really sure using location.key is the best way to compare two location, but seems more efficient than a deep comparison to me.

Of course i'm open to any suggestion to improve this piece of code.

@mileshenrichs

This comment has been minimized.

Copy link

commented Jul 8, 2018

@ryanflorence Have you had a chance to make that demo yet?

@Merri

This comment has been minimized.

Copy link

commented Jul 8, 2018

@jun-thong Your code has a running condition with the needPrefetch logic: if getDerivedStateFromProps gets called multiple times while getInitialProps is still waiting for data you may have multiple fetches that may return data in an incorrect order.

So there needs to be an additional logic to ensure only the latest fetch is accounted for and/or that the current location matches with the location the data was loaded for.

@lock lock bot locked as resolved and limited conversation to collaborators Sep 6, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.