Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Props to handlers on Route #615

Closed
torarnek opened this issue Dec 16, 2014 · 37 comments
Closed

Props to handlers on Route #615

torarnek opened this issue Dec 16, 2014 · 37 comments

Comments

@torarnek
Copy link
Contributor

I think it should be simple to add props to handlers. It is a very common scenario to initialize the react components with some starting data as props.

This was changed from 0.10-0.11, and think the suggested workaround with a function closing over the handler is not very good.

What about that Router.Route takes a parameter "props" object that is passed to the handler?

@gaearon
Copy link
Contributor

gaearon commented Dec 16, 2014

But React Router doesn't do the rendering anymore. So it can't even pass these props to the handler, can it?

If all your handlers need same props (e.g. some context), you can do it yourself:

router.run(function (Handler, state) {
  React.render(<Handler {...state} myProp={something} />;
});

Similarly, you can pass them down to RouteHandlers. It's a very simple conceptual model compared to magic props (that you'd have to pass yourself anyway because you own the rendering).

What is your scenario?

@mjackson
Copy link
Member

@torarnek @gaearon is right. We're not rendering components any more, so there's no way we can pass the props to the component instance.

@torarnek
Copy link
Contributor Author

My scenario is that various sub-routes go to the same component, but with different props.
menuRoute -> menuComp
-> listRoute1 -> ListComp with some props set to A
-> listRoute2 -> ListComp with some props set to B

Props can be passed to menuComp with React.render(React.createElement(menuComp, props))

And menuComp can pass these props down to ListComp, however, in this scenario various ListComps need different props.

I have currently solved this by placing some logic inside ListComp where it checks what list it is, and then sets the various props. However, it would have been more natural to have done that outside.

@gaearon
Copy link
Contributor

gaearon commented Dec 16, 2014

@mjackson

Can we make any custom props on <Route> available on state.routes?
This would solve the problem once and for all.

@gaearon
Copy link
Contributor

gaearon commented Dec 16, 2014

What I mean is

<Route handler={App}>
  <Route handler={SomePage} somethingElse='yo'>
  ...

and then

router.run(function (Handler, state) {
  React.render(<Handler {...state} />;
});

// App
var mountDepth = 0;
return <RouteHandler {...this.props.routes[mountDepth].props} />;

This doesn't look very nice because you need to know own mount depth though. (Why doesn't router give you that btw? It should be easy to calculate if each RouteHandler incremented mountDepth passed to it from parent context, and gave incremented mountDepth in own context.)

@agundermann
Copy link
Contributor

Can we make any custom props on available on state.routes?

👍

I don't really like the factory function approach for handlers either.

Instead of using mountDepth, I imagine you could also do some preprocessing in Router.run:

router.run(function (Handler, state) {
  // build nested route props object
  var routeProps = {};
  var curRouteProps = routeProps; 
  state.routes.forEach(function(route){
      curRouteProps._childRoute = route.props;
      curRouteProps = curRouteProps._childRoute;
  });
  React.render(<Handler routeProps={routeProps._childRoute} />;
});

// App
return <RouteHandler routeProps={routeProps._childRoute} />;

In a similar way, this could also be useful for determining the page title, meta data, flux actions to fire, etc.

@mjackson
Copy link
Member

The reason I don't like putting the props on the descriptor (or element, whatever) is because every other place you do that in normal React (i.e. inside your render methods) the props update automatically as props/state of their parents change.

Here, the value of these props never will change, so people get confused and start putting their <Route> components inside render methods and all hell breaks loose.

This is actually one of the strongest arguments for not using descriptors to describe your route hierarchy, IMO. Because props are confusing.

@mjackson
Copy link
Member

Why doesn't router give you that btw?

@gaearon There's a getRouteDepth method on route handlers that does this. <RouteHandler> uses it to know which route to render.

@gaearon
Copy link
Contributor

gaearon commented Dec 16, 2014

Ah, I see your point now. Indeed, to avoid confusion, it might be better to just close over the arguments when needed.

Didn't realize getRouteDepth was there, sorry!

@jbrantly
Copy link

jbrantly commented Jan 2, 2015

I just ran into this and wanted to chime in.

The close over technique is adding an additional abstraction that really shouldn't be needed. We already have props which are used to pass data into instances of React components. Now we're saying that we need to be able to create custom React components (classes) to pass data in. This seems wholly unnecessary and breaks from reusable conventions. As an example, say I have a reusable React component library. There is no way for me to use those components in the router and configure them with static data because there is no way to create the factory function necessary for the close over technique. In other words, a React component must know that it's going to be used in the router beforehand if we want to be able to pass static data to it.

Additionally, I could work around some of the issues if in my render() I could somehow figure out what route is actually being executed. For example, I could call <RouteHandler /> and inspect the resulting element to determine if I needed to do anything else like add a prop or change my own behavior in some way. That doesn't seem to be possible anymore.

I can somewhat see how people could get confused by the old behavior. However, it was incredibly convenient and the workaround really just doesn't work. Any chance you could reconsider this decision or add in similar functionality through another mechanism?

@gaearon
Copy link
Contributor

gaearon commented Jan 2, 2015

There is no way for me to use those components in the router and configure them with static data because there is no way to create the factory function necessary for the close over technique. In other words, a React component must know that it's going to be used in the router beforehand if we want to be able to pass static data to it.

Not quite sure what you mean.

var SomeComponent = require('library/someComponent');

var MyHandler = React.createClass({
  render() {
    return <SomeComponent someProp={1} someOtherProp={2} />;
  }
});

// ...
<Route handler={MyHandler} ...

Why can't you do that?

@jbrantly
Copy link

jbrantly commented Jan 2, 2015

@gaearon Yea you're right. After posting I wound up making a factory function that does roughly that.

var factory = function(Type, props) {
  return React.createClass({
    render: function() { return <Type {...this.props} {...props} /> }
  }
}

@jbrantly
Copy link

jbrantly commented Jan 2, 2015

Additionally, I could work around some of the issues if in my render() I could somehow figure out what route is actually being executed. For example, I could call and inspect the resulting element to determine if I needed to do anything else like add a prop or change my own behavior in some way. That doesn't seem to be possible anymore.

I also figured I should look at the docs since I haven't in awhile. This is solved with the State mixin and getRoutes(). Whoops.

@appsforartists
Copy link

@mjackson How do you feel about exposing the machinery to make a Route subclass? (It looks like you've got a handful of functions duplicated in each Route subclass (like _inherits and _classCallCheck.)

That would enable people who want to add static data to the descriptor to do so (by creating a subclass that whitelists the props they need) without having a wildcard free-for-all that confuses new users.

Something like:

var CustomRoute = ReactRouter._createCustomRouteDescriptor(
  {
    "myProp": React.propTypes.string
  }
);


ReactRouter.run(
  <Route>
    <CustomRoute
      path    = "login/"
      name    = "login"
      handler = { require("./components/Login.jsx") }
      myProp  = "testing"
    />
    <Route
      path    = "welcome/"
      name    = "welcome"
      handler = { require("./components/Welcome.jsx") }
    />
  </Route>,

  ReactRouter.HistoryLocation,

  (Handler, routerState) => {
    console.log(
      routerState.routes.map(
        route => route.myProp
      )
    );
    // "testing"
  }
);

@mjackson
Copy link
Member

mjackson commented May 9, 2015

I just wanted to note here that in the new API (see #1158) routes are just plain objects, so you can put whatever props you want on them. Your transition hooks will get a route arg and components will receive this.props.route, so you should be good to go.

@appsforartists
Copy link

Cool. Thanks @mjackson!

@gauravChaturvedi
Copy link

this.props.route always gives me the parent route.. is there anyway to get the current active route along with its props ?

Right now I'm using this.props.routes[1] to access the current route.

Alternatives, is there a way I can get the depth of the current active route ?

@gauravChaturvedi
Copy link

@mjackson ^^

@taion
Copy link
Contributor

taion commented Oct 10, 2015

Why are you commenting on an ancient, closed issue?

Anyway, what do you even mean by "current active route"? By the nature of RR being a nested router, there is not in general a single active route but an ordered set of active routes from parent to child.

@gauravChaturvedi
Copy link

Apologies. I meant the current route. So I need to know if the current route is the root or not. Ideally what getDepth would have given me. Is there a replacement for that ? Or can I assume that this.routes will always have the current route at index 1 ? @taion

@agundermann
Copy link
Contributor

this.props.routes[1] will always be the route that's matched at depth 1. I don't know what you mean by "current route" either; there's only a current set of routes (this.props.routes), each of which is associated with a rendered route component (this.props.route).

There's a couple of things you can do with that, eg:

const rootRoute = this.props.routes[0];
const innerRoute = this.props.routes[this.props.routes.length - 1];

const depth = this.props.routes.length;
const myDepth = this.props.routes.indexOf(this.props.route);

@tomasdev
Copy link

Hold on... why all this mess instead of (proposing) something like

<Route name="app" path="/" handler={Page key="value"} />

Are you guys planning on implementing such a thing, or should handlers always be propsless?

@taion
Copy link
Contributor

taion commented Oct 31, 2015

There are no more handlers, just call cloneElement.

@sb8244
Copy link

sb8244 commented Mar 23, 2016

I've been looking into introducing React Router into a project, and was confused by this issue as well as many others. There was no clear answer to "this is how you get parent props into a child component." I threw this together and hope that someone will provide concrete "don't do that" if necessary.

class AppComponent extends React.Component {
  render() {
    // this.props contains props bindings that I care about and want to pass to children
    // Must keep these in scope of createElement to make them available to children
    return (
      <Router history={browserHistory} createElement={createElementFn(this.props)}>
        <Route path="/">
          <IndexRoute component={ReflectionForm} />
        </Route>
      </Router>
    );
  }
}

function createElementFn(parentProps) {
  return function(Component, props) {
    return <Component {...parentProps} {...props} />
  }
}

@perrin4869
Copy link
Contributor

@sb8244 I think this is the best possible solution. There is one problem though, and it is that eslint-plugin-react complains about the new function. I opened an issue in their repo: jsx-eslint/eslint-plugin-react#512

I'm hoping maybe some of the react-router devs can chime in on this one since it is a very common use case.

@lucidlive
Copy link

lucidlive commented May 27, 2016

My solutions is as follows to get custom props (like a title in my case) from the current route:

<Route title="Something" component={App}>

// CHILD COMPONENT
var currentPath = this.props.location.pathname;
var currentRoute = this.props.routes.filter(route => route.path === currentPath);
var title = currentRoute[0].title;

@gaearon
Copy link
Contributor

gaearon commented May 27, 2016

@lucidlive Shouldn’t you have this.props.route already? (Assuming you use 2.0+)

@lucidlive
Copy link

lucidlive commented May 27, 2016

Yeah but it doesn't give me the current active route. The above is the only way I could figure out how to access custom props of the current active route.

@jvanbruegge
Copy link

jvanbruegge commented Jun 24, 2016

Why is it not possible to do something simple like this?
<Route path='/'><App myprop='Hello' /></Route>
Would cover all needs. looks pretty, is self explanatory.

@ibc
Copy link

ibc commented Sep 7, 2016

Why is it not possible to do something simple like this?
<Route path='/'><App myprop='Hello' /></Route>

I wonder the same. Why not? I've read the entire thread (and others) and still I don't understand how to pass props to a component within a <Route>.

@viridia
Copy link

viridia commented Sep 20, 2016

Same here. I'd really like to be able to pass in arbitrary props to child components based on the current route. (Either that, or I'd like a way for router.isActive() to do prefix matching, which it currently doesn't do.)

The general problem here is that you have some common view component that is rendered via several different routes, but you want to make some minor route-specific tweaks to that component, without having to write a whole new subclass for every route.

@timdorr
Copy link
Member

timdorr commented Sep 20, 2016

Because Route doesn't render anything, so it can't include any children. The Router only reads its children as a particular kind of configuration, not as plain React elements for rendering.

This kind of thing is fixed in 4.0, where <Match> is just a plain React component and does render.

@viridia
Copy link

viridia commented Sep 20, 2016

Route is able to specify components which are passed to the main component:
<Route components={{ main: ProjectView, details: DetailsView }} path="pv(/:nimbletId)" />
So why can't it pass other kinds of information?

@ibc
Copy link

ibc commented Sep 20, 2016

React works by easily passing props to components, so a routing system that does not allow it makes no much sense IMHO.

@timdorr
Copy link
Member

timdorr commented Sep 21, 2016

@ibc
React works by easily passing props to components, so a routing system that does not allow it makes no much sense IMHO.

Yep, that's why 4.0 was built. Go check it out. It's exactly what you're describing.

@viridia
Route is able to specify components which are passed to the main component:
<Route components={{ main: ProjectView, details: DetailsView }} path="pv(/:nimbletId)" />
So why can't it pass other kinds of information?

Because those are React components, not elements. An element is an instance of a React component. Meaning, it has props assigned to it at that point. You can't tell the ProjectView or DetailsView components what their props are because they haven't been provided yet. They exist in definition only, there is not yet a instance of them that exists.

As a workaround, you can just use higher order functions to do this:

const DetailsView = ({ color, fontSize }) => ({ params: { id } }) => (
  <div style={{ color, fontSize }}>Project {projects[id].title}</div>
)

const route = <Route components={ProjectView({ color: 'red', fontSize: '16px' })} path="projects/:id" />

Same basic idea for a class: Just wrap it in another function that takes args that you can pass down to the return within the same closure.

@ibc
Copy link

ibc commented Sep 21, 2016

Yep, that's why 4.0 was built. Go check it out. It's exactly what you're describing.

Great, I wasn't aware of it.

@viktortsarevskiy
Copy link

viktortsarevskiy commented Feb 10, 2017

Have problem with HOC wrapped component. It is always remount when I change query params for same route.
Anyone encountered with similar problem?

Manuelandro added a commit to Manuelandro/react-router that referenced this issue Apr 13, 2017
There are a lot of request about passing props from Route to Component (ex remix-run#615 (comment))
This provide access to everything we pass from the Route component without broken its kinda of render
@lock lock bot locked as resolved and limited conversation to collaborators Jan 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests