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

Support optional route segments #83

Closed
bryanrsmith opened this Issue May 5, 2015 · 39 comments

Comments

Projects
None yet
@bryanrsmith
Member

bryanrsmith commented May 5, 2015

No description provided.

@jedd-ahyoung

This comment has been minimized.

Show comment
Hide comment
@jedd-ahyoung

jedd-ahyoung May 5, 2015

Member

Mentioned this in the Aurelia gitter. For what it's worth, at the moment, you can pass an array of routes, like the following:

configureRouter(config, router){
    config.title = 'Courses';
    config.map([
        // { route: '', moduleId: ''}
        { route: '', moduleId: 'no-selection', title: 'Select'},
        { route: ['courses', 'courses/:id'],  moduleId: 'course-detail' }
    ]);

    this.router = router;
}

route-href will not allow you to omit the parameter or pass null in this case. However, it seems that, at the moment, passing an empty string to the route-href param bypasses the issue. (I don't know how well this works with more complex routes - for instance, a route where the parameter isn't the final segment in the route).

Member

jedd-ahyoung commented May 5, 2015

Mentioned this in the Aurelia gitter. For what it's worth, at the moment, you can pass an array of routes, like the following:

configureRouter(config, router){
    config.title = 'Courses';
    config.map([
        // { route: '', moduleId: ''}
        { route: '', moduleId: 'no-selection', title: 'Select'},
        { route: ['courses', 'courses/:id'],  moduleId: 'course-detail' }
    ]);

    this.router = router;
}

route-href will not allow you to omit the parameter or pass null in this case. However, it seems that, at the moment, passing an empty string to the route-href param bypasses the issue. (I don't know how well this works with more complex routes - for instance, a route where the parameter isn't the final segment in the route).

@Aaike

This comment has been minimized.

Show comment
Hide comment
@Aaike

Aaike May 21, 2015

Member

👍
The optional segments would really help with the i18n plugin. it's the last thing we need to finish it.
What i would like to do is prefix a segment to every route.
So depending on the current language of the i18n plugin i would get en/route or de/route when navigating to route.

Member

Aaike commented May 21, 2015

👍
The optional segments would really help with the i18n plugin. it's the last thing we need to finish it.
What i would like to do is prefix a segment to every route.
So depending on the current language of the i18n plugin i would get en/route or de/route when navigating to route.

@bfil

This comment has been minimized.

Show comment
Hide comment
@bfil

bfil May 29, 2015

Contributor

Another issue in this regards to keep into consideration is that when you navigate from (using Jedd's example) from /courses/:id to courses the url changes but it doesn't trigger a full navigation so it does not execute the view model logic again for courses.

This ended up being an issue when I was trying to show different things on the view based on the presence of the segment, navigating between the 2 routes breaks the use case.

UPDATE
I actually gave it a go in trying to fix the issue above, and opened a PR that fixes it #115

Contributor

bfil commented May 29, 2015

Another issue in this regards to keep into consideration is that when you navigate from (using Jedd's example) from /courses/:id to courses the url changes but it doesn't trigger a full navigation so it does not execute the view model logic again for courses.

This ended up being an issue when I was trying to show different things on the view based on the presence of the segment, navigating between the 2 routes breaks the use case.

UPDATE
I actually gave it a go in trying to fix the issue above, and opened a PR that fixes it #115

@Aaike

This comment has been minimized.

Show comment
Hide comment
@Aaike

Aaike Jun 15, 2015

Member

has there been any progress around this ? it would be really great to be able to use it :)

Member

Aaike commented Jun 15, 2015

has there been any progress around this ? it would be really great to be able to use it :)

@davismj

This comment has been minimized.

Show comment
Hide comment
@davismj

davismj Aug 13, 2015

Member

@bfil i read through your pr but i don't see anything about optional route segments. can you give an example?

Member

davismj commented Aug 13, 2015

@bfil i read through your pr but i don't see anything about optional route segments. can you give an example?

@bfil

This comment has been minimized.

Show comment
Hide comment
@bfil

bfil Aug 13, 2015

Contributor

@davismj the issue the PR fixed was the navigation between /route/:id and /route (which was not triggering the right logic), it did not implement anything in relation to optional routes, just fixed the router issue.

By the way for me using route: ['/route', '/route/:id'] works just fine in my app (but I'm not using route-href or any other fancy routing mechanics).

Contributor

bfil commented Aug 13, 2015

@davismj the issue the PR fixed was the navigation between /route/:id and /route (which was not triggering the right logic), it did not implement anything in relation to optional routes, just fixed the router issue.

By the way for me using route: ['/route', '/route/:id'] works just fine in my app (but I'm not using route-href or any other fancy routing mechanics).

@Secbone

This comment has been minimized.

Show comment
Hide comment
@Secbone

Secbone Sep 16, 2015

if I use route like this: ['/route', '/route/:id'] , can I use route-href without params.bind ? like this: route-href="route: route" ?

Secbone commented Sep 16, 2015

if I use route like this: ['/route', '/route/:id'] , can I use route-href without params.bind ? like this: route-href="route: route" ?

@jedd-ahyoung

This comment has been minimized.

Show comment
Hide comment
@jedd-ahyoung

jedd-ahyoung Nov 13, 2015

Member

Checking into this. As before, I'm running into issues where a route defined like this

let routes = [{ route: ['dashboard', 'dashboard/:companyId'], name: 'dashboard', moduleId: 'dashboard', nav: true, title:'Dashboard' }];

cannot be called like this:

this.router.navigateToRoute('dashboard');

I receive this error.

DEBUG [App] Login failed:  Error: A value is required for route parameter 'companyId' in route 'dashboard'.(…)

Changing the order of the routes in the array actually does seem to have an effect on this, as I get a different error if I reverse the order. I guess Aurelia scans the route table in order.

Member

jedd-ahyoung commented Nov 13, 2015

Checking into this. As before, I'm running into issues where a route defined like this

let routes = [{ route: ['dashboard', 'dashboard/:companyId'], name: 'dashboard', moduleId: 'dashboard', nav: true, title:'Dashboard' }];

cannot be called like this:

this.router.navigateToRoute('dashboard');

I receive this error.

DEBUG [App] Login failed:  Error: A value is required for route parameter 'companyId' in route 'dashboard'.(…)

Changing the order of the routes in the array actually does seem to have an effect on this, as I get a different error if I reverse the order. I guess Aurelia scans the route table in order.

@glen-84

This comment has been minimized.

Show comment
Hide comment
@glen-84

glen-84 Nov 22, 2015

@EisenbergEffect Have you considered using route definitions similar to those used in ASP.NET 5 attribute routing?

So instead of /article/:id you'd use /article/{id}, and you could then support features like route constraints (/article/{id:int}), optional parameters (/article/{id:int?}), and default parameter values (/article/{id:int=99}).

glen-84 commented Nov 22, 2015

@EisenbergEffect Have you considered using route definitions similar to those used in ASP.NET 5 attribute routing?

So instead of /article/:id you'd use /article/{id}, and you could then support features like route constraints (/article/{id:int}), optional parameters (/article/{id:int?}), and default parameter values (/article/{id:int=99}).

@davismj

This comment has been minimized.

Show comment
Hide comment
@davismj

davismj Nov 22, 2015

Member

That's actually a great idea, especially since it allows better flexibility and strengthens the familiarity MVC developers will have with Aurelia, which is a path we've been treading thus far. +1

Member

davismj commented Nov 22, 2015

That's actually a great idea, especially since it allows better flexibility and strengthens the familiarity MVC developers will have with Aurelia, which is a path we've been treading thus far. +1

@StrahilKazlachev

This comment has been minimized.

Show comment
Hide comment
@StrahilKazlachev

StrahilKazlachev Nov 22, 2015

Contributor

Quite usable feature/improvement +1. But shouldn't this be in the route-recognizer?

Contributor

StrahilKazlachev commented Nov 22, 2015

Quite usable feature/improvement +1. But shouldn't this be in the route-recognizer?

@CasiOo

This comment has been minimized.

Show comment
Hide comment
@CasiOo

CasiOo Nov 22, 2015

+1 @glen-84, seems to be a great idea

CasiOo commented Nov 22, 2015

+1 @glen-84, seems to be a great idea

@bryanrsmith

This comment has been minimized.

Show comment
Hide comment
@bryanrsmith

bryanrsmith Nov 22, 2015

Member

Unfortunately, we're too far along to consider changing the routing syntax. Changing it would be very disruptive to existing apps, and supporting both syntaxes would complicate code and cause confusion in docs and examples. We can (and intend to) support optional segments, default parameters, and route constraints with the current syntax.

Happy to discuss further, but it'd be great to do so in a separate issue.

Member

bryanrsmith commented Nov 22, 2015

Unfortunately, we're too far along to consider changing the routing syntax. Changing it would be very disruptive to existing apps, and supporting both syntaxes would complicate code and cause confusion in docs and examples. We can (and intend to) support optional segments, default parameters, and route constraints with the current syntax.

Happy to discuss further, but it'd be great to do so in a separate issue.

@glen-84

This comment has been minimized.

Show comment
Hide comment
@glen-84

glen-84 Nov 22, 2015

@bryanrsmith,

That's unfortunate. I thought that it may still be possible as the package is still marked as pre-release and it would be relatively easy to create a migration tool.

Anyway, what about:

/article/:id{int}/edit (constraint)
/article/:id{int?}/edit (optional)
/article/:id{int=99}/edit (default)

Or:

/article/:id(int)/edit (constraint)
/article/:id(int?)/edit (optional)
/article/:id(int=99)/edit (default)

I guess there are quite a few options for the syntax, and we best leave this for you and Rob to decide on. =)

glen-84 commented Nov 22, 2015

@bryanrsmith,

That's unfortunate. I thought that it may still be possible as the package is still marked as pre-release and it would be relatively easy to create a migration tool.

Anyway, what about:

/article/:id{int}/edit (constraint)
/article/:id{int?}/edit (optional)
/article/:id{int=99}/edit (default)

Or:

/article/:id(int)/edit (constraint)
/article/:id(int?)/edit (optional)
/article/:id(int=99)/edit (default)

I guess there are quite a few options for the syntax, and we best leave this for you and Rob to decide on. =)

@davismj

This comment has been minimized.

Show comment
Hide comment
@davismj

davismj Nov 24, 2015

Member

I'd suggest staying in sync with MVC, even if that means waiting until a later version.

Member

davismj commented Nov 24, 2015

I'd suggest staying in sync with MVC, even if that means waiting until a later version.

@johnmanko

This comment has been minimized.

Show comment
Hide comment
@johnmanko

johnmanko Mar 8, 2016

optional and with regex patterns. :)

johnmanko commented Mar 8, 2016

optional and with regex patterns. :)

@jods4

This comment has been minimized.

Show comment
Hide comment
@jods4

jods4 Jul 6, 2016

Contributor

Would it be so bad to support both :id (without optional, types, etc.) and {id} (for full support) syntaxes? Seems doable to me, benefits of not having to learn a new syntax is big.

BTW: +1 for this issue, I just had another bug #366 happen to me because we have separated routes for optional parameters :(

Contributor

jods4 commented Jul 6, 2016

Would it be so bad to support both :id (without optional, types, etc.) and {id} (for full support) syntaxes? Seems doable to me, benefits of not having to learn a new syntax is big.

BTW: +1 for this issue, I just had another bug #366 happen to me because we have separated routes for optional parameters :(

@jods4

This comment has been minimized.

Show comment
Hide comment
@jods4

jods4 Aug 4, 2016

Contributor

I did a PR for this. Any opinion?
I would like optional parameters to land because it will remove a few pain points for us.

I could port this to the old syntax if you prefer: /user/:id?.
Defaults and constraints can probably easily be supported there as well, maybe with alternative syntax: /user/:id=42 and /user/:id|int/:phone|regex(\d{3}-\d{2}-\d{2})

The one thing that is harder to introduce with the current syntax is allowing parameters to appear as sub-parts of a segment like /product-{id}-overview. But again, creativity could allow it: /product-:id:-overview

Contributor

jods4 commented Aug 4, 2016

I did a PR for this. Any opinion?
I would like optional parameters to land because it will remove a few pain points for us.

I could port this to the old syntax if you prefer: /user/:id?.
Defaults and constraints can probably easily be supported there as well, maybe with alternative syntax: /user/:id=42 and /user/:id|int/:phone|regex(\d{3}-\d{2}-\d{2})

The one thing that is harder to introduce with the current syntax is allowing parameters to appear as sub-parts of a segment like /product-{id}-overview. But again, creativity could allow it: /product-:id:-overview

@EisenbergEffect

This comment has been minimized.

Show comment
Hide comment
@EisenbergEffect

EisenbergEffect Aug 4, 2016

Member

I think I prefer the syntax from the pr, especially since it doesn't break anything we currently have. We haven't had an opportunity to review it yet. It will take a bit of time since it is a significant feature addition. We need to consider it carefully.

Member

EisenbergEffect commented Aug 4, 2016

I think I prefer the syntax from the pr, especially since it doesn't break anything we currently have. We haven't had an opportunity to review it yet. It will take a bit of time since it is a significant feature addition. We need to consider it carefully.

@jods4

This comment has been minimized.

Show comment
Hide comment
@jods4

jods4 Aug 4, 2016

Contributor

@EisenbergEffect I have been pondering about the breaking potential and there is one thing:

People could have static routes that look like /folder/{1234-56-7890}. It is not likely, but possible.
In that case it will continue to work (unless there is a ?, = or : inside that segment, which is even less likely) albeit in a slightly different way: the segment becomes a parameter named 1234-56-7890 and it will receive the value {1234-56-7890}.

But the nasty side effect is that many other routes will match that pattern. In fact if there were other static routes like /folder/{777-77-777} then it's pretty much guaranteed that routing is broken. And routes like /folder/zzz will start to match.

Not sure if this is unlikely enough to be ok or not. One option could be to hide the new syntax behind an opt-in switch but I don't like that as over time this approach becomes a ton of switches that people have to know/define when they start a new project (I'm looking at you Typescript).

A related aspect is that I did not include any support for escaping, which means that even with a heads-up in the release notes, the routes above currently can't be restored to their old behavior. The asp.net way would be /folder/{{1234-56-7890}} (doubling the braces) but I did not include that in the PR.

Contributor

jods4 commented Aug 4, 2016

@EisenbergEffect I have been pondering about the breaking potential and there is one thing:

People could have static routes that look like /folder/{1234-56-7890}. It is not likely, but possible.
In that case it will continue to work (unless there is a ?, = or : inside that segment, which is even less likely) albeit in a slightly different way: the segment becomes a parameter named 1234-56-7890 and it will receive the value {1234-56-7890}.

But the nasty side effect is that many other routes will match that pattern. In fact if there were other static routes like /folder/{777-77-777} then it's pretty much guaranteed that routing is broken. And routes like /folder/zzz will start to match.

Not sure if this is unlikely enough to be ok or not. One option could be to hide the new syntax behind an opt-in switch but I don't like that as over time this approach becomes a ton of switches that people have to know/define when they start a new project (I'm looking at you Typescript).

A related aspect is that I did not include any support for escaping, which means that even with a heads-up in the release notes, the routes above currently can't be restored to their old behavior. The asp.net way would be /folder/{{1234-56-7890}} (doubling the braces) but I did not include that in the PR.

@EisenbergEffect

This comment has been minimized.

Show comment
Hide comment
@EisenbergEffect

EisenbergEffect Aug 5, 2016

Member

I think this is probably very rare to non-existent. What we could do is blog about this to see if anyone has concerns. If they do, we could add some sort of switch.

Member

EisenbergEffect commented Aug 5, 2016

I think this is probably very rare to non-existent. What we could do is blog about this to see if anyone has concerns. If they do, we could add some sort of switch.

@JoClaes

This comment has been minimized.

Show comment
Hide comment
@JoClaes

JoClaes Sep 4, 2016

Don't know if it has anything to do with this but i have a weird problem.
I have a following route config :
{ route: ["Relation", "Relation/:Id"], moduleId: "modules/relation/relation", title: "Rels", nav: true }
If i don't provide an id a search form appears and when i provide an id a form with data appears.
When I navigate to a #/Relation/1 the data is loaded , when i then navigate back #/Relation the search form appears and is removed after a sec. When i reload the app and navigate to #/Relation the search appears again and stays on the screen.

this is a snippet of my template
<search module="Relation" show.bind='!(Data.Id.length>0)'></search>
<dataform module="Relation" id=(Data.Id) show.bind='(Data.Id.length>0)'></dataform>

Any suggestions ?

JoClaes commented Sep 4, 2016

Don't know if it has anything to do with this but i have a weird problem.
I have a following route config :
{ route: ["Relation", "Relation/:Id"], moduleId: "modules/relation/relation", title: "Rels", nav: true }
If i don't provide an id a search form appears and when i provide an id a form with data appears.
When I navigate to a #/Relation/1 the data is loaded , when i then navigate back #/Relation the search form appears and is removed after a sec. When i reload the app and navigate to #/Relation the search appears again and stays on the screen.

this is a snippet of my template
<search module="Relation" show.bind='!(Data.Id.length>0)'></search>
<dataform module="Relation" id=(Data.Id) show.bind='(Data.Id.length>0)'></dataform>

Any suggestions ?

@jods4

This comment has been minimized.

Show comment
Hide comment
@jods4

jods4 Sep 5, 2016

Contributor

@JoClaes I think it might be related but I'm not sure. In my experiments before creating this PR, I noticed that Aurelia's route generation doesn't have logic to look for the good route when multiple routes are available for the same name, with varying parameters.

In other words, route: ["Relation", "Relation/:id"] is not a workaround for the lack of optional parameters. And I discourage you to use that. Until this PR is merged you should create two different routes with different names.

Contributor

jods4 commented Sep 5, 2016

@JoClaes I think it might be related but I'm not sure. In my experiments before creating this PR, I noticed that Aurelia's route generation doesn't have logic to look for the good route when multiple routes are available for the same name, with varying parameters.

In other words, route: ["Relation", "Relation/:id"] is not a workaround for the lack of optional parameters. And I discourage you to use that. Until this PR is merged you should create two different routes with different names.

@WilliamT-Streamba

This comment has been minimized.

Show comment
Hide comment
@WilliamT-Streamba

WilliamT-Streamba Sep 6, 2016

@jods4 Totally agree, defining routes like route: ["Relation", "Relation/:id"] seems like a workaround at first glance but just causes major problems and not very maintainable code. @JoClaes best option is to define two routes { route: "Relation, name: "relation"}, { route: "Relation/:id", name : "relation-with-id"}

WilliamT-Streamba commented Sep 6, 2016

@jods4 Totally agree, defining routes like route: ["Relation", "Relation/:id"] seems like a workaround at first glance but just causes major problems and not very maintainable code. @JoClaes best option is to define two routes { route: "Relation, name: "relation"}, { route: "Relation/:id", name : "relation-with-id"}

@EisenbergEffect

This comment has been minimized.

Show comment
Hide comment
@EisenbergEffect

EisenbergEffect Sep 6, 2016

Member

@jods4 For now, we want to stick with something that matches and works with our current syntax. In the future, I think we could consider adding a new syntax or changing it but I'd like to consider that separately. Can you update the associated PR to represent only the current syntax?

Member

EisenbergEffect commented Sep 6, 2016

@jods4 For now, we want to stick with something that matches and works with our current syntax. In the future, I think we could consider adding a new syntax or changing it but I'd like to consider that separately. Can you update the associated PR to represent only the current syntax?

@jods4

This comment has been minimized.

Show comment
Hide comment
@jods4

jods4 Sep 6, 2016

Contributor

@EisenbergEffect Yes, I really want to see this (optional parameters) happen.
So for now I can remove the alternate syntax and just support url/:id?. That's easily done.

Contributor

jods4 commented Sep 6, 2016

@EisenbergEffect Yes, I really want to see this (optional parameters) happen.
So for now I can remove the alternate syntax and just support url/:id?. That's easily done.

@EisenbergEffect

This comment has been minimized.

Show comment
Hide comment
@EisenbergEffect

EisenbergEffect Sep 6, 2016

Member

If you can do that and then provide a short bit of description for how selection between multiple matched routes is handled with optional parameters so I can have that info while evaluating your code, I think we can probably get this in pretty soon. I would certainly be happy to have this feature.

Member

EisenbergEffect commented Sep 6, 2016

If you can do that and then provide a short bit of description for how selection between multiple matched routes is handled with optional parameters so I can have that info while evaluating your code, I think we can probably get this in pretty soon. I would certainly be happy to have this feature.

@jods4

This comment has been minimized.

Show comment
Hide comment
@jods4

jods4 Sep 6, 2016

Contributor

@EisenbergEffect I just committed changes that remove the "constrained" syntax and adds support for optional parameters in the "dynamic" syntax.

Breaking changes:

  • ? is not allowed anywhere inside the parameter name. Doing so results in a static route. Example: :what_is?this.
  • ? is allowed as a trailing character. The meaning of :id? changes from name id? to name id + optionality.
  • = is not allowed anywhere after a :. Doing so results in an error being logged. Reason is that I want to reserve a potential default value syntax: :id=0.

To answer your question about priority between matching routes: I have changed nothing. Which means (as far as I understood the code):

  • During route matching, your existing logic of less stars, less dynamic, more static is untouched. An optional dynamic segment counts as +1 dynamic statically, regardless of whether it was matched at runtime or not.
  • An ambiguous route with multiple optional parameters should be considered as undefined behavior, I think. Nonetheless, I have a test case that assigns the optional parameters in left to right order. So matching /42 against /:x?/:y? will result in x: 42, y: undefined.
  • Route generation works the same as today. If there are multiple routes for the same name, the last (or was it first?) one is taken and applied as if it was the only one. This untouched from existing code base.
Contributor

jods4 commented Sep 6, 2016

@EisenbergEffect I just committed changes that remove the "constrained" syntax and adds support for optional parameters in the "dynamic" syntax.

Breaking changes:

  • ? is not allowed anywhere inside the parameter name. Doing so results in a static route. Example: :what_is?this.
  • ? is allowed as a trailing character. The meaning of :id? changes from name id? to name id + optionality.
  • = is not allowed anywhere after a :. Doing so results in an error being logged. Reason is that I want to reserve a potential default value syntax: :id=0.

To answer your question about priority between matching routes: I have changed nothing. Which means (as far as I understood the code):

  • During route matching, your existing logic of less stars, less dynamic, more static is untouched. An optional dynamic segment counts as +1 dynamic statically, regardless of whether it was matched at runtime or not.
  • An ambiguous route with multiple optional parameters should be considered as undefined behavior, I think. Nonetheless, I have a test case that assigns the optional parameters in left to right order. So matching /42 against /:x?/:y? will result in x: 42, y: undefined.
  • Route generation works the same as today. If there are multiple routes for the same name, the last (or was it first?) one is taken and applied as if it was the only one. This untouched from existing code base.
@EisenbergEffect

This comment has been minimized.

Show comment
Hide comment
@EisenbergEffect

EisenbergEffect Sep 6, 2016

Member

Is they any chance this would affect routes with query params?

Member

EisenbergEffect commented Sep 6, 2016

Is they any chance this would affect routes with query params?

@jods4

This comment has been minimized.

Show comment
Hide comment
@jods4

jods4 Sep 6, 2016

Contributor

@EisenbergEffect I don't think so, but I didn't think this through.

Given a pattern /user/:id?,

  • For matching, /user?id=42 will match (not by special logic but by virtue of optionality /user is ok). Now the question becomes: what is the id parameter value? I have not tested this, but I believe Aurelia sets query parameters after route matching, so the VM should see id: 42 I guess, which would seem ok to me.
  • Note that there is no special handling of query parameters during matching (no more than before), so this can't override the route resolution. /:x?/:y? will recognize /42?x=7, but it will end up with x: 7, y: undefined (I think). It will not see x assigned from query string and pass 42 to y. This is no different from the recognizer not accepting /user/:id with /user?id=1.
  • URL generation consumes any parameter in the route and leaves the rest in the query string, as before.
Contributor

jods4 commented Sep 6, 2016

@EisenbergEffect I don't think so, but I didn't think this through.

Given a pattern /user/:id?,

  • For matching, /user?id=42 will match (not by special logic but by virtue of optionality /user is ok). Now the question becomes: what is the id parameter value? I have not tested this, but I believe Aurelia sets query parameters after route matching, so the VM should see id: 42 I guess, which would seem ok to me.
  • Note that there is no special handling of query parameters during matching (no more than before), so this can't override the route resolution. /:x?/:y? will recognize /42?x=7, but it will end up with x: 7, y: undefined (I think). It will not see x assigned from query string and pass 42 to y. This is no different from the recognizer not accepting /user/:id with /user?id=1.
  • URL generation consumes any parameter in the route and leaves the rest in the query string, as before.
@EisenbergEffect

This comment has been minimized.

Show comment
Hide comment
@EisenbergEffect

EisenbergEffect Sep 6, 2016

Member

That all sounds good. I'll do a review of the code itself this week. Thanks for working on this. It's a huge deal. Any interest in working on other router features?

Member

EisenbergEffect commented Sep 6, 2016

That all sounds good. I'll do a review of the code itself this week. Thanks for working on this. It's a huge deal. Any interest in working on other router features?

@jods4

This comment has been minimized.

Show comment
Hide comment
@jods4

jods4 Sep 6, 2016

Contributor

@EisenbergEffect I would love to contribute more, but the honest answer is that I'm very busy at work so that doesn't leave me a lot of time to work on other projects. This is also the reason why I contribute mostly where it benefits us (e.g. optional parameters is something we really want to simplify some stuff we do).

Another area where I'd love to help because of personal interest is in building up the tooling. E.g. extending Typescript reach into bindings inside templates, enabling hot module reload, adding Aurelia as a first-class citizen in the upcoming ASP.NET SPA extensions... But those things are too big to tackle as side-projects. Would love to go to a company (Microsoft?) to do that if they were willing to hire me 😉

Contributor

jods4 commented Sep 6, 2016

@EisenbergEffect I would love to contribute more, but the honest answer is that I'm very busy at work so that doesn't leave me a lot of time to work on other projects. This is also the reason why I contribute mostly where it benefits us (e.g. optional parameters is something we really want to simplify some stuff we do).

Another area where I'd love to help because of personal interest is in building up the tooling. E.g. extending Typescript reach into bindings inside templates, enabling hot module reload, adding Aurelia as a first-class citizen in the upcoming ASP.NET SPA extensions... But those things are too big to tackle as side-projects. Would love to go to a company (Microsoft?) to do that if they were willing to hire me 😉

@EisenbergEffect

This comment has been minimized.

Show comment
Hide comment
@EisenbergEffect

EisenbergEffect Sep 6, 2016

Member

@jods4 Have you tested this implementation out locally in your company's project? It would definitely be re-assuring to know that it was working in a real-world app.

Member

EisenbergEffect commented Sep 6, 2016

@jods4 Have you tested this implementation out locally in your company's project? It would definitely be re-assuring to know that it was working in a real-world app.

@jods4

This comment has been minimized.

Show comment
Hide comment
@jods4

jods4 Sep 6, 2016

Contributor

@EisenbergEffect Not yet. I only modified the route-recognizer and used the tests to check if the routes were properly recognized/generated.

Contributor

jods4 commented Sep 6, 2016

@EisenbergEffect Not yet. I only modified the route-recognizer and used the tests to check if the routes were properly recognized/generated.

@EisenbergEffect

This comment has been minimized.

Show comment
Hide comment
@EisenbergEffect

EisenbergEffect Sep 6, 2016

Member

If I merge this to master will you have an opportunity to try it out any time soon? Also, I'd love to feature you on our official blog when we introduce the feature. If your company let you work on this "on the clock" we can feature them too.

Member

EisenbergEffect commented Sep 6, 2016

If I merge this to master will you have an opportunity to try it out any time soon? Also, I'd love to feature you on our official blog when we introduce the feature. If your company let you work on this "on the clock" we can feature them too.

@jods4

This comment has been minimized.

Show comment
Hide comment
@jods4

jods4 Sep 6, 2016

Contributor

This wasn't "on the clock", unfortunately. <_<
I can sometimes get some work done during my day job, but it really has to apply to our projects directly.

If this is merged, yes sure I want to incorporate it into our current project asap. That's the goal!

Contributor

jods4 commented Sep 6, 2016

This wasn't "on the clock", unfortunately. <_<
I can sometimes get some work done during my day job, but it really has to apply to our projects directly.

If this is merged, yes sure I want to incorporate it into our current project asap. That's the goal!

@jods4

This comment has been minimized.

Show comment
Hide comment
@jods4

jods4 Sep 26, 2016

Contributor

@EisenbergEffect Since your last release, you might want to know that we're now running our project on top of several optional routes without issue. I think this issue can be closed? 🎉

Contributor

jods4 commented Sep 26, 2016

@EisenbergEffect Since your last release, you might want to know that we're now running our project on top of several optional routes without issue. I think this issue can be closed? 🎉

@EisenbergEffect

This comment has been minimized.

Show comment
Hide comment
@EisenbergEffect

EisenbergEffect Sep 27, 2016

Member

Awesome!

Member

EisenbergEffect commented Sep 27, 2016

Awesome!

@niemyjski

This comment has been minimized.

Show comment
Hide comment
@niemyjski

niemyjski Jul 6, 2017

Was an new issue opened for having default route values / type validation as discussed in this issue?

niemyjski commented Jul 6, 2017

Was an new issue opened for having default route values / type validation as discussed in this issue?

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