remix-run / react-router Goto Github PK

These don't work on initial load

http://rpflorence.github.io/react-router/examples/master-detail/#/contact/new
http://rpflorence.github.io/react-router/examples/transitions/#/form

But this does:

http://rpflorence.github.io/react-router/examples/master-detail/#/contact/ryan

My guess is because it has params?

If you click around the classes get added, just not when you land at the route.

http://rpflorence.github.io/react-router/examples/dynamic-segments/#/user/123/tasks/foo

The "bob" link should have an .active class like it used to.

Looks like we're matching the path when we really just need to see if the link's name is in the list of active routes, paths can't tell you if parent's are active.

Are there any plans to support IE 8 with this router? React officially supports IE 8 but as it stands this router doesn't seem to. I am working on it a bit myself, but wanted to see if there are any official plans.

It would be nice to have functionality (in transition and Router itself) to dispatch specific route without change real URL.

E.g. I have auth flow close to https://github.com/rpflorence/react-nested-router/tree/master/examples/auth-flow . I visit my auth-only page /secret. Then I must be redirected to my auth page and URL in my address bar is /auth/signin. But it would be better for user still see /secret as URL and have some auth Route as activeRoute. In such case if I press Refresh I still have /secret URL and can return to it after sign in. Now I have /auth/signin and can't it.

I see it as something like

// inside my AuthMixin
willTransitionTo: function(transition) {
    //...
   if (!loggedIn) {
     transition.abort();
     authService.setTransition(transition);
     transition.forward('/auth/signin')
  } 
}

<Link to="foo" id="123">123</Link>
<Link to="foo" id="abc">abc</Link> 

Are both active when at either route, this used to be not be case.

Is there a specific motivation for making Router() not be - ie a standard, renderable React component? This would seem to make server-side rendering easier as you can tie in directly to React's rendering capabilities.

On line 200 of Router.js:

var component = React.renderComponent(route.handler(state.props), container, callback);

Managed to get it to work with:

var component = React.renderComponent(route.handler(route.staticProps), container, callback);

Not sure what the original plan was.

like users/:userId is fine but users/:userId/tasks/:taskId is not

seems we're far enough along now to write some tests

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'));

I'd like to have a match based on a param that contains a period, instead of using the period as a separator and therefore considering it as two separate params. Possible?

It should be really easy to animate between routes when the URL changes, preferably using ReactCSSTransitionGroup.

I hate promises ... can we not use promises?

willTransitionFrom: function() {
  undefined(); // ignored :(
}

I'm able to get a bower component (like react has itself) by doing the following

browserify modules/main.js -o main.js --standalone=ReactRouter

I can then reference ReactRouter like I do React (globals). I've got a proof of concept to show this working on bower (my own personal hack'd version to prove a point basically)

https://github.com/toranb/toranb-react-nested-router

I'd like to contribute something here that would make the repo more bower /browser friendly if possible.

update for anyone who might follow the above does not generate a working bundle (hacking today to find out why/how to resolve this so people can pull this via bower)

Is there any way to do the following?

Main

      <Routes handler={App}>
        <Profile path="profile" />
        <Explore path="explore" />
      </Routes>

Profile

        <Route>
          <Route name="dashboard" handler={ProfileDashboard} />
          <Route name="repos" handler={ProfileRepos} />
        </Route>

Explore

        <Route>
          <Route name="home" handler={ExploreHome} />
          <Route name="showcase" handler={ExploreShowcase} />
        </Route>

I would like to do this to separate my app out so i don't have to include every component/route in the Main handler.

Is it possible for a route handler to pass props to its child route handler? Currently I do this for setting/getting signed in state and opening modal dialogs, among other things. I notice that the auth example depends on localStorage instead. Is it necessary to externalize things from the app state like this to make them accessible from nested route handlers?

The current implementation of Router.dispatch waits for all async transition hooks to resolve before updating the UI. The result of willTransitionTo hooks is also stored off in the currentContext property of routers so that it can be used in components. This is similar Ember's beforeModel and model hook.

In a discussion with @rpflorence this morning, we determined that in the Flux model this method of fetching data from the server doesn't really fit. Components that need model data register to get them from stores as they are mounted. When they are first mounted, if they don't have their data yet, they can show some "loading..." UI. As the store gets the data, the components update. This also has the great benefit of immediately updating the UI in response to user action, which is critical. So we should probably remove the context stuff.

Thus, the primary purpose of transition hooks becomes simply aborting or redirecting the transition. The question is: is there ever a time when we need to asynchronously determine which of these actions to take?

The transitions example currently gives this console warning when switching away from the dashboard page:

Invalid access to component property "refs" on Form at the top level. See http://fb.me/react-warning-descriptors . Use a static method instead:

.type.refs(...)

because of the line

if (component.refs.userInput.getDOMNode().value !== '') {

in transitions/app.js, but the router passes the component descriptor (basically just type and props) to the willTransitionFrom function.

In React master, I don't believe the example will work at all because descriptors and components have been more properly separated. If the intention is for willTransitionFrom to receive the actual component instance, your best approach is probably to use cloneWithProps in conjunction with new-style refs (facebook/react#1554) after that lands.

Is there a recommended way to set document.title upon routing? I suppose one could just set it in all of the componentDidMount handlers, but it might be nice if there was a general strategy for this. (For example, maybe one page doesn't really care about title, in which case we'd want to fall back to the default.)

(Someone was also asking in IRC about this last night right after you left, @rpflorence.)

It should be possible to cancel/abort a transition that is about to happen. Similarly, it should be really easy to store and retry a transition that was previously canceled for some reason.

The auth-flow example, which I broke with this commit, should be updated to use this functionality.

As of ef8bfba, it was possible to pass extra props to routes. On the current master, this no longer appears to work. Has this functionality been intentionally removed, and if so, is there another way to pass extra props into routes?

I've been following along w/ the changes recently from 0.1.0 and one thing I liked about the initial router is that I could create it /export it as a module and then in my index.html I could kick off the React.renderComponent (making a clean separation from the module itself). This was nice because my tests could just import the module (like my index.html did) and add it to a fake dom / etc

var router = Router(
    Route({handler:App},
        Route({name:"dashboard", path:"dashboard", handler:PreDashboard},
            Route({name:"inbox", path:"dashboard/inbox", handler:PreInbox})
        )
    )
);
export default router;

System.import('app/router').then(function(router) {
    router['default'].renderComponent(document.body);
});

In the new syntax I'm not sure how I'd do something similar because it doesn't look like I have an "export friendly" module as I did before. I'm sure you guys are thinking about testability as you move through this refactor of the api, but how can I do what I was previously in the new version? (to enable testability)

React.renderComponent((
  <Route handler={App}>
  </Route>
), document.body);

I'm pretty sure it should, since we wait for transition hooks to resolve before rendering, we should also allow devs (especially during tests) to be able to wait for the transition to complete and then continue on with their task.

context is determined by where a component is created (in your mount() function), not where it is rendered (in your App component). Also note that context is experimental and likely to change in future versions. -- @spicyj

Context is not passed down to subroutes, because the handler component is created outside the rendering component.

Thoughts on having access to activeRouteHandler & activeRouteProps or some function to create the component explicitly?

Hey!

I am very impressed by your partial app loading example and after some playing i got it going with browserify, i thought you might be interested :)

https://github.com/pieterv/react-partial-app

I based the code off @petehunt's cjs version from #1 with a couple of things to note that are different:

• I am using the browserify module loader as the module cache and try requiring the base component on componentWillMount if that fails it will then load in the bundle, this should also in theory allow for synchronous loading of the components when rendering on the server. https://github.com/pieterv/react-partial-app/blob/master/partial-app.js#L58
• The browserify bundling setup isn't very good at the moment as you have to manually specify what modules go in what bundle, we should be able to automate this in the near future with browserify's factor-bundle plugin but there are a few bugs blocking this.

I really like the direction this router is going, i'm looking forward to using it in my projects!

Currently you must require a root route, the following doesn't do anything:

Router(
  Route({name: 'foo', handler: Foo}),
  Route({name: 'bar', handler: Bar})
);

You have to do this:

Router(
  Route({handler: App},
    Route({name: 'foo', handler: Foo}),
    Route({name: 'bar', handler: Bar})
  )
);

Maybe we can detect that routes.length > 1 and then provide a default route and handler like:

React.createClass({
  render: function() {
    return <div>{this.props.activeRoute}</div>
  }
});

Even though I suspect every real app out there will have a root route (shared layout, global nav, etc), when you're just getting started this is a hurdle that isn't obvious and it would be nice to feel productive the first time you use this router.

We should support rendering on the server.

Right now the router calls the handlers and just gives you a pre-packaged this.props.activeRoute.

This is good because:

1. It makes adding the activeRoute into render dead simple, just like this.props.children.
2. We fill it up with the parameters from the url automatically.

This is maybe bad because

1. Developers have less control
2. You can't send any other props down the view hierarchy, forcing child routes to send their actions to some dispatcher/store that the parent(s) subscribe to.

If we changed this it could go from this:

var Something = React.createClass({
  render: function() {
    return <div>{this.props.activeRoute}</div>;
  }
});

to something like this:

var Something = React.createClass({
  createStuff: function() {},
  render: function() {
    var ActiveRoute = this.props.ActiveRoute;
    return <div><ActiveRoute id={this.props.params.id} onCreate={this.createStuff} /></div>;
  }
});

Strange issue:

I had a root Route handler like this:

var App = React.createClass({
  displayName: 'App',
  render: function(){
    return  E.p(null,
              Router.Link({ to:'dashboard' }, 'dashboard'),
              Router.Link({ to:'intro' }, 'intro'),
              this.props.activeRoute
            )
  }
})

This worked fine when loading the page at / and navigating from there. But if I loaded the page at e.g. /#/dashboard then dashboard is not rendered... (but oddly if I console.log(this.props.activeRoute) it does have value!. My workaround is to use App handler for basically just returning this.props.activeRoute or a temporary DOM node:

var App = React.createClass({
  displayName: 'App',
  render: function(){
    if (this.props.activeRoute) return this.props.activeRoute
    return  E.div()
  }
})

<Route name="contact" path="/contact/:id" handler={Contact}/>
<Route name="new"     path="/contact/new" handler={NewContact}/>

In this scenario transitioning to "new" will incorrectly match "contact" first and render. Reverse them and it'll work as expected like below:

<Route name="new"     path="/contact/new" handler={NewContact}/>
<Route name="contact" path="/contact/:id" handler={Contact}/>

woops

Links throw errors if they can't find a route. When you're unit testing you probably don't have your routes configured.

Workaround (Solution?)

Have a file named routes.js that exports a router, but doesn't render:

// app/router.js
var Router = require('react-nested-router').Router;
module.exports = Router( /* ... */ );

Then a main file that renders it

// app/main.js
var AppRouter = require('./router');
AppRouter.renderComponent(document.body);

And then in your unit test, bring in the router, but don't render it. This should be enough to register all the routes so the links don't freak out.

// test/some-component.js
var AppRouter = require('../app/router');
var Component = require('../app/components/some-component');

// test as usual

Options for this lib

• Don't throw an error, just warn, then who cares?
• Make it easy to stub
• Have some testing flag to ignore links to missing routes
• Do nothing, having a routes.js file that doesn't render seems like the right thing to do, especially for integration tests, gives you control to render and teardown before each test.

I want to give my users instant response to <Link> activation, then have the route handler component dispatch an event to have its store loaded according to the params.

I'm also in the habit of injecting dependencies in props. Sneaking in the dependencies as enclosed variables is easy when every component is in the same file, but feels wrong the moment you put a component in its own file:

var Component = require('../component.jsx')(stores, dispatcher.fire);
React.renderComponent(Route({ handler: Component }), document.body);

I'd prefer this:

var Component = require('../component.jsx');

React.renderComponent(
    Route({ handler: Component,
            stores: stores,
            fire: dispatcher.fire  }
    ), document.body);

Component gets stores and fire as RNR-style "static props"; it can start watching the stores in componentDidMount, and fire events from its event handlers. All we need is fire that first event to load the data. So, in willTransitionTo, we calls fire on the dispatcher…

… except, we can't. There's no such argument, and we can't provide component like willTransitionFrom gets because the component hasn't been created yet. Indeed, we want to handle this before the component is created so we can avoid a flash of not-even-trying-to-load unanticipated state. That leaves the route's static props as a way to provide willTransitionFrom access to the dependencies.

Am I missing some cleaner way to accomplish this?

when you cancel a transition on willTransitionFrom

• the url changes to the next route, needs to be changed back
• any views don't update anymore when clicking links

Check out the transitions example to see this behavior.

In the master/details example, we already have the full contact object available in the master component.
When linking to the detail component, we pass the id then fetch back the contact.

My current project has a similar use case and I would like to optionally add exra data to the Link (e.g. a full contact) so I avoid a request when I already have needed data.

Any idea ?

Trying the example from README, I'm getting a warning and an error:

react-with-addons-0.10.0.js:5670: Invalid access to component property "render" on Unknown at the top level. See http://fb.me/react-warning-descriptors . Use a static method instead: <Unknown />.type.render(...)

react-with-addons-0.10.0.js:5670: Error: Invariant Violation: ReactCompositeComponent.render(): A valid ReactComponent must be returned. You may have returned null, undefined, an array, or some other invalid object.

I can reproduce this in the Node REPL:

> var React = require('react'), Route = require('react-nested-router').Route;
> var App = React.createClass({ render: function() { return React.DOM.p(null, 'app'); } });
> React.renderComponentToString(App())
'<p data-reactid=".2b0hddk49a8" data-react-checksum="-225571812">app</p>'

> React.renderComponentToString(Route({ handler: App }))
Error: Invariant Violation: ReactCompositeComponent.render(): A valid ReactComponent must be returned. You may have returned null, undefined, an array, or some other invalid object.

This is with [email protected] and [email protected].

On the first pass, the handler's props are undefined. As soon as the route is mounted it runs all transition hooks needed to sync with the current URL. When this is done, it knows its props which include any static props that were passed to the Route. -- @mjackson

Apps with required props (like with fluxxor) would get warnings of missing props on first render

var routes = (
  <Route handler={FluxxorApp} flux={flux}>
    ...
  </Route>
);

"Warning: Required prop flux was not specified in FluxxorApp."

Do you need to be able to pass static props through to the handler on the first render pass? If so, we could easily make that change.

Yes 👍

maybe just transferPropsTo if no this.state.matches

render: function() {
  var props = computeHandlerProps(this.state.matches || [], this.state.activeQuery);
  var handler = this.props.handler(props);
  if (!this.state.matches) {
    return this.transferPropsTo(handler);
  }
  return handler;
}

Hey, I've encountered a bug while using this library and attached a failing test case.

It seems like the props.activeRoute component is not re-rendered when calling router.renderComponent().

The first render call behaves correctly. After that I expect 'render index' and 'render app' to be logged every second. Instead only render index appears. The timestamp doesn't change either.

I'm not really sure where to start looking for the bug. Maybe you could give me a hint?

browserify -t reactify main.jsx > bundle.js

var React = require('react')
var RRouter = require('react-nested-router')
var Router = RRouter.Router
var Route = RRouter.Route

var App = React.createClass({
    render: function() {
        console.log('render app')
        return (
            <div>
                <h1>App</h1>
                {this.props.activeRoute}
            </div>
        )
    }
})

var Index = React.createClass({
    render: function() {
        console.log('render index')
        return <span>{Date.now()}</span>
    }
})

var router = Router(
    <Route handler={App}>
        <Route name='index' path='/' handler={Index} />
    </Route>
)

setInterval(function() {
    router.renderComponent(document.body)
}, 1000)

<!DOCTYPE html>
<body></body>
<script src='bundle.js'></script>

Route.spec.js contains a commented out test for server rendering but it seems, server rendering would at least require exposing findMatches in the main module.

Any plans for reintroducing server rendering support?

This is bad for TravisCI

It seems now that the root hander will be inited and rendered twice, because root Route calls render first time on initial and the second time after parsing URL and setState. It means Route creates two instances of handler and any functionality in render and componentDidMount is called twice almost at the same time, what is totally confusing and can lead to strange bugs.

<Route handler={FrontController} path="/">
    <Route handler={PageController} path="/">
      <Route handler={SubPageController} path="/" />
    </Route>
 </Route>

 // in each controller
componentDidMount: function() {
    console.log(name);
}

// output
FrontController
FrontController
PageController
SubPageController

Currently, when none of the routes in a router match the URL, the router sets its top-level props to null which effectively removes everything below it from the DOM. It would be nice in these instances to let users provide a "not found" handler that can essentially show a 404 page. We would only need one such handler per router.

Perhaps the existing API could be altered to include this?

<Route notFound handler={NotFound}/>

@rpflorence thoughts?

Is there a notion of a "catch-all" or default route?

Or routes that match based on globs or regular expressions?

Is there a reason that lib is included in .npmignore? Its inclusion means that react-router doesn't work properly when installed via NPM and required by a tool like browserify (since the main file specified in package.json is within the lib directory).

Removing the lib path from .npmignore fixes the issue - but I didn't know if there were a specific reason for its inclusion in the first place...

<Link to="/">Home</Link>

used to work.

I am not sure how related to this project this issue is but here goes.

When navigating from / after a fresh page load routing seems to work ok.

When navigation from e.g. /#/intro after a fresh page load I see the following error:

Uncaught TypeError: Cannot read property 'parentNode' of undefined react.0.10.0.js:3486
DOMChildrenOperations.processUpdates react.0.10.0.js:3486
(anonymous function) react.0.10.0.js:9751
ReactPerf.measure react.0.10.0.js:13865
processQueue react.0.10.0.js:13315
ReactMultiChild.Mixin.updateChildren react.0.10.0.js:13429
ReactDOMComponent.Mixin._updateDOMChildren react.0.10.0.js:9447
(anonymous function) react.0.10.0.js:9300
ReactPerf.measure react.0.10.0.js:13865
ReactComponent.Mixin._performUpdateIfNecessary react.0.10.0.js:6843
ReactComponent.Mixin.receiveComponent react.0.10.0.js:6814
ReactDOMComponent.Mixin.receiveComponent react.0.10.0.js:9273
(anonymous function) react.0.10.0.js:8401
ReactPerf.measure react.0.10.0.js:13865
ReactCompositeComponentMixin._performComponentUpdate react.0.10.0.js:8337
ReactCompositeComponentMixin._performUpdateIfNecessary react.0.10.0.js:8280
ReactComponent.Mixin.receiveComponent react.0.10.0.js:6814
ReactCompositeComponentMixin.receiveComponent react.0.10.0.js:8365
(anonymous function) react.0.10.0.js:8401
ReactPerf.measure react.0.10.0.js:13865
ReactCompositeComponentMixin._performComponentUpdate react.0.10.0.js:8337
ReactCompositeComponentMixin._performUpdateIfNecessary react.0.10.0.js:8280
Mixin.perform react.0.10.0.js:16742
ReactComponent.Mixin.performUpdateIfNecessary react.0.10.0.js:6824
ReactCompositeComponentMixin.performUpdateIfNecessary react.0.10.0.js:8230
enqueueUpdate react.0.10.0.js:15203
ReactCompositeComponentMixin.replaceState react.0.10.0.js:8114
ReactCompositeComponentMixin.setState react.0.10.0.js:8093
(anonymous function) react.0.10.0.js:755
invokeCallback react.0.10.0.js:1735
publish react.0.10.0.js:1780
publishFulfillment react.0.10.0.js:1886
flush

For cases like #52 it would be lovely to provide interesting extension points.

We could start by first making everything in Route a RouteMixin, and then export Route as nothing more then a component that uses it. This would allow people to start adding their own custom behavior to their Route config.

Any plans to distribute this as a complied JS file suitable for including via a plain old script tag? (and then potentially served via CDN)

I ran into some confusing behaviour that I would appreciate explained and documented better. In my root Route:

var routes = Route({ handler: z },
  Route({ name: 'intro', handler: viewIntro }),
  Route({ name: 'dashboard', handler: viewDashboard }),

z does this:

var z = React.createClass({
  displayName: 'z',
  statics: {
    willTransitionTo: function(transition){
      if (user_used_app_count() === 1) {
        transition.redirect('intro')
      }
    }
  },
  render: function(){

This does not work.

I got it working by not redirecting on root:

var viewDashboard = React.createClass({
  displayName: 'dashboard',
  statics: {
    willTransitionTo: function(transition){
      if (user_used_app_count() === 1) {
        transition.redirect('/intro')
      }
    }
  },
  render: function(){

Clarity about what's going on would be greatly appreciated.

It would be nice if pushState routing could fall back to regular page loads when pushState isn't supported, so you get all the SPA goodness in modern browsers but the site still works in older ones.

(It might also be nice to be able to fall back to hash-based routing which is what our current Backbone routing does at KA but that's also a pain because the server-side code won't get the full URL and so can't decide what bundles to send down, etc.)