Skip to content

Pass props to component#1

Merged
didierfranc merged 2 commits intodidierfranc:masterfrom
perzi:feature-pass-props-to-component
Feb 19, 2017
Merged

Pass props to component#1
didierfranc merged 2 commits intodidierfranc:masterfrom
perzi:feature-pass-props-to-component

Conversation

@perzi
Copy link
Copy Markdown
Contributor

@perzi perzi commented Feb 15, 2017

Async can pass componentProps to the lazy loaded component. For instance if you need the react router params in props.

@perzi perzi closed this Feb 15, 2017
@perzi perzi reopened this Feb 15, 2017
@didierfranc
Copy link
Copy Markdown
Owner

Indeed it could be great to pass props, I never needed it but it's a basic we should support. I think it will be better like the following, if you make the changes I'll merge this PR.

PS: don't forget to build the lib, some people like to have git commit as dependencies ;)

class Async extends React.Component {
  componentWillMount = () => {
    this.props.load.then((c) => { this.C = c; this.forceUpdate() })
  }

  render = () => this.C ? <this.C.default {...this.props} /> : null
}

Async.propTypes = {
  load: React.PropTypes.instanceOf(Promise).isRequired,
}
const Home = props => <Async load={import('./Home')} {...props} />

@perzi
Copy link
Copy Markdown
Contributor Author

perzi commented Feb 19, 2017

@didierfranc Updated PR with built lib.

With my solution there's no risk of conflicting prop names, the lazy loaded component can also have a prop named load. In your example, if you pass a load prop to Home, that will be used instead of the promise. Another solution would to rename loadto something more obscure, but there's still risk for conflict. Your example is shorter in code but I would prefer to keep the code as I wrote it.

In general I prefer not pass any props only intended for a parent component to a child component. It's a risk of passing to much and leading to unforseen consequences.

@didierfranc
Copy link
Copy Markdown
Owner

Yes I was thinking about that risk too, thanks for that !

@didierfranc didierfranc merged commit 0b95a99 into didierfranc:master Feb 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants