Skip to content

Conversation

@dancannon
Copy link
Contributor

You are now able to create links with optional parameters, before if you tried to do this the following error was returned Missing "param" parameter for path "/path/:param?".

Fixes some of the issues mentioned in #290.

@dancannon
Copy link
Contributor Author

I should have mentioned above this PR is not yet complete but it adds some basic functionality.

@dancannon
Copy link
Contributor Author

Ok I have added some more tests and I am now happy with the changed behaviour.

This should now be ready to go however since you mentioned in #290 that you were not sure if ? should be supported do you see any issues with adding this change?

@mjackson
Copy link
Member

@dancannon This looks great!

@rpflorence What do you think? Can we keep ? ?

@th0r
Copy link

th0r commented Oct 13, 2014

I'm for this PR with both hands! I have route catalog which looks like /catalog/:category?/:subcategory?. User can visit either /catalog, or /catalog/1 or catalog/1/2. Now I've separated catalog into 3 routes: catalog-root, catalog-category and catalog-subcategory, but generation of the navigation menu for them is a PAIN, because the name of the route depends on route params.
With optional params these 3 routes becomes one - catalog.

@tcoopman
Copy link

Would love to get this included!

For me the ? doesn't really matter, it's a bit more explicit but that's ok. Removing the ? is fine for me too.

@ryanflorence
Copy link
Member

my only beef with ? was that you can't create links to it. this seems good.

If ? ends up causing other problems, and we're constantly fixing bugs in code around it, then I'll probably want to kill it again, but until then, this looks great :)

mjackson added a commit that referenced this pull request Oct 13, 2014
[fixed] Added support for building URLs with optional parameters
@mjackson mjackson merged commit 24a0af2 into remix-run:master Oct 13, 2014
@mjackson
Copy link
Member

Thanks @dancannon !

@dancannon
Copy link
Contributor Author

Glad to help :)

@dancannon dancannon deleted the patch-1 branch October 13, 2014 19:11
@ryanflorence
Copy link
Member

this is released on v0.9.4, thanks again!

@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.

5 participants