Skip to content

Conversation

@abergs
Copy link
Contributor

@abergs abergs commented Sep 16, 2014

Added four tests. Only 1 (the first one) is succeeding.

These are related to issue #279.

Hope I didn't mess things up too much..

@ryanflorence
Copy link
Member

thanks for the test case (it really is the most valuable thing a project can get)

as @mjackson pointed out, the code is working as expected. To optionally match the parameter you'd want /comments/:id?/?edit to make both :id and the trailing slash optional.

(there's been talk about making trailing slashes optional by default, but we haven't dug in too much yet with that)

I'm going to close this ticket. Please make some noise if you think we should reopen it and we will.

@ryanflorence
Copy link
Member

shows that this is indeed unsupported

http://jsbin.com/pamon/4/edit

@ryanflorence
Copy link
Member

Alright, after some talk with @mjackson the optional param support is for matching, not building urls.

I don't suspect we'll get to this any time soon, so I'm going to close. For now you can:

  1. build the url yourself <Link to="/some/path/you/built"/>
  2. have a different <Route name="otherRoute" handler={SameHandler} /> and link to it instead.
  3. Send a PR to make the code be able to create links with optional parameters

Thanks again for the ticket :)

@tcoopman
Copy link

tcoopman commented Oct 6, 2014

Any pointers on how to start this PR?

I would guess changing paramMatcher = /:([a-zA-Z_$][a-zA-Z0-9_$]*)|[*.()\[\]\\+|{}^$]/g to paramMatcher = /:([a-zA-Z_$?][a-zA-Z0-9_$?]*)|[*.()\[\]\\+|{}^$]/g and handling this in Path.injectParams would be a start, but if I'm correct, the route matching would also need to be changed?

@ryanflorence
Copy link
Member

I'm questioning the use case behind ? in the first place.

I'd personally just create two paths ...

<Route path="foo/:name" handler={Handler} />
<Route path="foo" handler={Handler} />

And the handler can just check if it got name or not.

Can you think of any use-case that can't be handled this way?

@tcoopman
Copy link

tcoopman commented Oct 7, 2014

I thought the ? was because you wanted to validate the Links. If that's
not the case, I am in favor to drop the ?

On Mon, Oct 6, 2014 at 10:49 PM, Ryan Florence notifications@github.com
wrote:

I'm questioning the use case behind ? in the first place.

I'd personally just create two paths ...

And the handler can just check if it got name or not.

Can you think of any use-case that can't be handled this way?


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

Thomas Coopman
Thomas.coopman@gmail.com

dancannon added a commit to dancannon/react-router that referenced this pull request Oct 12, 2014
@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