Don't throw errors when links can't find a route

[ryanflorence]

We should probably just warn here:

https://github.com/rpflorence/react-nested-router/blob/468bf3b20aa88413a03b3f94d518b3d70a437ffe/modules/helpers/makePath.js#L16-L20

Otherwise, you have to render a router before testing your handlers with something like:

var routes = require('../app/routes');
React.renderComponent(routes, document.createElement('div'));

[mjackson]

You don't actually need to render your route in order for links to find it. You just need to register it with the RouteStore. Maybe we should expose RouteStore.registerRoute as a high-level method for testing purposes?

var Router = require('react-nested-router');
var routes = require('../app/routes');

Router.register(routes);

[ryanflorence]

Seems good.

Cons

Something I really don't like is having to do things differently for tests v. app code. If we instead just warn, then developers don't have to know anything extra to test their route handlers.

Pros

However, if we just warn, applications could have busted links due to typos and tracking those down could get difficult, throwing an error stops you in your tracks, ensuring your app works before you ship it.

Additionally, not requiring a render is good so that you don't have to worry about everything your app is going to do with the initial handlers (like fetching user data, etc.)

I'd say yes, lets expose register. Wonder if we need a more explicit name like registerForTesting so that its clearly not intended to be used for anything else (unless anybody can think of a use-case where you'd want to register routes like this outside of testing).

[phated]

Would the register method be useful for server side rendering?

[mjackson]

@phated Yes, it would.

Basically register would just be sugar for RouteStore.registerRoute which is the method that populates the RouteStore with all the route data that Links need to know their href and whether or not they are active. When you mount a Route in the DOM, this is done for you. On the server, or in testing scenarios, we need to register all routes manually.

@rpflorence If we had a use for a Router() function, this would be it.

[ryanflorence]

Yeah, before we went to just a component this testing scenario simply required pulling in your route config, no rendering required.

Sent from my iPhone

On Jul 3, 2014, at 7:35 PM, Michael Jackson notifications@github.com wrote:

@phated Yes, it would.

Basically register would just be sugar for RouteStore.registerRoute which is the method that populates the RouteStore with all the route data that Links need to know their href and whether or not they are active. When you mount a Route in the DOM, this is done for you. On the server, or in testing scenarios, we need to register all routes manually.

@rpflorence If we had a use for a Router() function, this would be it.

—
Reply to this email directly or view it on GitHub.

[mjackson]

I removed the Router() function because I wanted to have something that I could pass to React.renderComponent instead of creating our own renderComponent method. I haven't written too much component testing code, but I think that most of it would require you to mount your component in the DOM, so it may not be unfamiliar to React devs.

[ryanflorence]

Router.registerRoutes(routes) then? I'm not excited about exposing the store so I'd rather just add it to the top-level module, I'm still a little unclear on the server-rendering use-case.

[nhunzaker]

👍 for Router.registerRoutes. Particularly when paired with Router.makeHref.

[couchand]

The other potential benefit of Router.register would be ad-hoc routing, which is a frequently annoying but quite common use-case.

Something like:

Router.register Route
  name: 'foobar'
  handler: React.createComponent
    render: ->
      div {}, 'foobar'

(note the space between register and Route)

I prefer (and it seems like you guys do, too) an explicit routing table, but this functionality is present in many modern web systems.

[ryanflorence]

(note the space between register and Route)

only somebody dealing with coffeescript pain daily would say something like this :P

[couchand]

Ha! One man's pain is another man's pleasure 😎

I thought it was worth pointing out, but perhaps a newline would've been a better way to do that.

[mjackson]

I'm not so convinced we need this anymore. I haven't really seen any solid use cases.

[mtscout6]

I'm testing a generic WorkflowItem link component that passes down props to the anchor. Which means its unit tests really don't care about a full application routing table. I'm still fleshing out this test right now but here's where it stands: (in CoffeeScript, sorry about your virgin eyes!)

WorkflowItem = require '../../../assets/scripts/widgets/workflow/workflow-item'
{Router, Routes, Route} = require 'react-router'
RouteStore = require 'react-router/modules/stores/RouteStore'
ActiveStore = require 'react-router/modules/stores/ActiveStore'

describe 'Workflow Item', ->
  route = <Route name='somewhere' handler={EmptyComponent}/>

  beforeEach ->
    TestUtils.renderIntoDocument <Routes>{route}</Routes>

  afterEach ->
    ActiveStore.updateState undefined
    RouteStore.unregisterAllRoutes()

 ...

  it 'routes to', ->
    instance = TestUtils.renderIntoDocument <WorkflowItem to='somewhere' />
    anchor = TestUtils.findRenderedDOMComponentWithTag(instance, 'a')
    TestUtils.Simulate.click anchor, button: 0
    ActiveStore.isActive(route).should.be.true

And the component I'm testing:

React = require 'react'
IsActiveMixin = require 'react-router-bootstrap/lib/IsActiveMixin'
cx = React.addons.classSet

WorkflowItem = React.createClass
  displayName: 'WorkflowItem'
  mixins: [IsActiveMixin]
  additionalReservedProps:
    complete: true
  propTypes:
    to: React.PropTypes.string.isRequired
    complete: React.PropTypes.bool
  render: ->
    classes =
      'workflow-item': true
      'workflow-item-incomplete': @props.complete? and !@props.complete
      'workflow-item-complete': @props.complete
      'workflow-item-todo': !@props.complete?
      active: @props.to and @state.isActive

    <li className={cx classes}>
      <a href={@getHref()} onClick={@handleRouteTo}>
        {@props.children}
      </a>
    </li>

module.exports = WorkflowItem

The issue I'm running into right now is that the simulate click is not actually routing, even though debugging through the transitionTo call is getting invoked properly. The IsActiveMixin is coming from react-router-bootstrap.

I demonstrate this example, because I feel that it is a fine example that you shouldn't need a full blown route table to test something trivial like this.

[ryanflorence]

Now that we have RouteStore.registerRoutes we can close this.

[nhunzaker]

👍