Query Params and Route matching #176

Open
kamrenz opened this Issue Mar 14, 2015 · 11 comments

Comments

Projects
None yet
5 participants
@kamrenz

kamrenz commented Mar 14, 2015

Is there a way for us to specify in our router.config query parameters?
I would like to hit something like the following URL

#/information/42/wisdom?isUser='car'

If I map my router like the following my information controller get's triggered, but the $routeParams only has {id: 42, type: 'wisdom'}. No isUser property is in the object.

  $router.config([
    { path: '/information/:id/:type', component: 'information' }
  ]);

If I map my router like the following my information controller get's triggered, but the $routeParams has {id: "42", type?isUser: "wisdom"}. The type and isUser properties are not separated and still no query parameter

  $router.config([
    { path: '/information/:id/:type?isUser', component: 'information' }    
  ]);

If I map my router like the following my information controller doesn't get triggered.

  $router.config([
    { path: '/information/:id/:type?isUser', component: 'information' }    
  ]);
@timkindberg

This comment has been minimized.

Show comment
Hide comment
@timkindberg

timkindberg Mar 15, 2015

Doesn't seem supported yet. But I think it's essential that query params are either put into $routeParams or that $location.search() changes triggers view updates.

Right now I have to do this:

home.js

function HomeController($routeParams, $location, $scope){
  var self = this;
  this.params = $routeParams;

  // set query when coming to this route anew
  refreshQuery();
  function refreshQuery(){
    self.query = $location.search();
  }
  // set query when only the $location.search value changes, also deregister on destroy
  $scope.$on("$destroy", $scope.$on("$locationChangeSuccess",refreshQuery));
}

home.html

<h2>Home</h2>
<p>{{ home.params | json }}</p>
<p>{{ home.query | json }}</p>

In index.html

<a href="#/home">Go Home</a>
<a href="#/home?foo=5&bar=10">Go Home with Query</a>
<a href="#/new">Go New</a>

When you go from '/home' to '/home?foo=5...' the controller is not reloaded, so that's why I need to listen for $locationChangeSuccess.

If we don't have built in support, everyone is going to have to add some custom code like this and that seems unnecessary.

Doesn't seem supported yet. But I think it's essential that query params are either put into $routeParams or that $location.search() changes triggers view updates.

Right now I have to do this:

home.js

function HomeController($routeParams, $location, $scope){
  var self = this;
  this.params = $routeParams;

  // set query when coming to this route anew
  refreshQuery();
  function refreshQuery(){
    self.query = $location.search();
  }
  // set query when only the $location.search value changes, also deregister on destroy
  $scope.$on("$destroy", $scope.$on("$locationChangeSuccess",refreshQuery));
}

home.html

<h2>Home</h2>
<p>{{ home.params | json }}</p>
<p>{{ home.query | json }}</p>

In index.html

<a href="#/home">Go Home</a>
<a href="#/home?foo=5&bar=10">Go Home with Query</a>
<a href="#/new">Go New</a>

When you go from '/home' to '/home?foo=5...' the controller is not reloaded, so that's why I need to listen for $locationChangeSuccess.

If we don't have built in support, everyone is going to have to add some custom code like this and that seems unnecessary.

@timkindberg

This comment has been minimized.

Show comment
Hide comment
@timkindberg

timkindberg Mar 15, 2015

Route recognizer is definitely solid in its support for query params. https://github.com/tildeio/route-recognizer/blob/master/tests/recognizer-tests.js

Hopefully that means adding support will be easier.

Route recognizer is definitely solid in its support for query params. https://github.com/tildeio/route-recognizer/blob/master/tests/recognizer-tests.js

Hopefully that means adding support will be easier.

@SamVerschueren

This comment has been minimized.

Show comment
Hide comment
@SamVerschueren

SamVerschueren Mar 15, 2015

I had a look into this and actually it is possible and it isn't possible at the same time.

With the ng-link directive, you can specify your query string parameters.

<a ng-link="home">Go Home</a>
<a ng-link="home({queryParams: {foo: 5, bar:10}})">Go Home With Query</a>

The link is parsed by Route Recognizer and they use the queryParams property to add the query parameters.

But if you do this, it will result in your html as home?foo=5&bar=10, but when you click on it, the question mark is URL encoded wich results in home%3Ffoo=5&bar=10. I tracked down the reason why it got url encoded and found that in dist/router.es5.js, the URL is changed by using $location.path(). You can only change the path with it and not the query params. This should be changed to $location.url() and then it works.

I'll add a pull request for this issue. Not sure if it will be accepted. Maybe there is a reason why they want it to be with $location.path() and they want to handle the query params somewhere else.

I had a look into this and actually it is possible and it isn't possible at the same time.

With the ng-link directive, you can specify your query string parameters.

<a ng-link="home">Go Home</a>
<a ng-link="home({queryParams: {foo: 5, bar:10}})">Go Home With Query</a>

The link is parsed by Route Recognizer and they use the queryParams property to add the query parameters.

But if you do this, it will result in your html as home?foo=5&bar=10, but when you click on it, the question mark is URL encoded wich results in home%3Ffoo=5&bar=10. I tracked down the reason why it got url encoded and found that in dist/router.es5.js, the URL is changed by using $location.path(). You can only change the path with it and not the query params. This should be changed to $location.url() and then it works.

I'll add a pull request for this issue. Not sure if it will be accepted. Maybe there is a reason why they want it to be with $location.path() and they want to handle the query params somewhere else.

@btford

This comment has been minimized.

Show comment
Hide comment
@btford

btford Mar 19, 2015

Contributor

I've been thinking about this as well, but I don't feel like I have an especially strong feeling of the best way to solve it. I'm totally open to suggestions though.

Contributor

btford commented Mar 19, 2015

I've been thinking about this as well, but I don't feel like I have an especially strong feeling of the best way to solve it. I'm totally open to suggestions though.

@SamVerschueren

This comment has been minimized.

Show comment
Hide comment
@SamVerschueren

SamVerschueren Mar 19, 2015

Tried to fix it in router.es5.js by using location.url() instead of location.path(). At first it seemed to work when adding the queryParams property in the ng-link directive.

<a ng-link="home({queryParams: {foo: 5, bar:10}})">Go Home With Query</a>

But when I wrote tests for this, calling router.navigate('/home?foo=5&bar=10'), it didn't work. I get really strange behaviour.

This is what I wrote as a test

  it('should work with querystrings', inject(function ($router) {
    $router.config([
      { path: '/user/:name', component: 'userAge' }
    ]);
    compile('<ng-viewport></ng-viewport>');

    $router.navigate('/user/brian?age=10');
    $rootScope.$digest();
    expect(elt.text()).toBe('hello brian (10)');

    $router.navigate('/user/igor');
    $rootScope.$digest();
    expect(elt.text()).toBe('hello igor (unknown)');
  }));

This is the result of running that test

Chrome 41.0.2272 (Mac OS X 10.10.2) ngViewport should work with querystrings FAILED
    Expected 'hello brian (unknown)' to be 'hello brian (10)'.
    Error: Expected 'hello brian (unknown)' to be 'hello brian (10)'.
        at Object.<anonymous> (/Users/sam/Documents/Projects/opensource/router/test/router-viewport.es5.spec.js:64:24)
        at Object.invoke (/Users/sam/Documents/Projects/opensource/router/node_modules/angular/angular.js:4185:17)
        at Object.workFn (/Users/sam/Documents/Projects/opensource/router/node_modules/angular-mocks/angular-mocks.js:2364:20)
    Expected 'hello igor (10)' to be 'hello igor (unknown)'.
    Error: Expected 'hello igor (10)' to be 'hello igor (unknown)'.
        at Object.<anonymous> (/Users/sam/Documents/Projects/opensource/router/test/router-viewport.es5.spec.js:68:24)
        at Object.invoke (/Users/sam/Documents/Projects/opensource/router/node_modules/angular/angular.js:4185:17)
        at Object.workFn (/Users/sam/Documents/Projects/opensource/router/node_modules/angular-mocks/angular-mocks.js:2364:20)
Chrome 41.0.2272 (Mac OS X 10.10.2): Executed 21 of 21 (1 FAILED) (0.215 secs / 0.207 secs)

So although I pass the querystring to the controller when I use Brian, the controller doesn't receive it. But when navigating away to igor (with no queryparams), then the params are being received by the controller.

Didn't have time to look more into this behaviour.

Tried to fix it in router.es5.js by using location.url() instead of location.path(). At first it seemed to work when adding the queryParams property in the ng-link directive.

<a ng-link="home({queryParams: {foo: 5, bar:10}})">Go Home With Query</a>

But when I wrote tests for this, calling router.navigate('/home?foo=5&bar=10'), it didn't work. I get really strange behaviour.

This is what I wrote as a test

  it('should work with querystrings', inject(function ($router) {
    $router.config([
      { path: '/user/:name', component: 'userAge' }
    ]);
    compile('<ng-viewport></ng-viewport>');

    $router.navigate('/user/brian?age=10');
    $rootScope.$digest();
    expect(elt.text()).toBe('hello brian (10)');

    $router.navigate('/user/igor');
    $rootScope.$digest();
    expect(elt.text()).toBe('hello igor (unknown)');
  }));

This is the result of running that test

Chrome 41.0.2272 (Mac OS X 10.10.2) ngViewport should work with querystrings FAILED
    Expected 'hello brian (unknown)' to be 'hello brian (10)'.
    Error: Expected 'hello brian (unknown)' to be 'hello brian (10)'.
        at Object.<anonymous> (/Users/sam/Documents/Projects/opensource/router/test/router-viewport.es5.spec.js:64:24)
        at Object.invoke (/Users/sam/Documents/Projects/opensource/router/node_modules/angular/angular.js:4185:17)
        at Object.workFn (/Users/sam/Documents/Projects/opensource/router/node_modules/angular-mocks/angular-mocks.js:2364:20)
    Expected 'hello igor (10)' to be 'hello igor (unknown)'.
    Error: Expected 'hello igor (10)' to be 'hello igor (unknown)'.
        at Object.<anonymous> (/Users/sam/Documents/Projects/opensource/router/test/router-viewport.es5.spec.js:68:24)
        at Object.invoke (/Users/sam/Documents/Projects/opensource/router/node_modules/angular/angular.js:4185:17)
        at Object.workFn (/Users/sam/Documents/Projects/opensource/router/node_modules/angular-mocks/angular-mocks.js:2364:20)
Chrome 41.0.2272 (Mac OS X 10.10.2): Executed 21 of 21 (1 FAILED) (0.215 secs / 0.207 secs)

So although I pass the querystring to the controller when I use Brian, the controller doesn't receive it. But when navigating away to igor (with no queryparams), then the params are being received by the controller.

Didn't have time to look more into this behaviour.

@kamrenz

This comment has been minimized.

Show comment
Hide comment
@kamrenz

kamrenz Mar 26, 2015

@btford I think the ui-router had a simple enough way of dealing with query parameters https://github.com/angular-ui/ui-router/wiki/URL-Routing. It allows us to specify query parameters directly in the URL path.

$router.config([
    { path: '/information/:id/:type?isUser', component: 'information' }    
  ]);

To interact with the parameters then I could utilize $routeParams to grab the parameter as:

$routeParams.isUser

If we want to pass query parameters I like the simplicity of ui-router also. It doesn't make me specify queryParams like @SamVerschueren pointed out. Instead, it is smart enough to know if they are query parameters or URL parameters:

<li>
  <a ng-link=“information({id:42, type:’wisdom’, isUser:false})“ >       
    Information</a>
</li>   

& programmatically:

$router.navigate('/information/42/wisdom?isUser=false');
//This could be an interesting solution also
//$router.navigate('/information', {id: 42, type:'wisdom', isUser: false}); 

kamrenz commented Mar 26, 2015

@btford I think the ui-router had a simple enough way of dealing with query parameters https://github.com/angular-ui/ui-router/wiki/URL-Routing. It allows us to specify query parameters directly in the URL path.

$router.config([
    { path: '/information/:id/:type?isUser', component: 'information' }    
  ]);

To interact with the parameters then I could utilize $routeParams to grab the parameter as:

$routeParams.isUser

If we want to pass query parameters I like the simplicity of ui-router also. It doesn't make me specify queryParams like @SamVerschueren pointed out. Instead, it is smart enough to know if they are query parameters or URL parameters:

<li>
  <a ng-link=“information({id:42, type:’wisdom’, isUser:false})“ >       
    Information</a>
</li>   

& programmatically:

$router.navigate('/information/42/wisdom?isUser=false');
//This could be an interesting solution also
//$router.navigate('/information', {id: 42, type:'wisdom', isUser: false}); 
@SamVerschueren

This comment has been minimized.

Show comment
Hide comment
@SamVerschueren

SamVerschueren Mar 27, 2015

The new router uses Route Recognizer for this so I guess we should follow their rules... It is that library that forces you to add the queryParams property.

The new router uses Route Recognizer for this so I guess we should follow their rules... It is that library that forces you to add the queryParams property.

@niemyjski

This comment has been minimized.

Show comment
Hide comment
@niemyjski

niemyjski Mar 30, 2015

I don't like that you have to specify query params:

<a ng-link="home({queryParams: {foo: 5, bar:10}})">Go Home With Query</a>

@btford Why can't we just pass the object as we do in ui router. I get that this change will make it easier to build up query params as you don't have to look at the route to determine which properties are actually part of the route vs query parameters?

I don't like that you have to specify query params:

<a ng-link="home({queryParams: {foo: 5, bar:10}})">Go Home With Query</a>

@btford Why can't we just pass the object as we do in ui router. I get that this change will make it easier to build up query params as you don't have to look at the route to determine which properties are actually part of the route vs query parameters?

@timkindberg

This comment has been minimized.

Show comment
Hide comment
@timkindberg

timkindberg Mar 30, 2015

@niemyjski good on you for writing up such a detailed migration experience!! 👏 👍 That will be incredibly valuable to @btford and the rest of the new router team!

I also agree that I'd rather just pass query params along with the other params and not on a special queryParams object, if possible.

@niemyjski good on you for writing up such a detailed migration experience!! 👏 👍 That will be incredibly valuable to @btford and the rest of the new router team!

I also agree that I'd rather just pass query params along with the other params and not on a special queryParams object, if possible.

@SamVerschueren

This comment has been minimized.

Show comment
Hide comment
@SamVerschueren

SamVerschueren Mar 31, 2015

@niemyjski My fix works for the ng-link directive. But when I tried to create tests for that functionality by calling $router.navigate() directly with query parameters, I really got weird results. So this needs further investigation. I will investigate if I find more time.

Passing query params directly instead of via the queryParams property should be changed in Route Recognizer as that one is used to parse the routes. Don't think this change can be made on the routers side.

@niemyjski My fix works for the ng-link directive. But when I tried to create tests for that functionality by calling $router.navigate() directly with query parameters, I really got weird results. So this needs further investigation. I will investigate if I find more time.

Passing query params directly instead of via the queryParams property should be changed in Route Recognizer as that one is used to parse the routes. Don't think this change can be made on the routers side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment