remix-run / react-router

Declarative routing for React
https://reactrouter.com
MIT License
53.11k stars 10.31k forks source link

Add example for preloading data before navigate #4407

Closed lanwin closed 7 years ago

lanwin commented 7 years ago

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.

goshacmd commented 7 years ago

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

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

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

goshacmd commented 7 years ago

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

What about starting your data load in componentWillMount()?

pshrmn commented 7 years ago

@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.

goshacmd commented 7 years ago

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

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

goshacmd commented 7 years ago

@mjackson sure!

goshacmd commented 7 years ago

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

@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?

goshacmd commented 7 years ago

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

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

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>.

goshacmd commented 7 years ago

@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?

Velenir commented 7 years ago

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

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.

mjackson commented 7 years ago

@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).

goshacmd commented 7 years ago

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

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

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

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

@ryanflorence Very good summary!

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

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

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

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

try https://github.com/dlmr/react-router-redial for preloading data

timdorr commented 7 years ago

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

jguddas commented 7 years ago

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)
hampusohlsson commented 7 years ago

Another approach to this problem that is becoming more and more widespread, is to render interface previews or skeleton screens while waiting for the data. The Facebook newsfeed is probably the best example of this.

With that approach you would be able to use react-router as is, dispatch data requests in the component lifecycle methods, and just render preview screens while waiting for the data.

This can be especially useful if you're loading a page that has to make multiple API calls. You would experience a progressive enhancement of the page as requests completes, instead of having to wait for all requests before displaying anything.

So basically; old screen -> interface previews -> filled in screen

ryanflorence commented 7 years ago

Yep, that's also how lots of native apps work too

windsome commented 7 years ago

I struggled with a similar problem, when using with dynamic routing. we use require.ensure() to load component async, and webpack packs the component js to webpackJsonp. when browser fetching component files, it still shows old component until new component receive from network. if this time interval is long, user feels bad.

I made a simple loading reducer, then dispatch pageLoad* actions in route files. then pages will listen to the pageLoading to determine whether show loading indicator.

loadingReducer.js:

import { createActions } from 'redux-actions';
export const { pageLoadRequest, pageLoadSuccess, pageLoadFailure } = createActions('PAGE_LOAD_REQUEST', 'PAGE_LOAD_SUCCESS', 'PAGE_LOAD_FAILURE');
const ACTION_HANDLERS = {
    ['PAGE_LOAD_REQUEST']: (state, action) => ({ ...state, pageLoading: true }),
    ['PAGE_LOAD_SUCCESS']: (state, action) => ({ ...state, pageLoading: false }),
    ['PAGE_LOAD_FAILURE']: (state, action) => ({ ...state, pageLoading: false }),
}
const initialState = { pageLoading:false }
export default function Reducer(state = initialState, action) {
    const handler = ACTION_HANDLERS[action.type]
    return handler ? handler(state, action) : state
}

routes.js:

        {
            path: 'stations',
            getComponent(nextState, cb) {
                store.dispatch (pageLoadRequest());
                require.ensure([], (require) => {
                    store.dispatch (pageLoadSuccess());
                    cb(null, require('./containers/LayoutMainContainer').default)
                })
            },
            indexRoute: {
                getComponent(nextState, cb) {
                    store.dispatch (pageLoadRequest());
                    require.ensure([], (require) => {
                        store.dispatch (pageLoadSuccess());
                        const reducer = require('../modules/xxedata').default;
                        injectReducer(store, { key:'xxedata', reducer:reducer })
                        cb(null, require('./containers/StationListContainer').default)
                    })
                }
            },
            childRoutes: [
                {
                    path: 'map',
                    getComponent(nextState, cb) {
                        store.dispatch (pageLoadRequest());
                        require.ensure([], (require) => {
                            store.dispatch (pageLoadSuccess());
                            cb(null, require('./components/StationMap').default)
                        })
                    }
                },
                {
                    path: 'new',
                    getComponent(nextState, cb) {
                        store.dispatch (pageLoadRequest());
                        require.ensure([], (require) => {
                            store.dispatch (pageLoadSuccess());
                            cb(null, require('./components/Station').default)
                        })
                    }
                },
            ]
        },

then components (mostly, the outmost wrapper component)

import React, { Component, PropTypes } from 'react'
import classes from './styles.scss'
import classNames from 'classnames';

const Loading = () => (
    <div className={classes.progressing}>
        <span className="glyphicon glyphicon-refresh glyphicon-refresh-animate"></span>
    </div>
)

class LayoutMain extends Component {
    static propTypes = {
        children: PropTypes.element.isRequired,
        loading: PropTypes.object.isRequired,
    }
    render () {
        const {children, loading} = this.props;
        return (
          <div className={classes.layoutMainContent}>
            {
                loading.pageLoading && 
                <Loading />
            }
            {children}
            {/* other things... */}
         </div>
        )
    }
}

const mapStateToProps = (state) => ({
    loading: state.loading,
})
export default connect(mapStateToProps, null)(LayoutMain)

Is this the right way ???

ryanflorence commented 7 years ago

closing, we know people want suggestions on how to do this, and we've got plans to make a big fatty demo of the different approaches. I'll make sure to come back here and post it when its done.

davincho commented 7 years ago

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

jtrein commented 7 years ago

Any progress on the demo? :-) Just checking in, as I am starting "#4407-ish" work today.

lpillonel commented 7 years ago

I have a working demo / starter kit here with react-router 4 and route preloading with progressBar. There is also redux, material-ui, etc... but it's not the point here.

ro-savage commented 7 years ago

@ryanflorence - Any update on the examples/demo ?

oleggromov commented 7 years ago

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

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

For the record, this worked great.

cloutiertyler commented 7 years ago

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

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

edgarberm commented 7 years ago

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

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

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 :)

kellyrmilligan commented 6 years ago

@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 commented 6 years ago

@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 commented 6 years ago

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

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

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

LastDreamer commented 6 years ago

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)

mwilc0x commented 6 years ago

Will this be solved with suspense?

timdorr commented 6 years ago

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