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

Passing Props to Child Components with explicit props defined per route #1857

Closed
joewood opened this issue Sep 9, 2015 · 58 comments
Closed

Comments

@joewood
Copy link

joewood commented Sep 9, 2015

I've been using the 0.13 version and taking a look at the version 1 beta. One thing that I would like to do with react-router is be more specific about passing props from a parent to a child via a Router Handler. I'm not sure if this is easier in version 1. This is what I was thinking...

In 0.13 you do this you pass props and state from a parent component to the RouterHandler like this:

<RouteHandler {...this.props}/>

This way, regardless of the route all props (and/or state) is passed down to any child component. The problem here is that those components may require different properties - they shouldn't be dependent on the shape of the parent's component.

I was thinking that the Route map itself could use a callback where the function parameters are the props/state of the parent component. This way the props can be shaped in the route map itself. It could also include the URL parameters as third function parameter.

   <Route path="/inbox" handler={ (props,state) => <Inbox user={props.users} />}>
      <Route path=":messageId" handler={ (props,state,params) => <Message messageId={params.messageId}/> }/>
      <Route path="*" handler={InboxStats}/>
    </Route>
   <Route path="/calendar" handler={ (props,state) => <Calendar user={props.user} timezone={props.timezone} />} />

This may be possible in v.1 in other ways (using some sort of wrapper component for example). Just highlighting a frustration I had with 0.13.

@knowbody
Copy link
Contributor

knowbody commented Sep 9, 2015

@joewood could you check the #1531 which is related to your issue and join the discussion there?

@jdelight
Copy link

jdelight commented Sep 9, 2015

@joewood Passing state down from parent to child components seems to go against the principles of React IMHO. For passing props there's an example in master using React.cloneElement: https://github.com/rackt/react-router/blob/master/examples/passing-props-to-children/app.js#L48

But it might be worth looking into Contexts too. That article is a few months old but explains the concept well.

@joewood
Copy link
Author

joewood commented Sep 9, 2015

@jdelight not sure I understand how passing state from parent to child is an anti-pattern. I may be misunderstanding here, but passing state down to contained components as properties is kind of the mainstay of React. A parent knows the contract of the components it consumes.

What I'm trying to do is to avoid creating a dependency between the child components and the parent app. The child components shouldn't have to know the shape of the props of where they're being used. If the parent component is a simple data container, then that means all the components participating in the route map are exposed to the entire state of the container.

@knowbody
Copy link
Contributor

knowbody commented Sep 9, 2015

closing this, let's discuss in #1531
thanks @joewood

@knowbody knowbody closed this as completed Sep 9, 2015
@joewood
Copy link
Author

joewood commented Sep 9, 2015

Not much discussion in #1531 :-)
Can this be reopened - as I think that was a separate issue regarding children and nested Routes.

@ryanflorence
Copy link
Member

You should think of your route components as entry points into the app that don't know or care about the parent, and the parent shouldn't know/care about the children.

We have some nice hooks in 1.0 to build data loading abstractions that we hope to highlight with a sample implementation in AsyncProps that we'll put out there shortly after the 1.0 release, which would allow you to create exactly what you're wanting here.

@joewood
Copy link
Author

joewood commented Sep 10, 2015

OK, I'll have to look into the 1.0beta code a bit. As it stands Route components have to know about the parent's properties because that's what they receive as their properties. I guess that's fine as you treat them as part of the app and not supposed to be reusable. It just seems cleaner and more modular to me to specify the properties in the route map in the form of a reducer expression.

@agundermann
Copy link
Contributor

Have a look at the new <Router createElement>. You can easily set it up to selectively pass props from router state or your app state to route components (based on route config if you want to). Getting parent props or state there would require some work though.

@joewood
Copy link
Author

joewood commented Sep 18, 2015

OK, this page helps: Router.md. The problem with this is that only the properties are passed through and not the state.

If, as suggested, these routes are entry points to the app the composability of the GUI really suffers. It feels like the design is geared towards using Redux, where a single store is accessed to pull down the properties rather than propagating state to properties in more typical React.

Am I missing something?

@epitaphmike
Copy link

@joewood I am not sure if you ever figured this out, but what I am doing to solve this is the following

I pass

{React.cloneElement(this.props.children, {siteData: this.state.siteData})}

into the initial router.
My nested route 'AboutUsComponent', this is the main AboutUs page with a separate Menu within it. And all the pages under AboutUs have this Menu.

I did the same React.cloneElement inside the AboutUsComponent but used props instead of state, since I passed the siteData into AboutUsComponent as a prop.

var AboutUsComponent = React.createClass({

    render() {
        return (
            <div className="about-us-page">
                {React.cloneElement(this.props.children, {siteData: this.props.siteData})}
            </div>
        );
    }
});
<Route path="/about-us" component={AboutUsComponent}>
                <IndexRoute component={AboutUs} />
var App = React.createClass({

   --- Other Methods like getInitialState and ones to get the siteData---

    render: function() {
        return (
            <div className="container">
                <Header siteData={this.state.siteData}/>
               {/* This is the Router Components ie. "<Route path="/" component={App}>" */}
                {React.cloneElement(this.props.children, {siteData: this.state.siteData})}
                <Footer siteData={this.state.siteData}/>
            </div>
        )
    }
});

ReactDOM.render(
    <Router onUpdate={() => window.scrollTo(0, 0)} history={history}>
        <Route path="/" component={App}>
            <IndexRoute component={Home} />
            <Route path="/pricing" component={Pricing} />
            <Route path="/services" component={Services} />
            <Route path="/small-business" component={SmallBusiness} />
            <Route path="/about-us" component={AboutUsComponent}>
                <IndexRoute component={AboutUs} />
                <Route path="/our-team" component={OurTeam}/>
                <Route path="/jobs" component={Jobs}/>
                <Route path="/faq" component={Faq}/>
                <Route path="/press" component={Press}/>
                <Route path="/legal" component={Legal}/>
            </Route>
            <Route path="/privacy-policy" component={PrivacyPolicy} />
            <Route path="/terms-of-service" component={TermsOfService} />
            <Route path="/guarantee" component={Guarantee} />
            <Route path="/contact" component={ContactUs}/>
        </Route>
    </Router>, document.getElementById('app')
);

I hope this makes sense, and I hope it may help someone else. I read a lot of confused comments regarding this, with no real "example" explanations.

@OliverOliverOliverAnders

Hi epitaphmike,

is it possible to set up a small working example? I was also searching for the answer and looking for an live experience.

It would be very helpful...

Kind Regards,
Oliver

@hamxiaoz
Copy link

@OliverOliverOliverAnders isn't his example a working one?

The essential part is if you want to pass data from parent to child, initially you have:

var AboutUsComponent = React.createClass({

    render() {
        return (
            <div className="about-us-page">
                {this.props.children}
            </div>
        );
    }
});

But instead, you clone the children with the passed data:

{React.cloneElement(this.props.children, {dataChildrenNeeded: this.props.parentsData})}

You can read more details from: #1531 (comment)

@mockdeep
Copy link

mockdeep commented Feb 6, 2016

Can't comment on #1531 since that one was closed, but I have to agree with the OP there that cloneElement feels like an anti-pattern. It's probably a minor issue that it instantiates the components only to instantiate a new copy and throw the original away, but another issue is that we can't validate props with propTypes because that check runs before we can pass our additional props down. I'm not sure what would be a better api, but something like this might work for my case:

{this.props.renderChildren(moreProps)}

Though you might want to be able to account for the specific component being rendered with some sort of callback:

{this.props.renderChildren(function(child) { return propsDependingOnChild(child); })}

@taion
Copy link
Contributor

taion commented Feb 6, 2016

It's not an anti-pattern. Nothing is getting "instantiated". Your component class is not getting newed more than once.

cloneElement is exactly the idiomatic thing here.

@mockdeep
Copy link

mockdeep commented Feb 6, 2016

Whether or not it calls new, my understanding is that it creates an entirely new copy of the object, which is the point I'm trying to make, and seems to be what the docs are saying. We can debate semantics over whether that's instantiation or not, or whether or not it's idiomatic, but ultimately it makes it impossible to apply other idiomatic elements of React, like propTypes.

@taion
Copy link
Contributor

taion commented Feb 6, 2016

Of what object? What do you think a React element is, exactly?

@mockdeep
Copy link

mockdeep commented Feb 6, 2016

I'm not sure I understand the question, or how it's relevant. You seem to be taking a pretty condescending attitude, though, and not addressing my point that it makes it impossible to validate with propTypes.

@timdorr
Copy link
Member

timdorr commented Feb 6, 2016

@mockdeep In DEV, React's cloneElement does check propTypes.

@mockdeep
Copy link

mockdeep commented Feb 6, 2016

But the problem is that it also checks propTypes before the cloneElement, so I'm going to end up with console warnings regardless if I have required props.

@timdorr
Copy link
Member

timdorr commented Feb 6, 2016

That's a valid point. But stepping back, I normally treat my route components as top-level, container-style components. They don't receive props outside of the ones router provides. They dish those out to presentational components that can check PropTypes like normal. This keeps me from building monolithic über-components that pack in so much functionality that I've basically built everything in one place and I'm not getting any of the benefits of the toolchain I've chosen.

I'd try to keep route components aware of only router-provided props and try to maintain SRP.

@taion
Copy link
Contributor

taion commented Feb 7, 2016

That's definitely correct. The actual anti-pattern is passing props through route boundaries – in general you just shouldn't be doing this.

@epitaphmike
Copy link

I am on the same page as @timdorr. My route components are top-level, container-style components as well. I have actually altered the example I used above, since I am now using express too. I am leaning on another repo https://github.com/paypal/react-engine which is a composite render engine for universal (isomorphic) express apps to render both plain react views and react-router views. My routes file handles nothing but the routes. The props are all handled in the App and AboutUsWrapper files and passed down accordingly.

@taion
Copy link
Contributor

taion commented Feb 7, 2016

👍

@mockdeep
Copy link

mockdeep commented Feb 7, 2016

Thanks for the feedback. I think that makes sense, though it seems kind of out of scope for the router to push architectural decisions like that. In my case, I'm trying to have my AppBase pass down the global state to each of the container components like so:

var AppBase = React.createClass({
  ...
  render: function () {
    <Boilerplate globalState={this.state} />
    {React.cloneElement(this.props.children, {globalState: this.state})}
  }
});

ReactDOM.render(
  <Router history={history}>
    <Route path='/' component={AppBase}>
      <Route 'whatwhat' ... />
    </Route>
  </Router>,
  Document.getElementById('app-base')
);

How do you go about keeping state across containers without having some global state to pass down?

@timdorr
Copy link
Member

timdorr commented Feb 7, 2016

Redux! 😄

@ryanflorence
Copy link
Member

Usually if you're passing props across route boundaries your parent route knows exactly what it's rendering:

<Route path="/inbox" component={Inbox}>
  <Route path=":messageId" component={Message}/>
  <IndexRoute component={InboxStats}/>
</Route>

const Inbox = React.createClass({
  render() {
    return (
      <div>
        {/* this is only ever `Message`, except the weird case
          of `InboxStats` which doesn't need the prop */}
        {React.cloneElement(this.props.children, {
          onDelete: this.handleMessageDelete
        })}
      </div>
    )
  }
})

Instead, use a componentless route and just do "normal" React stuff.

<Route path="/inbox" component={Inbox}>
  {/* no more `Message` */}
  <Route path=":messageId"/>
</Route>

const Inbox = React.createClass({
  render() {
    const { messageId } = this.props.params
    return (
      <div>
        {messageId ? (
          <Message onDelete={this.handleMessageDelete}/>
        ) : (
          <InboxStats/>
        )}
      </div>
    )
  }
})

cloneElement is not bad practice on its own, but it can often be an indicator that there's a bit more straightforward way of doing something.

@mockdeep
Copy link

mockdeep commented Feb 7, 2016

Hmm, I'm trying to think about how that would apply in my situation. In this case, to be more specific, I'm trying to add a notification message that persists across containers:

var AppBase = React.createClass({
  getInitialState: function () {
    return { flashes: [] };
  },

  addFlash: function (flash) {
    this.setState({flashes: this.state.flashes.concat(flash)});
  },

  removeFlash: function (flash) {
    this.setState({flashes: _.without(this.state.flashes, flash)});
  },

  render: function () {
    return (
      <div>
        <Flash flashes={this.state.flashes} removeFlash={this.removeFlash} />
        {React.cloneElement(this.props.children, {addFlash: this.addFlash})}
      </div>
    );
  }
});

It looks to me like what you suggest makes sense when there's a particular resource that is being referenced, but I'm still not sure how it will work for periphery state like notifications or tracking loading state.

@taion
Copy link
Contributor

taion commented Feb 7, 2016

Please use Stack Overflow or Reactiflux for support-related discussions. We want to keep the issue tracker and updates on issues to only touch on features and bugs.

@Download
Copy link
Contributor

Download commented Jun 16, 2016

@underscorefunk You may want to have a look at Context.

I just keep finding this issue when searching how to do this with React Router.

I have routes set up like this:

export const routes = {
  <Route path="/" component={App}>
    <Route path="/brands" lang="en" components={{mainComponent:BrandBrowser, appbarComponent:BrandBrowserAppBar}} />
    <Route path="/merken" lang="nl" components={{mainComponent:BrandBrowser, appbarComponent:BrandBrowserAppBar}} />
  </Route>
}

What I am seeing is that my App component gets the prop lang passed in, but the BrandBrowser does not. Is that expected behavior?

@underscorefunk
Copy link

underscorefunk commented Jun 16, 2016

I didn't want to rely on context because of:

Context is an advanced and experimental feature. The API is likely to change in future releases.

So far filtering props and passing them on down is working flawlessly.

Also, I believe React router wants a single component to render... i.e. component={} as the prop... not components.

Edit: Thanks to Download for pointing out Named Components!

@Download
Copy link
Contributor

Multiple components is a feature called Named Components.

@prattl
Copy link

prattl commented Dec 13, 2016

This is essentially just syntactic sugar for the wrapper component solution suggested above by using an inline functional stateless component. This seems to be working for me so far.

<Route path='dashboard' component={props => <MyComponent myProp='some-value' {...props} />} />

@eladnava
Copy link

eladnava commented Dec 19, 2016

A workaround is simply to pass in custom props by assigning them to this.props.children.props like so:

render() {
    // Pass account info to routes
    this.props.children.props.account = this.state.account;

    // Render dashboard
    return (
        <div className="app">
            {this.props.children}
        </div>
    );
}

You could implement a check for this.props.router.routes[1].path and inject relevant props per route if that's what you're after.

@Stevearzh
Copy link

I have these code in my Root component:

<Router history={ hashHistory }>
  <Route path='/' component={ Main }>
    <IndexRedirect to='detail' />
    <Route path='detail' component={ Detail }/>
    <Route path='comment' component={ CommentBox }/>          
  </Route>
</Router>

and I want to pass some props to Main component, If I change the code like this:

<Route path='/' component={ props => <Main data={...props}/> }>

the browser would throw an error to told me that the props is undefined, even I change it to this:

<Route path='/' component={ props => <Main/> }>

the error still occurred. @prattl Any suggestion? Thanks.

@mismith
Copy link

mismith commented Jan 6, 2017

@eladnava That doesn't seem to work, I'm afraid. Even if it did, seems pretty hacky, no?

@eladnava
Copy link

eladnava commented Jan 6, 2017

@mismith might be slightly hacky, but it worked for me. If there is any cleaner way to do it without changing the entire router syntax / adding a middleware component, please share.

@mismith
Copy link

mismith commented Jan 6, 2017

I ended up just using what @Karthik248 proposed, e.g.:
React.cloneElement(this.props.children, {account: this.state.account})

Not pretty, but seems like it's using the methods we're supposed to as they were intended.

@mrjones91
Copy link

Isn't @epitaphmike 's solution the same as what @jdelight meant by using React.cloneElement? Just wondering why his comment elicited so many downvotes. That same solution received at least half as many votes when Mike gave it his own example, even though James also provided an example using it.

Just a quick "wtf, community?!" moment that I felt needed to be pointed out.

Carry on

@epitaphmike
Copy link

@mrjones91 It took he a few minutes to find the comments you were referring to. It's been about a year since I've looked at this thread. To what you are saying, I think most voters may have misunderstood what @jdelight was trying to say about passing props instead of state. But then again I cannot speak to their reasons. In my experience, when providing feedback in comments, (Which I rarely do, because of your exact point above) I always try to provide an example instead of linking to one. Your point is definitely noted though. I had never noticed the amount of downvotes that reply received.

@jtulk
Copy link

jtulk commented Feb 27, 2017

I'm reading through this, and it doesn't seem like the original question was ever answered (or if it was it's buried in the noise). The original question, as I understand it, is about declaratively exposing specific properties to a Route's child routes. Is that possible?

For example, because of Lifting State I frequently end up with most of my 'smart' happening inside a parent component, with the child routes being mostly dumb. For example, say I'm editing user data across multiple pages.

<Route path="user(/:userId)" component={UserContainer}>
  <IndexRoute component={UserProfile} />
  <Route path="profile" component={UserProfile} />
  <Route path="settings" component={UserSettings} />
</Route>

My <UserContainer /> component might encapsulate all my API logic and pass down the user data as props into each child. Right now I'd end up with something like this (say this.props.userData comes in from a Redux store):

class UserContainer extends Component {
  updateProfile = data => { ...logic },
  updateSettings = data => { ...logic },
  render() {
    return (
      React.cloneElement(this.props.children, {
        userData: this.props.userData,
        updateProfile: this.updateProfile,
        updateSettings: this.updateSettings
      })
    )
  }
}

Is there a way to only expose this.updateProfile to the child /profile/ route? and this.updateSettings to the child /settings route?

@fselcukcan
Copy link

fselcukcan commented Mar 3, 2017

TL;DR

The problem here, and in such similar cases, especially with the frameworks or libs written in some langs, a certain lack of means of combination (MoC).

Primitives seems ok in React.js they are pretty good.
Defining components, means of abstraction, with primitives, named element and the component in React.js, which seems ok as well in React.js.
But means of combination is incomplete or say broken. One must be able to pass the props to a component while combining a component to another component, doesn't matter if by putting one component inside another component as a child of it or passing one component as a prop to another component. One must be able to use abstractions, i.e. higher order creations, however higher order they are as one is able to use primitives.

With some already used syntax like;
<ComponentA x={<ComponentB y={<ComponentC z={} />} />} />
OR
<ComponentA x={ComponentB(ComponentC()) } />
,both of which are valid and already used in React.js.

Otherwise, this problems of combinations of abstractions will recur and will need some less than optimal and indirect solutions called workarounds like wrapping etc, etc. Abstractions must be first class citizens as primitives, whatever the first class means. All IMHO.

@ryanflorence
Copy link
Member

v4 this is easy stuff now.

<Route component={Something}/>
<Route render={(props) => <Something {...props} otherStuff="stuff"/>}/>

I have no clue if this meets the MoC discussed above but I'm having a blast combining things with these means.

@elyobo
Copy link
Contributor

elyobo commented Mar 8, 2017

Looks good, if only the v4 docs didn't error out so that I could read them @ryanflorence.

On https://reacttraining.com/react-router/ -

Chrome

Uncaught DOMException: Failed to read the 'localStorage' property from 'Window': Access is denied for this document.
    at <anonymous>:2:15
    at <anonymous>:8:9
main.js:4 Uncaught TypeError: Cannot set property '_key' of null
    at r (main.js:4)
    at main.js:31

FF

TypeError: t is null[Learn More]  main.js:4:9344
	r https://reacttraining.com/3770f2bc/main.js:4:9344
	a/< https://reacttraining.com/3770f2bc/main.js:31:31039

@ryanflorence
Copy link
Member

We just had a build misconfiguration earlier today, it's back up now.

@elyobo
Copy link
Contributor

elyobo commented Mar 8, 2017

Thanks, but I'm still seeing the same errors even after a hard refresh and in incognito mode. I'll check back later.

@ryanflorence
Copy link
Member

https://github.com/ReactTraining/react-router/tree/v4

Every package has a docs/ folder.

@dferber90
Copy link

dferber90 commented Mar 15, 2017

We are creating a tab system with react-router v3 where each tab receives different props from the parent. Imagine a user page with tabs for Welcome, Profile and Settings.

We use isActive to determine the rendered child and then figure out the props from there.
This is pseudo-code but hopefully you'll get the idea:

Routes

<Route path="user" component={UserContainer}>
  <IndexRoute component={Welcome} />
  <Route path="profile" component={UserProfile} />
  <Route path="settings" component={UserSettings} />
</Route>

In UserContainer:

const TABS = {
  PROFILE: 'profile',
  SETTINGS: 'settings',
}

// ..
render () {
  const activeTab = Object.values(TABS).find(tab => this.props.router.isActive(`/user/${tab}`))
  const tabProps = (() => {
    switch (activeTab) {
      case TABS.SETTINGS: return { title: 'Settings' }
      case TABS.PROFILE: return { title: 'Profile' }
      default: return { title: 'Welcome' }
    }
  })()
  return (
    <div>
      {React.cloneElement(this.props.children, tabProps}
    </div>
  )
}
  • 👍 Each tab gets only the props it requires
  • 👎 title is a poor example as the tabs could know about this themselves, but it was the shortest.
  • 👎 This still has the issue of the props of the child being validated before cloneElement adds the additional props. This forces you to mark all of these passed-in props as non-required to avoid the warning
  • 👎 The component is aware of the route it is being rendered in

I'm under the impression that all of this is a lot easier with v4, but we haven't upgraded yet.

@janusch
Copy link

janusch commented Mar 17, 2017

@ryanflorence Just to get this totally straight. Elaborating on your example:

v4 this is easy stuff now.

<Route component={Something}/>
<Route render={(props) => <Something {...props} otherStuff="stuff"/>}/>
I have no clue if this meets the MoC discussed above but I'm having a blast combining things with these means.

The below two routes want to get both the router props and the app state/props. Is this legit way to do it?

<Router>
  <Switch>
    <Route
      exact
      path="/list"
      render={routeProps => <List {...props} {...routeProps} />}
    />
    <Route
      exact
      path="/list/action"
      render={routeProps => <ListAction {...props} {...routeProps} />}
    />
  </Switch>
</Router>;

Is there a way to pass props to <Route component="List" />?
I think this is a very common use case, since the route <Switch/> will often be in a container component administrating state/props. Please correct me if I am wrong.

Thank you very much for the router and the training! I have been holding out 'til now before using the router. The docs are great now, I tiny addition could be to mention the <Switch /> earlier, e.g. in a basic example.

@cweems
Copy link
Contributor

cweems commented Mar 18, 2017

@janusch @ryanflorence I came here with the exact same question. Also agree that sending both props and router props to a component is a very common use case.

For example, I'm updating the parent component's state and then passing that down to a child in the router, but need to send the params from the route to that child component as well.

@ryanflorence
Copy link
Member

@janusch that's exactly how I do it.

@Download
Copy link
Contributor

To be honest, I feel the render function is a bit of a kludge. It breaks the declarative nature of using React to define the routes.

Ideally, we should be able to serialize the routes to JSON and back and have them survive. Putting functions as props on routes breaks that.

@xeoncross
Copy link

xeoncross commented Jul 27, 2017

This issue is one of the best React Tutorials I've ever read. Thanks everyone. I didn't understand ~40% of the application/concepts though. Bookmarking for later after I've spent more than a week learning React. Someone with more experience needs to clean this up and contribute it to a wiki.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 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