Skip to content

Add params prop to link component #109

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

Closed

Conversation

zpchavez
Copy link

This change allows a params prop to be passed to the Link component as an alternative to passing a separate prop for each route param. Although you could achieve the same result using transferPropsTo, doing so will also transfer extraneous props that aren't route params, causing isActive to return false when it should return true.

@@ -71,7 +72,7 @@ var Link = React.createClass({
* Returns a hash of URL parameters to use in this <Link>'s path.
*/
getParams: function () {
return Link.getUnreservedProps(this.props);
return this.props.params || Link.getUnreservedProps(this.props);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should probably funnel this through Link.getUnreservedProps also, so you don't blow away reserved props on accident.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using reversed prop names in the params prop actually works just fine. If you wanted, you could have a route param called 'to'.

@ryanflorence
Copy link
Member

This is convenient, but you could bail out of JSX and get the same thing if you're imperatively building up your params somewhere

Link(props)

Unsure if I want to support this. Is there an objective answer to the question "When do I use params?"

@zpchavez
Copy link
Author

Using plain JS as a workaround totally works. I hadn't thought of that. It does seem like a workaround though.

@zpchavez
Copy link
Author

To answer your question "When do I use params?", it's useful if you have a component that wraps the Link component. I even think the params prop might be the preferred way to do it, and one-prop-per-param would be kept around for backward compatibility if at all.

@ryanflorence
Copy link
Member

I regularly bail out of jsx when I want to construct properties
imperatively, I don't consider it a workaround.

I'd like to let this sit a bit and see if there is much interest, right now
it just sounds like preference.

On Wed, Jul 23, 2014 at 1:56 PM, Zachary Chavez [email protected]
wrote:

To answer your question "When do I use params?", it's useful if you have a
component that wraps the Link component. I even think the params prop might
be the preferred way to do it, and one-prop-per-param would be kept around
for backward compatibility if at all.


Reply to this email directly or view it on GitHub
#109 (comment)
.

@ryanflorence
Copy link
Member

I've thought about this a bit more and I keep thinking that you could argue for convenience properties like this for every react component.

<div onClick={this.handleClick} className={className}/>

v.

<div props={props}/>

Am I thinking wrongly about this? Typically if you want to just pass along an object of props to a component you bail out of jsx, is there a reason Link should be different?

@ryanflorence
Copy link
Member

please re-open if you think I'm not thinking straight about this, or if there's a compelling case for Link to have convenience properties over other components.

Thanks for the PR :) Sorry I closed w/o merging :(

@sophiebits
Copy link
Contributor

In the next React release you'll be able to write <div {...props} />, much like the ES6 spread operator.

@bobeagan
Copy link
Contributor

Hey @rpflorence a couple thoughts:

a) If you don't like this approach then at least need to fix the incorrect example in the README which seems to indicate that params translates into the query string of a link. It should indicate that it is query and not params.

b) I think that if you have more complex routes that take more than a single param, the benefits of this start to show (e.g. /:companyId/:userId/:itemId/:action turns into <Link to="long-route" companyId="5" userId="10" itemId="7" action="something">)

c) I've had a component that generates a wrapped that didn't know what the params would be since the route it was generating for would be dynamic. In that case, if I could just pass a params object in that would make it work as desired.

There are obviously workarounds, as you pointed out, that make it possible to accomplish what I need to do without something like this but just wondering if accepting params as an object instead of explicitly adding them might be a better long-term solution.

@ryanflorence
Copy link
Member

a) what typo? :shifty-eyes: 154c074

b) normally that stuff is in a map handler, so the props are already dynamic and you don't type but one line, but if not, bail out of JSX and just do Link(props), or <Link {...props}/> as @spicyj says.

c) Again, don't use JSX when being more dynamic is friendlier.

I still just think this is a conversation about JSX and not Link.

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

Successfully merging this pull request may close these issues.

4 participants