Skip to content
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

parsing query params after optional rest path #130

Open
Standa666 opened this issue Apr 27, 2015 · 6 comments
Open

parsing query params after optional rest path #130

Standa666 opened this issue Apr 27, 2015 · 6 comments
Labels

Comments

@Standa666
Copy link

I have following query

one/two/three/four?id=1&test=1

and router based on string

one/two/:path*::?query:

I expect two values

0. "three/four"
1. {id: 1, test: 1}

but I receive

0. "three/four?id=1&test=1"
1. undefined

I found that compiled pattern looks like this (example)

/^/?one/two/?(.*)?/?(?:?([^#]*))?/?$/i

but it should probably looks like this (examle)

^/?one/two/?([^?]*)?/?(?:?([^#]*))?/?$

The generated pattern for :path*: covers also query which starts with question mark. So this character should probably be excluded.

Am I right?

@millermedeiros
Copy link
Owner

rest segments match anything. you only have the conflict because both segments are optional and we don't have any special logic for this case.

@olavorn
Copy link

olavorn commented Aug 24, 2015

Same for me, i'm in a similar case strugling with the parser to route:

Path:
"Route/App#container1=/Account/Dashboard&id=1"

Expression:
"Route/App#container1=/{controller}/{action}:&id={id}:"

Expected:
controller => Account
action => Dashboard
id => 1

Result:
controller => Account
action => Dashboard&id=1
id => null

how can we contribute to fixing it with a right aproach?

@millermedeiros
Copy link
Owner

@olavorn your expression should really be Route/App#container1=/{controller}/{action}:?query: - :&id={id}: is not a valid segment. and change the path to "Route/App#container1=/Account/Dashboard?id=1"

another option is to use the rest segment ({action*}) and parse the ID by yourself and/or use the route.rules.normalize_

@olavorn
Copy link

olavorn commented Aug 25, 2015

Hi @millermedeiros!!

as I stated, I understand the workarounds, but, as I'm using Ben Alman's jQuery BBQ (which is also awesome) to manage the hash variables. the &id part is not about the url itself, but is a separate variable. see:

"Route/App#container1=/Account/Dashboard&id=1" is built as
$.bbq.pushState({ container1 : '/Account/Dashboard', id: 1 } )

$.bbq.getState('container1') //returns "/Account/Dashboard"
$.bbq.getState('id') //returns 1

Agreed that '/Account/Dashboard&id=1' is not valid as url, but that is not the case, as you can see. They are diferent variables within hash section and there is no reason the parser should not understand that as so.

@rg1
Copy link

rg1 commented Aug 9, 2016

I'm hit with the same problem. The solution I found is to change the match for the OR (and RR) token from greedy to non-greedy. Do you understand what I mean or should I submit a pull request?

@Yogeshberia
Copy link

Also, in the first case, if the optional "path" parameter doesn't have a rest segment and is omitted, the query param can't be used without using a preceding slash (/).

Doesn't Work: one/two?id=1&test=1
Works: one/two/?id=1&test=1

guillegr123 added a commit to 2600hz/monster-ui that referenced this issue Jun 6, 2022
Use RegExp instead of pattern for app route, because Crossroads.js
extracts the query string as part of the rest segment. This is because
it does not support having both segments as optional.
See: millermedeiros/crossroads.js#130
guillegr123 added a commit to 2600hz/monster-ui that referenced this issue Jun 6, 2022
Use RegExp instead of pattern for app route, because Crossroads.js
extracts the query string as part of the rest segment. This is because
it does not support having both segments as optional.
See: millermedeiros/crossroads.js#130
guillegr123 added a commit to 2600hz/monster-ui that referenced this issue Jun 6, 2022
Use RegExp instead of pattern for app route, because Crossroads.js
extracts the query string as part of the rest segment. This is because
it does not support having both segments as optional.
See: millermedeiros/crossroads.js#130
guillegr123 added a commit to 2600hz/monster-ui that referenced this issue Jun 6, 2022
Use RegExp instead of pattern for app route, because Crossroads.js
extracts the query string as part of the rest segment. This is because
it does not support having both segments as optional.
See: millermedeiros/crossroads.js#130
guillegr123 added a commit to 2600hz/monster-ui that referenced this issue Jun 17, 2022
Use RegExp instead of pattern for app route, because Crossroads.js
extracts the query string as part of the rest segment. This is because
it does not support having both segments as optional.
See: millermedeiros/crossroads.js#130
guillegr123 added a commit to 2600hz/monster-ui that referenced this issue Jun 19, 2022
Use RegExp instead of pattern for app route, because Crossroads.js
extracts the query string as part of the rest segment. This is because
it does not support having both segments as optional.
See: millermedeiros/crossroads.js#130
joristirado pushed a commit to 2600hz/monster-ui that referenced this issue Jun 21, 2022
* Use RegExp to handle optional rest and query segments

Use RegExp instead of pattern for app route, because Crossroads.js
extracts the query string as part of the rest segment. This is because
it does not support having both segments as optional.
See: millermedeiros/crossroads.js#130

* Expose utility function parseQueryString

* Parse route query string into an object

* Reload app only if there is no rest segment

* Restrict app name capturing group to expected pattern

* Use variables to check custom routing condition
joristirado pushed a commit to 2600hz/monster-ui that referenced this issue Jun 21, 2022
* Use RegExp to handle optional rest and query segments

Use RegExp instead of pattern for app route, because Crossroads.js
extracts the query string as part of the rest segment. This is because
it does not support having both segments as optional.
See: millermedeiros/crossroads.js#130

* Expose utility function parseQueryString

* Parse route query string into an object

* Reload app only if there is no rest segment

* Restrict app name capturing group to expected pattern

* Use variables to check custom routing condition
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants