Passing props to activeRoute about react-router HOT 22 CLOSED

We had a feature where you'd get the ActiveRoute class in addition to the activeRoute descriptor.

Maybe we should bring it back?

While discussing it's removal we opted to encourage the use of stores. (the auth example eventually uses localstorage, but the handler uses the store.)

Sent from my iPhone

On Jun 29, 2014, at 11:34 PM, Chris Lee [email protected] wrote:

Is it possible for a route handler to pass props to its child route handler? Currently I do this for signed in state, among other things. I notice that the auth example depends on localStorage instead.

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

from react-router.

This is mentioned in #30 but the idea of adding this capability wasn't followed up.

It does seem strange to call for a component to be rendered and not have a way to pass in props.

I expected to be able to say something like:

{this.props.activeRoute({foo:"bar", bing:"baz"})}

That specific syntax might be problematic. Also, the usefulness of this might not be as great as one would first think since you don't really know which component is going to be rendered in the above example. Using the Flux methodology, state like "logged in" should probably reside in a Store that you can check whenever you need to.

from react-router.

You also have to remember to pass in the props the router creates for you, like params and query.

Using the Flux methodology, state like "logged in" should probably reside in a Store that you can check whenever you need to.

This is exactly what we talked about when we removed ActiveRoute. These route handlers should make every attempt to be independent entry points to your app; depending on props from a parent limit their portability/isolation.

For example, if your user profile is nested inside some UI but you change the design and remove the nesting, you simply move the route in your route config and the app still works.

I still kinda like getting the ActiveRoute descriptor though...

Sent from my iPhone

On Jun 30, 2014, at 7:18 AM, kastork [email protected] wrote:

This is mentioned in #30 but the idea of adding this capability wasn't followed up.

It does seem strange to call for a component to be rendered and not have a way to pass in props.

I expected to be able to say something like:

{this.props.activeRoute({foo:"bar", bing:"baz"})}

That syntax specific might be problematic. Also, the usefulness of this might not be as great as one would first think since you don't really know which component is going to be rendered in the above example. Using the Flux methodology, state like "logged in" should probably reside in a Store that you can check whenever you need to.

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

from react-router.

+1 for getting the route descriptor. Seems like it would be useful for rendering navigation devices.

from react-router.

Or maybe

this.props.renderActiveChild(extraProps)

Then we can still ensure you get the props the router sets up.

Sent from my iPhone

On Jun 30, 2014, at 7:39 AM, kastork [email protected] wrote:

+1 for getting the route descriptor. Seems like it would be useful for rendering navigation devices.

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

from react-router.

if the fn this.props.renderActiveChild() also creates the child component then the context will also get passed down the child. 👍 (#60)

from react-router.

That's how I imagine it working.

My hesitancy is that now your routes are getting coupled to their view hierarchy, which is something I'd like to avoid. Also, timing of when things are created is out of the routers control, which might expose problems I can't think about right now.

Sent from my iPhone

On Jun 30, 2014, at 8:14 AM, Luigy Leon [email protected] wrote:

if the fn "this.props.renderActiveChild()" also creates the child component then the context will also get passed down the child. (#60)

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

from react-router.

My hesitancy is that now your routes are getting coupled to their view hierarchy, which is something I'd like to avoid.

This is the big issue for me as well. I agree that this.props.activeRoute(props) feels more React-y (and we could easily make it work by providing a function that already knows about params, query, etc. and adds them to any custom props you pass in) but the real issue is that now your routes are coupled to their parent route.

And BTW, activeRoute could be any # of route handlers, so the props you're passing in won't be very specific to that component anyway. Most likely it will be application-level stuff like "logged in" state, etc., which should be kept in stores in the Flux methodology. This feels like a way that we could encourage good application architecture by decoupling routes from the view hierarchy.

Also, timing of when things are created is out of the routers control, which might expose problems I can't think about right now.

I don't see any problems here that we couldn't work around. We would probably need to figure out a different way to get a handle on components for willTransitionFrom hooks since this.refs would no longer contain references to all routes from the top level, but that's doable (I think) if that's something we want to do.

from react-router.

I think i'd appreciate having a renderActiveRoute() fn. I feel like people are misplacing coupling worries. Props allow for me to define clear declarative api surfaces for my components, this need not result in tight parent/child hierarchy, if the api's are designed wisely.

In terms of flux, I prefer passing props parent -> child over having each view watch the store for its piece of state. In practice this does lead to a bit cruft with passing state down to children that the parent doesn't use directly, but even there we've found that the little extra redundancy is worth it for clarity of data flow through the views. It also means that each view is not tightly bound to a store.

As an aside, while children views may have many different parents (reusable!), views with children almost always have the same children, in that regard it seems silly to say that the parent view doesn't use the state. Even if it passing it directly through to a child it does use it, it uses for a child view which is an integral and defining part of the itself.

For us the usefulness of React is in its easy-to-reason-about-ness and a big part of that in our complex apps (react or not) have always declarative, easy to intuit api surfaces for component pieces. In React that generally means props, and propType validations

from react-router.

In terms of flux, I prefer passing props parent -> child over having each view watch the store for its piece of state. ...

I think the difference here is that a route handler is an entry point to your app and should be able to function as the root route or a deeply nested one.

Before dismissing this idea as "not fluxy", think about your route handlers as something different than the view they render.

var UserHandler = React.createClass({
  // this class deals with being a route
  render: function() {
    // User deals with the view, and is passed props like you want it to
    return User(someProps);
  }
});

Just a thought. I still like renderActiveRoute and think we should do it.

(I just don't see myself using it because I know how often in ember I've moved my route config around to change view nesting and then cursed the nested assumptions in the rest of the code.)

from react-router.

I think the difference here is that a route handler is an entry point to your app and should be able to
function as the root route or a deeply nested one.

that's a fair point, I don't think that each route being independent is 'not fluxy' but I do think that routes don't always need that flexibility. It seems like some nested routes don't make sense outside their resource, or nested resource?

I feel a little odd about the UserHandler it feels like a bunch of extra components without a ton of added value? Especially when it feels like the route component could/should handle that stuff? I did like the suggestion in another issue of the Route Mixin so you could make the UserHandler a route itself. Is that an analogous thought? or am i confusing issues

to the point of the renderActiveRoute could we just make activeRoute a partial fn like:

childDesc = function(extra, children) {
   return route.props.handler(_.extend(props, extra), children) // not sure about the inclusion of children here
}

then we can just use it like any other component, do nothing to get the same behavior, or add in extra props. I assume that might mess with the transition hooks?

from react-router.

I think the difference here is that a route handler is an entry point to your app and should be able to function as the root route or a deeply nested one.

Yes, that's it. It doesn't make any sense to pass custom props into a route handler because it should be able to function without a parent component.

renderActiveRoute is starting to feel like a foot gun.

from react-router.

I did like the suggestion in another issue of the Route Mixin so you could make the UserHandler a route itself.

Nah, that's different, though there's some overlap because route props get moved to their handlers.

The original analogy was this.props.children, this.props.activeRoute. Making it "just another component" ruins that analogy.

If you have more ideas, please share. At the moment I'm still liking this.props.renderActiveRoute(extraProps).

from react-router.

it should be able to function without a parent component

That's how I feel too, but I'm also aware that people are happy to couple their routes together.

React apps start with passing the props down and data back up, if we don't allow this flexibility then we are enforcing a bigger concept (stores) right out of the gate. This is right on my flexibility v. enforced-good-practice line.

from react-router.

React apps start with passing the props down and data back up, if we don't allow this flexibility then we are enforcing a bigger concept (stores) right out the gate.

ok, I can agree with that.

If we're going this route, I'd like to use the this.props.activeRoute(props, children) syntax. In this style, this.props.activeRoute would be a partially-applied function that already knows about params and query, etc. It would be a breaking change, but it would be the most familiar API for React devs IMO. The upgrade path for existing users would be to change this.props.activeRoute to this.props.activeRoute().

Also, I have no idea what people would be using the children argument for. I guess we would just pass that on through to the route handler.

from react-router.

The original analogy was this.props.children, this.props.activeRoute. Making it "just another component" ruins that analogy

that is true, hmm.

Yes, that's it. It doesn't make any sense to pass custom props into a route handler because it should be able to function without a parent component.

I can see how that might be an issue, although, does that not make sense in plenty of cases? Perhaps i am missing some of the philosophical background on why a route would always want to be like that? I feel like practically we end up having plenty of routes that don't/shouldn't work without a parent component. specifically in master/detail sorts of situations.

for instance a page with a list of artists side panel and a detail <main/> where you click one you get /artist/ -> /artist/1 generally I have a single say LibraryStore that is covering all the state for that page, so the store data is: { artists: [..], selectedArtist: {..} }. To display the correct detail view i'd need to pass in the selected artist. Of course could have the detail view listen directly to the store but that couples the view directly to a store instance, which assumes a whole lot of background infrastructure, when it really only needs an artist passed in as props (Which also lets me use the component against a different Store in another app area). Seems like the issue here is a choice about where to couple the view too? Personally we find it better to couple as few views to stores as possible and manage the annoyance of making sure sub components are handed the correct props, but I see how that is merely an architectural preference

React apps start with passing the props down and data back up, if we don't allow this flexibility then we are enforcing a bigger concept (stores) right out the gate.

this sort of nails my thought process about the whole thing (it appeared as I was writing :P)

from react-router.

Guys, does this mean that this gist isn't actually a bug? I've noticed that props do get password to the handler at first, but don't change on state change. https://gist.github.com/joecritch/586a6868874c99fa92eb

from react-router.

@joecritch You shouldn't put Route components inside a render method. Instead, pass your top-level Route descriptor directly to React.renderComponent as shown in the README. This makes it easier to see that the props you pass to your Route are "static".

from react-router.

I would be curious to know how FaceBook has chosen to do this. They clearly make a big a deal about props, @BinaryMuse's Fluxxor has mixin support for them, etc.

I'm not convinced there has to be ambiguity around this issue with n relatively equal approaches. If the problem space of the app is well defined there ought to be a functional composing declarative immutable etc. pattern that clearly stands out.

On that note I would also put stock into any work the clojure community has done on these issues by way of Om https://github.com/swannodette/om.

Edit:

https://github.com/swannodette/om#does-om-provide-routing
https://github.com/gf3/secretary

from react-router.

If we're going this route, I'd like to use the this.props.activeRoute(props, children) syntax. In this style, this.props.activeRoute would be a partially-applied function that already knows about params and query, etc. It would be a breaking change, but it would be the most familiar API for React devs IMO. The upgrade path for existing users would be to change this.props.activeRoute to this.props.activeRoute().

I've been thinking about this a lot and I think we should do it.

Or ... if we could swing this API (not sure we can), then it would feel even more react idiomatic, reacomatic, idioreactic, ridiomactic?):

var SomeHandler = React.createClass({
  render: function() {
    return (
      <div>
        <h1>Welcome</h1>
        <ActiveRoute/>
      </div>
    );
  }
});

from react-router.

I pushed a branch to get us started with this.props.activeRoute(). I looked into doing <ActiveRoute/> and I think the best way to do it would be to write a small component wrapper for this.props.activeRoute(). It would be like 4 lines and we could just forward props and children. The downside is that to do this we'd need to access the parent's props, and that would require using _owner which is private...

from react-router.

Yeah but ... that's a lovely, lovely API. Maybe we can get @petehunt and co. to bless our shenanigans.

Awesome work :)

from react-router.