[v4] Breaking Changes

[rizedr]

The same idea was reflected in #4697, but that part was ignored, therefore opening a new issue to specifically address that.

I can see that v4 is coming up with the pre-releases from the past days to which I can only say congrats 👍

However, I cannot see anything about the very useful hooks that were available in v3:

• onEnter
• onChange
• onLeave

Furthermore, I do not see anything to reflect the above decision in the change log, nor cannot find a roadmap that describes this methods are deprecated. Should I expect v4 will break all my code?

I find this hooks very useful, for example, I was using the onEnter hook to validate if the user is authenticated in a very elegant way compared to the way is illustrated in the v4 docs. In my opinion, my solution works better because I need the user to be authenticated on all pages except /login.

Another thing I cannot find with v4 is Configuration with Plain Routes, where, why did that go?

Maybe let's not close this right away and use this forum to debate as this package is essential in my projects and, hopefully, this will end in a clear roadmap for all of us.

[MrXyfir]

On the topic of breaking changes: what happened to the location.query object? I see that the query string itself is there as location.search, but not the parsed object. Am I missing something or was this completely removed from v4? Seems an important feature that wouldn't make sense to remove...

[tylermcginnis]

@MrXyfir You can read about that decision here

[timdorr]

v4 is a complete rewrite. As such, there is no singular breaking change. We have some similar-looking things (<Route path="/foo" component={Foo} />), but the behaviors are completely different. You should expect none of your existing react-router usage to work under 4.0.

As for the on* hooks, they were removed because they already exist in React as the lifecycle methods. We were reimplementing them inside the Router, but they didn't exactly behave like they should. So, why provide an inferior or conflicting API? Everything is way more React-y now, and it's way better as a result. You no longer have to mix paradigms or code for two different systems. It's just React now. It's a much lower cognitive load.

And for "plain routes", we've extracted that to a separate package: https://github.com/ReactTraining/react-router/tree/master/packages/react-router-config You can see the reasoning for this here: #4410 (comment)

[johnomalley]

@timdorr - re: "they were removed because they already exist in React as the lifecycle methods"

What in the world are you talking about? Are we supposed to subclass Route and override componentDidMount and componentWillReceiveProps or something? It sure doesn't seem like there's an easy way to implement an onEnter or onLeave hook in V4.

[pshrmn]

@johnomalley The lifecycle methods get placed on the component rendered by the <Route>.

[johnomalley]

Yeah, did a dry run of converting v3 -> v4 and it's going to be super-painful on my project. The nice thing about the onEnter/onLeave handlers is that I would write my containers in my redux app so that they were only dependent upon the store state. Now I have to extract the path variables (e.g. something like itemId in /items/42) from the 'match' prop and update the store that way. On top of the lifecycle methods require that I can no longer use stateless functional components and need to implement React lifecycle methods just to make the router integration work. I think I'm staying with v3 for the foreseeable future. The v4 code is just a lot uglier.

[arempe93]

@timdorr how do you prevent a route change now then? onEnter isnt the same as componentWillMount because onEnter was called before the mounting process began as far as I know.

This seems to be a large loss in functionality as far as the replace and callback pairing goes that you cant replicate with componentWillMount.

[Johnius]

@arempe93 I bet now we have to use component Prompt https://github.com/ReactTraining/react-router/blob/master/packages/react-router/modules/Prompt.js

Which is in my opinion much better than calling setRouteLeaveHook when only a prompt needed

[arempe93]

@Johnius that seems helpful for prompts, but not for what I had in mind. A pattern that I use, and I think is common, is to wrap my routes that require login in a Route, such as:

export default function createRoutes(store) {
  return (
    <Route component={App} path='/'>
      <Route component={Login} path='/login' />
      <Route component={Authenticated} onEnter={Authenticated.enter(store)}>
        <IndexRoute component={Dashboard} />
        ...
      </Route>
    </Route>
  )
}

And the Authenticated.enter(store) function might return something like:

(nextState, replace, callback) => {
  if (<logged in>) {
    if (<! store has user data>) {
      store.dispatch(fetchCurrentUser())
        .then(() => callback())
    }
  } else {
    replace('/login')
    callback()
  }
}

I assume that v4 expects me to have instead have a wrapper component in the Authenticated component's render() that implements something similar in its componentWillMount. But that still leaves me wondering about the functionality of replace and callback

Edit

Looking over the docs more - it seems like replace can be replaced with <Redirect /> and composing the Route component. Not sure if Route composition can be used to the same effect for callback (ie. performing an action before the page rendering starts to change), I'll have to experiment with it.

What I would recommend to the maintainers is to elaborate more on the on* section of the migration doc. It's a bit sparse and could use examples of how to handle replace and callback

[jfconde]

We're trying to sync breadcrumbs with the router in our project and not having an event fired or a callback in the components is very ugly. There is no way we can implement this "generic" hooks in each and every single component rendered by a route. Maybe we should try v3

[rizedr]

@jfconde to avoid making your components "ugly" you can create a withTrailName high order component, that's how I handled that with v4

[jfconde]

@rizedr Yes! Thank you! In the end I solved this using a HoC as you suggest. My HoC might be a bit of an anti-pattern, since it makes all "routed components" connected to Redux. Basically, it allows each of the routed components to have a defined title, and each time one of them is mounted or unmounted, a PUSH/POP_BREADCRUMB action is automatically dispatched (we could have passed props to these components, but then IDK if we could have used the clean <Route component={X}...> syntax).

Each routed component also gets a unique breadcrumbId, in order for a "dynamic title" to be managed (ie: you're loading a product with id: X called "Chair" in a component with title "Product detail" and in the breadcrumbs you want to show "Chair", but you will not have this info until the serverresponds but since the breadcrumb has an id we can replace "Product detail" with "Chair" using such id).

I think it looks a little dirty (dispatching from a HoC 👎 ), but for the time being (we don't have much time to refactor until our first deadline) it works, and breadcrumbs are automatically managed (we just use a dumb component to read the breadcrumbs array from the redux state and convert ['Home', 'Section', 'Sub-section'] into buttons like Home > Section > Subsection).

Here's the "BreadcrumbManager" code looks like: https://pastebin.com/0sv9tYzK
PS: withBreadcrumb would be the initial HoC and withSmartBreadcrumb would connect it to Redux.

[rizedr]

@jfconde Looks pretty good IMO - I don't see any issue in having all route components connected to redux.

You can fix the "dirty" part by using the connect directly in withBreadcrumb and exporting that afterwards:

function withBreadcrumb(WrappedComponent, breadcrumbTitle = '') {
  class AppRoute extends React.Component { ... }
  return connect(mapState, mapDispatch)(AppRoute);
}

[arthurhoch]

The simple answer.

import React from 'react';

import {
  Route,
  Redirect
} from 'react-router-dom';

const RouterCreate = (props) => {
  return (
    (props.onEnter !== undefined ?
      (props.onEnter() ?
        <Route {...props} />
        :
        <Redirect to="/login" />
      )
      :
      <Route {...props} />
    )
  );
}

export default RouterCreate;

Usage:

<BrowserRouter>
    <div>
        <Navbar />
        <Switch>
            <RouterCreate exact path="/" component={HomePage} onEnter={isLoggedIn} />
            <RouterCreate exact path="/login" component={() => isLoggedIn() ? <Redirect to="/" /> : <Login />} />
            <RouterCreate exact path="/protectedroute" component={ProtectedRoute} onEnter={isLoggedIn} />
            <RouterCreate exact path="/currency" component={Currency} onEnter={isLoggedIn} />
            <RouterCreate exact path="/logout" component={Logout} onEnter={isLoggedIn} />
            <RouterCreate exact path="/about" component={About} onEnter={isLoggedIn} />

            <Route component={() => <Redirect to="/" />} />
        </Switch>
    </div>
</BrowserRouter>

[agileago]

@arthurhoch you don't consider onEnter is async. onEnter should have a callback(next) to tirgger router change