New top level API

[mjackson]

We had a great discussion on gitter this morning that helped to consolidate a few ideas around what we need from the router API going forward. In the shower, I thought about what everyone said and tried to put it all together. Here's what I propose:

var React = require('react');
var Router = require('react-router');

class MyComponent extends React.Component {

  static routes = [
    // ... not sure what this looks like yet
  ]

  static propTypes = {
    location: React.PropTypes.instanceOf(Router.Location).isRequired,
    params: React.PropTypes.object.isRequired,
    query: React.PropTypes.object.isRequired
  }

  shouldComponentUnmount() {
    return confirm('Are you sure you want to leave?'); // return false to cancel
  }

  render() {
    return React.createElement(this.props.childRoute);
  }

}

var MyRouter = Router.create(MyComponent);

// client-side
var HashLocation = require('react-router/lib/HashLocation');
HashLocation.addChangeListener(function (location) {
  React.render(<MyRouter location={location}/>, document.body);
});

// server-side
React.renderToString(<MyRouter location="/the/path"/>);

Benefits of this approach over our current API:

• Both client and server APIs are much more explicit because they use props. We will still use context, but only behind the scenes for stuff like <Link>
• onChange is much more explicit. Quite a few people have been asking about this
• We don't expose our semi-private state API, so people don't rely on it
• The overall build should be slimmer because you only need to require the location that you're actually going to use on the client instead of all of them. HistoryLocation will still require('RefreshLocation') so it can do the auto-fallback thing

Drawbacks:

• We lose async transitions. This is a big break w/ backwards compat, but I don't think I actually mind it that much because:
  • Our current async API is too magical (we call the callback for you if you don't give us one)
  • The way we do async is a point of contention (callbacks vs. promises ... boo)
  • You can still do whatever async stuff you want before you render
• You can't see all your routes defined in one place any more ... or can you? I guess it all depends on what the static routes prop looks like

Obviously there are still some holes, like what the actual route definition looks like and how you add transition hooks, etc. But for a top-level API I really like this approach.

/cc @ryanflorence @gaearon @taurose

Update: Added more propTypes.
Update: Added shouldComponentTransition lifecycle method.
Update: Renamed shouldComponentTransition => shouldComponentUnmount.

[gaearon]

Something to consider:

• Will we even need transition hooks if we kill async? Can we just use lifecycle hooks now?
• How do we expose a way to go over handlers before rendering? For example, to reduce promises.
• We should support “named views” (Question: react-router equivalent to Angular's named view? #908, [Proposal] Multiple route handlers with `<RouteHandler for="..."/> #1008)
• We should probably support async handlers (Get Route Handler Components Asynchronously #755). Can we implement this in such a way that getHandlerForRoute returns one handler while loading, and another “real” handler when it's loaded?

[ryanflorence]

That's the direction I've been headed too. Almost identical ... pretty much exactly identical.

I'll post my code after I think through a few more bits, but regarding transitions, this is something I've been thinking about. Instead of tracking active components and passing them around to transition hooks and people having to figure out how to get their flux/flummox/fluxxor/freekz into transition hooks, we could maybe have users "block" transitions before they even happen, and then unblock them later.

For example, instead of waiting for a transition from a half-filled out form (literally the only use case for onLeave that I can think of), the component can just block transitions whenever it decides the app has entered a state that it doesn't want the user to leave:

var Signup = Router.createHandler(class extends React.Component {
  handleInputChange (event) {
    if (formIsNotEmpty(this.getDOMNode()))
      this.blockTransitions((allowTransition) => {
        if (prompt('Unsaved changes, click okay to leave page'))
          allowTransition();
      });
    else
      this.unblockTransitions();
  },

  render () {
    return (
      <form>
        <input name="address" onChange={this.handleInputChange.bind(this)}/>
      </form>
    );
  }
});

[ryanflorence]

We don't really need onEnter because we have the location subscription and then people can introspect their route branch (and even add their own willTransitionTo)

[ryanflorence]

I hope you're ready for this:

// can still do this for most apps and people who like it
/*
var routes = (
  <Route handler={App}>
    <Route path="/signup" handler={Signup}>
    <Route path="/courses" handler={Courses}/>
    <Route path="/course/:course_id" handler={Course}>
      <Route path="assignments" handler={Assignments}/>
      <Route path="assignments/:assignment" handler={Assignment}/>
      <Route path="/grades/:course_id" handler={Grades}/>
    </Route>
  </Route>
);
*/

// but it all becomes this somewhere

////////////////////////////////////////////////////////////////////////////////
// Routes
var SignupRoute = {
  path: '/signup',
  handlers: {
    main: require('./Signup'),
    sidebar: require('./Signup/Sidebar')
  }
};

var CoursesRoute = {
  path: '/courses',
  handlers: {
    main: require('./Courses'),
    sidebar: require('./Courses/Sidebar')
  },
  // because paths will be gradually matched, need to provide hints because
  // the GradesRoute won't be loaded yet so the matcher won't know that
  // `/grades/econ-101` is a descendant route of `/courses`. Could preprocess
  // this in apps that can't have the whole route config at boot)
  descendentPathMatches: ['/grades/:course_id']
};

var CourseRoute = {
  path: '/course/:course_id',
  handlers: {
    // handlers can be functions that receive a callback, working seemlessly
    // with webpack lazy bundles
    main: require('bundle!lazy?./Course'),
    sidebar: require('bundle!lazy?./Course/Sidebar')
  },
  // bummer it has to go all the way up the tree
  descendentPathMatches: ['/grades/:course_id']
}

var AssignmentsRoute = {
  path: '/course/:course_id/assignments',
  handlers: {
    main: require('bundle!lazy?./Course/Assignments'),
    toolbar: require('bundle!lazy?./Course/Assignments/Toolbar')
  }
};

var AssignmentRoute = {
  path: '/course/:course_id/assignments/:assignment_id',
  handlers: {
    main: require('bundle!lazy?./Course/Assignments/Assignment'),
    toolbar: require('bundle!lazy?./Course/Assignments/Assignment/Toolbar')
  }
};

var Grades = {
  path: '/grades/:course_id',
  handlers: {
    main: require('bundle!lazy?./Course/Grades'),
    toolbar: require('bundle!lazy?./Course/Grades/Toolbar')
  }
};

////////////////////////////////////////////////////////////////////////////////
// Components

var App = Router.createRootHandler(class extends React.Component {
  static routes = [
    SignupRoute,
    CoursesRoute,
    CourseRoute
  ];

  render () {
    return (
      <div>
        <h1>App</h1>
        <Outlet for="sidebar"/>
        <Outlet/>
      </div>
    );
  }
});

class SignupSidebar extends React.Component {
  render () {
    return (
      <div _dangerouslySetInnerHTML={{html: STATIC_CONTENT.SIGNUP_STUFF}}/>;
    );
  }
}

var Signup = Router.createHandler(class extends React.Component {
  handleInputChange (event) {
    if (formIsNotEmpty(this.getDOMNode()))
      this.blockTransitions((allowTransition) => {
        if (prompt('Unsaved changes, click okay to leave page'))
          allowTransition();
      });
    else
      this.unblockTransitions();
  }

  handleSubmit (event) {
    createUser(getFormValues(event.target)).then(() => {
      // transition to route definitions
      this.transitionTo(CoursesRoute);
    });
  }

  render () {
    return (
      <form>
        <input name="name" onChange={this.handleInputChange.bind(this)}/>
        <input name="email" onChange={this.handleInputChange.bind(this)}/>
        <button type="submit" onSubmit={this.handleSubmit.bind(this)}>sign up</button>
      </form>
    );
  }
});

class Courses extends React.Component {}

class CourseSidebar extends React.Component {
  render () {
    return (
      <nav>
        <ul>
          <li><Link to={AssignmentsRoute}>Assignments</Link></li>
          <li><Link to={GradesRoute}>Grades</Link></li>
        </ul>
      </nav>
    );
  }
}

var Course = Router.createHandler(class extends React.Component {

  // the "main" handler defines the nested routes it responds
  // to. We would gradually match paths so that the route config
  // can be loaded lazily along with the components
  static routes = [
    AssignmentsRoute,
    AssignmentRoute,
    GradesRoute,
  ];

  render () {
    return (
      <div>
        {/*
            we inject params/query/routeBranch/router
            assists with getting started quickly (not having to learn about
            context or passing the router state down the hierarchy) and also
            assists with docs/commenting on issues because we have a base
            assumption about how people can access this stuff.
         */}
        <h2>{this.props.params.course_id}</h2>
        <Outlet for="toolbar"/>
        <Outlet/>
      </div>
    );
  }
});

class Assignments extends React.Component {}
class AssignmentsToolbar extends React.Component {}

class Assignment extends React.Component {}
class AssignmentToolbar extends React.Component {}

class Grades extends React.Component {}
class GradesToolbar extends React.Component {}


////////////////////////////////////////////////////////////////////////////////
// running the app

// lets say we land at /course/econ-101/assignment/234
App.run(HistoryLocation, (state) => {
  // still use App.run and not just the location so that we can provide the
  // matched branch of routes in here for data fetching and also so that
  // the router can wait for any async component loading to happen before calling
  // back in here
  state.branch; // [{ route: CourseRoute, handlers: { main: ..., sidebar ...}, ...];
  React.render(<App/>, document.body);
});

[mjackson]

We don't have the route branch in the location subscription in the example that I gave, only the new location. Of course, routers will still need a match method to figure out the branch they need to use, so you may be able to just use that, e.g.

HashLocation.addChangeListener(function (location) {
  var match = MyRouter.match(location);

  // Here you could introspect the routes, but you'll end up doing the same
  // matching work in the call to render unless we possibly have another
  // prop we can use for the pre-computed match object.

  React.render(<MyRouter match={match}/>, document.body);
});

Ultimately it seems this thread is about doing a lot less stuff than we were trying to do previously, which I think is a positive direction to take. We can delegate all async behavior/prop loading to something else.

[ryanflorence]

@mjackson I don't want users to have to match the location and then pass it in, I'd rather handle that for them.

[mjackson]

@ryanflorence The state arg has always felt sloppy to me. I'm thinking about enabling stuff like #757. We can pretty easily serialize a location object (it's really just a URL and possibly a payload from the popstate event. i've got it done locally, just haven't pushed yet), but not so with a big bag of state.

[mjackson]

In other words, I don't like magically putting state onto the App. I'd really like to make it more explicit. We can always use something higher-level to fetch data, etc.

[ryanflorence]

I'm after:

• async component loading
• enough information for people (and us) to build data fetching abstractions
• enough information to make record/replay possible
• eliminate boilerplate that will be literally identical for everybody

[gaearon]

@ryanflorence

That's exactly what I tried except I was missing descendentPathMatches. This forced me to have strict URL nesting, which I didn't like. Nice way to solve it!

I like @mjackson's explicit approach with change handler on location over App.run magic. Mostly because #757; also obvious how to set it up or tear it down. It's also more natural for folks who want router to be somewhere inside the view hierarchy as they can do something like

class RouterEntryPoint extends Component {
  componentDidMount() {
    HashLocation.addChangeListener(this.handleLocationChange);
  }

  componentWillUnmount() {
    HashLocation.removeChangeListener(this.handleLocationChange);
  }

  handleLocationChange(location) {
    this.setState({ match: MyRouter.match(location) });
  }

  render() {
    const { match } = this.state;
    return <MyRouter match={match} />
  }
}

[gaearon]

@ryanflorence

How do you feel about passing outlets as a prop instead of <Outlet> component?

var Course = Router.createHandler(class extends React.Component {
  static routes = [
    AssignmentsRoute,
    AssignmentRoute,
    GradesRoute,
  ];

  render () {
    // Grab outlets from a prop
    let { Toolbar, Main } = this.props.outlets;

    // Parent can show a spinner while async outlet is loading!
    if (!Main) {
      Main = Spinner;
    }

    return (
      <div>
        <h2>{this.props.params.course_id}</h2>
        <Toolbar />
        <Main />
      </div>
    );
  }
});

I see several benefits:

• Easy to show custom UI while outlet is loading;
• People can do if (SomeOutlet === SomeKnownChild) if they want to (I've seen this asked for several times)
• Less magic

[agundermann]

I like where this is going, especially

• explicitly passing state/location as prop
• not enforcing a way to do data fetching
• replacing RouteHandler with props

I'm not sure yet about

• baked-in outlets
• defining routes on handlers
• this.blockTransition

As for using Location directly: I'm not strongly opposed to it, but I'm not agreeing with some of the benefits mentioned here, like it being less magical, or reducing bundle size, which is more of a question of how we provide modules I'd say (single entry file). Also, doesn't addChangeListener imply that there's no initial change event?

I've also recently been thinking about ways to "relax" react-router to enable more use cases and better library integration. Here's what I had in mind:

1. Passing the transition object of today to run/change callback, and providing something like router.finishTransition(transition).

   I guess this would be a compromise between what we have today and what was proposed here (not offering anything). If the router stayed completely out of it, I think there would be a lot of boilerplate code involved to keep the routing in sync with async data fetching (aborting requests, superseding unfinished requests, redirects) and I think the transition object could help there.

   It doesn't have to be managed and provided by the router (or location) instance though; it might also be possible to provide it as an optional utility (e.g. var transition = transitionManager.startTransition(location)) and manually hook it up with the rest in some way.

2. Not enforcing a top level react component to render things. Similar to how different people with different technology stacks require different ways to fetch data, I feel like the same is true for rendering, e.g. only rerendering components when stores/observables change, or even using another view library similar to react (or react-native?).

   // Default option: use the component we provide
   var MainComponent = createRouterComponent({
     // config needed to make it stand-alone..
   
     // perhaps we could also provide hooks for async handler loading,
     // or injecting props (from location, route or payload) here
   });
   
   HashLocation.addChangeListener(function(location){
     React.render(<MainComponent location={location} payload={getPayload(location)} />);
   });
   
   
   // Alternative: handle rendering yourself
   HashLocation.addChangeListener(function(location){
     var matches = routes.match(location);
     // do something with location and cause rerenders if necessary
     actions.changeLocation(location); 
   });

[ryanflorence]

Also, there are some use-cases we've all talked about before that we should make sure we can handle:

1. Navigation aware routing. Pinterest/Instagram style UI where if you're in a feed, and click on a photo, the photo shows up in a modal with the stream behind it still but the url updates. If you share that URL you are not in the stream, but at the photo. I used to consider this a "bug" but I think its totally valid now. You want the URL and you want to stay in context in the stream, unless its the first entry into the app.
2. Navigation Stacks. Imagine a mobile app with icons at the bottom for top-level pages. You navigate around one tab and the url changes, then you click another tab and navigate around, when you click the old tab again it doesn't send you to the "default" page for that tabs route but rather to the last spot you were at (and have some data around to build up the navigation UI, maybe the history state arg could hold this information?)
3. Dynamically Mounted Routes: Imagine you've got a settings button at the top of your app that opens in a modal. Inside that modal is more navigation like "profile, account, preferences" etc. You'd want that navigation to be mounted to the current URL. For example, you're at /courses/23/ and you click settings you'd want the url to be /courses/23/settings then you click profile and you've got /courses/23/settings/profile ... now that I'm typing this I realize that (1) navigation aware routing would solve this better so that you don't end up with a cartesian product of routes :D

[ryanflorence]

let { Toolbar, Main } = this.props.outlets;

👍 I had a version like that too, but didn't consider the value the introspection.

[ryanflorence]

I like the idea of not doing App.run and starting a location instead--as long as we can have that know enough about your app to be able to load up the components asynchronously.

HashLocation.addChangeListener(function (location) {
  var branch = MyRouter.match(location); // <-- perfect, now people can do their own data loading

  // but how do we, the Router, gather up the async components before rendering here?
  React.render(<MyRouter match={match}/>, document.body);

  // I don't want users to have to do boilerplate like this:
  loadHandlers(MyComponent, location).then(() => {
    React.render(<MyRouter match={match}/>, document.body);
  });
});

That's why I think we still need MyRouter.run. If you don't have lazily loaded handlers then we don't need MyRouter.run, so maybe its fine to require extra boilerplate for folks with lazy handlers.

[ryanflorence]

// we can easily provide something like this
Router.run = (Root, Location, cb) => {
  Location.listen((location) => {
    var branch = Root.match(location);
    resolveComponents(branch).then(() => {
      cb(location, branch);
    });
  });
};

// to allow for this as the minimal boilerplate and what we do for examples/guides/etc.
Router.run(App, HistoryLocation, (location) => {
  React.render(<App location={location} />, document.body);
});

// but this is still public API for other use cases
HistoryLocation.listen((location) => {
  React.render(<App location={location} />, document.body);
});

[ryanflorence]

Sorry, one more thing, ANIMATIONS.

CSSTransitionGroup has to just work™, but we can also maybe provide the previous handlers somehow, or something ... Anyway, I want the animation story to be better than it is today.

[mjackson]

Just a heads up, I added a shouldComponentTransition lifecycle hook to the example that replaces willTransitionFrom. It feels more natural to have this method directly on the component.

[ryanflorence]

The original transition from hook was an instance method, I always liked it better. I think we went with a static for parity with willTransitionTo which can't be an instance method. The primary use-case for willTransitionTo is authenticated routes, do we just push that to user land to be called in the location listener? Could even provide a function for people to use:

HistoryLocation.listen((location) => {
  var branch = App.match(location);
  runTransitionToHooks(branch, () => {
    React.render(<App location={location}/>, document.body);
  });
});

[agundermann]

A synchronous shouldComponentUnmount seems a bit limited. What if I wanted to show a custom dialog? Also, I think we should discourage people from aborting transitions, since we can't cleanly "undo" browser navigation without messing up the browser history (#952 (comment)).

I like the recent snippets with resolveComponents and runTransitionToHooks that can be provided by default, but can be opted out of for more control. This is also what I had mind when giving the user the transition object for the run callback.

var transitions = createTransitionManager(..);

HistoryLocation.listen((location) => {
  var branch = App.match(location);
  var transition = transitions.start();
  resolveComponents(branch, () => {
    // in case it was superseded by transition.start()
    if (!transition.isActive()) return; 

    runTransitionHooks(branch, transition, () => {
      // apply redirects etc; return .isActive() for convenience
      if (!transition.finish()) return; 

      React.render(<App location={location}/>, document.body);
    });
  });
});

(I'm not super fond of the API, but I think something like this is necessary to manage asynchronous flows)

[gaearon]

I think we should discourage people from aborting transitions, since we can't cleanly "undo" browser navigation without messing up the browser history

Agree, we try to do too much & fail there.

[mjackson]

@taurose Interesting. I hadn't seen your comment on #952, but it makes a lot of sense. Thanks for the link. It seems like both of the early return statements in the example you gave are boilerplate code that we could possibly eliminate using the transition manager, which would be responsible for keeping track of transitions that are currently in progress.

var manager = createTransitionManager(...);

HistoryLocation.listen(function (location) {
  var match = App.match(location);

  manager.resolveComponents(match, function () {
    manager.transitionTo(match, function () {
      React.render(<App location={location}/>, document.body);
    });
  });
});

[gaearon]

Why do we pass match down instead of just path? I forgot. Isn't the whole point that router can calculate it?

[mjackson]

@gaearon There is some work involved in calculating the match, namely traversing the route hierarchy and figuring out which routes match on the location.

[gaearon]

There is some work involved in calculating the match, namely traversing the route hierarchy and figuring out which routes match on the location.

Yep, but Router should have enough information to do it already. I mean, why do we need that explicit match call if Router can do it in its own componentWillReceiveProps and put the match into its state?

Sure, if user wants to traverse routes themselves, they want to call match themselves, but why force people to call it?

[gaearon]

Just throwing in something that has been bugging me about transitions.
I've been hating transition object for some time now, I think I can finally explain why.

transition.redirect and transition.abort are a leaky abstraction and IMO cause more trouble than they're worth.

1. They often act the same but not quite as replaceWith and goBack.
2. IIRC you can't use replaceWith and goBack inside transition hooks, why?
3. If there's no previous route, abort on client will just error
4. You can't really cleanly “abort” a client navigation as @taurose reminds us

All of these are solved by throwing away redirect and abort and making it possible to use transitionTo, goBack, canGoBack and replaceWith inside transition hooks. This will correspond 100% to the way redirects/aborts are handled throughout the rest of application. Instead of failing on abort of the first route, people can solve it as usual:

willTransitionTo(router, retry) {
  // transition.abort(); <--- meh
  if (router.canGoBack()) {
    router.goBack();
  } else {
    router.replaceWith('/some-other-page');
  }
}

Please correct me if I'm missing something obvious here.

This also makes more sense on server than catching redirects and aborts in onError and matching them by names like people do now. canGoBack would return false on server.

[agundermann]

It seems like both of the early return statements in the example you gave are boilerplate code that we could possibly eliminate using the transition manager, which would be responsible for keeping track of transitions that are currently in progress.

I guess we could put that check into resolveComponents etc. However:

• we still need isActive() for custom async functions
• we still need some object for manager to be able to distinguish between transitions and avoid "race conditions"
• without finishTransition, it's not clear when the transition is processed for redirects etc. We can't do it during render

transition.redirect and transition.abort are a leaky abstraction and IMO cause more trouble than they're worth.

Not entirely disagreeing, but, on the other hand, it

• reduces boilerplate
• increases readability
• abstracts router navigation from your handlers (what about server-side rendering?)

As I said before, my concern is that if we removed transition entirely, people might attempt to recreate it (or something similar).

But if we do leave it in, we should do something about transition.abort. Maybe just make everything configurable (onRedirect, onAbort, ..)? Or make it easier to parse the transition result so that one can implement their own finishTransition. Might be useful for server-side rendering, too.

// client
var manager = createTransitionManager(route, history);

history.listen(function(location) {
  var transition = manager.startTransition();
  somethingAsync(transition, function() {
    if(manager.isActive(transition)) {
      // ok
      React.render(..); 
    } else if(transition.isAborted()){
      // aborted by user; e.g. keep URL consistent
      history.replaceWith(lastLocation.path);
      // alternatively do nothing
    } else {
      // superseded/redirected
      // nothing to do
    }
  });
});

// server: just a single transition
var transition = createTransition(route);

somethingAsync(transition, () => {
  if(transition.isRedirected()){
    res.redirect(transition.getRedirectPath());
  } else if(transition.isAborted()){
    res.sendStatus(500);
    logError(transition.getAbortReason());
  } else {
    // ok
    res.send(React.renderToString(..));
  }
});

Edit: changed code snippet to process transition result manually instead of using handlers.

[mjackson]

Much of the information here is stale, so let's close. Thanks for the discussion tho everyone :)