Skip to content

Get rid of ? in paths #960

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
mjackson opened this issue Mar 16, 2015 · 8 comments
Closed

Get rid of ? in paths #960

mjackson opened this issue Mar 16, 2015 · 8 comments

Comments

@mjackson
Copy link
Member

Using a ? in path definitions has limitations. For example, if you want to match a path that has an optional segment in it, you actually need two of them: one for the parameter name and another for the slash. So people end up using paths like /users/:userID?/?edit.

I'd like to suggest that instead of ? we use ( and ) to indicate that portions of a path are optional. This gives us the ability to indicate where the optional portion of a path begins and ends. So you can do /users(/:userID)/edit which, at least to me, is a little cleaner.

This may help us address #820

See also #929

@BerkeleyTrue
Copy link

+1 adds readability IMHO. I hate this ?:someParam? it reminds me of this "¿ por que ? ".

@jquense
Copy link
Contributor

jquense commented Mar 16, 2015

so... backbone router syntax? +1 I much prefer ( ) to ?

@johanneslumpe
Copy link
Contributor

Looks definitely cleaner, because of the visual grouping going on there! 👍

@ryanflorence
Copy link
Member

Or kill it altogether and allow route configs w/o handlers to simply configure a route to match. Any use-case for optional segments can be solved with child route configs.

<Route name="foo" path="/foo(/:id)" handler={Foo}/>

<!-- can already be handled with -->

<Route name="foo" path="/foo" handler={Foo}/>
  <Route name="fooWithId" path=":id">
</Route>

You don't need a handler on fooWithId, Foo can just not ever render a RouteHandler and do the same parameter branching it does already w/ optional params. Removing optional params would also clean up our path matching and active link className code simpler. I think it also makes user code clearer.

@jquense
Copy link
Contributor

jquense commented Mar 25, 2015

Personally I'd really prefer keeping a path syntax for optional parts.

<Route name="foo" path="/foo" handler={Foo}/>
  <Route name="fooWithId" path=":id">
</Route>

seems unnecessarily verbose, and I find it harder to grok quickly compared to: /foo(/:id)...take that with as much weight as appropriate for anecdotal aesthetic opinions from strangers.

I definitely see how it would make your lives easier to be able to remove some complexity from path matching. If i'm being honest though, I'd prefer you folks handle the complexity instead of me :P

@ryanflorence
Copy link
Member

The thing I don't like is this:

<Route name="foo" path="/foo(/:id)" handler={Foo}/>

<!-- URL is `/foo/123` -->
<Link to="foo">do I have the active class?</Link>
<Link to="foo" params={null}>do I have the active class?</Link>
<Link to="foo" params={{}}>do I have the active class?</Link>
<Link to="foo" params={{id: 123}}>do I have the active class?</Link>
<Link to="foo" params={{id: 234}}>do I have the active class?</Link>

@rowbare
Copy link

rowbare commented Mar 27, 2015

I like the idea of killing off the magic characters. Using child routes, while more verbose, is clean, readable and declarative. It think it will also be a bit more intuitive when using Redirect to set default paths. It will ultimately make for more maintanable code which is more important that saving a few keystrokes initially.

@imevro
Copy link

imevro commented Apr 27, 2015

@mjackson when it'll be released?

pm5 added a commit to g0v/itaigi that referenced this issue Jul 6, 2015
@lock lock bot locked as resolved and limited conversation to collaborators Jan 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants