Daniel15 / RouteJs

JavaScript URL routing for ASP.NET MVC and WebForms
84 stars 19 forks source link

Route .action - invalid route created (regression in v2.0) #42

Closed barbedCoil closed 8 years ago

barbedCoil commented 9 years ago

This is an issue that only started after I upgraded to v2.0 of RouteJS

Called this route to validate - works fine, the route: {route} is from my js file SiteRoutes.js (see below for code snippet):

SiteRoutes.js:42 route:  /Auth/Login

Login validated, go to the main page, again works fine (see Inventory.IndexView() just below):

SiteRoutes.js:143 route:  /Inventory/Index

I believe this is output from RouteJS when it navigated to the above route:

Navigated to http://localhost:49850/Inventory/Index

Now I call AdminUser.IndexView() function below (note: same function everywhere, no variation) and you can see that Router.action('AdminUser', 'Index') is appending the last route at the end

SiteRoutes.js:81 route:  /AdminUser/Index/%2FInventory%2FIndex

And the attempt to navigate to that route is going to fail since I want /AdminUser/Index only

Navigated to http://localhost:49850/AdminUser/Index/%2FInventory%2FIndex

Here is the basic routing js object I have setup to manage all my routes in one place: You can see that every function just returns the Router.action(...) result

Inventory:
{
    IndexView: function ()
    {
        var route = Router.action('Inventory', 'Index');
        console.log('route: ', route);
        return route;
    },
},

AdminUser:
{
    IndexView: function ()
    {
        var route = Router.action('AdminUser', 'Index');
        console.log('route: ', route);
        return route;
    },

    // [Route("Create")]
    CreateView: function () 
    {
        var route = Router.action('AdminUser', 'Create');
        console.log('route: ', route);
        return route;
    },

    // [Route("Edit")]
    EditView: function () 
    {
        var route = Router.action('AdminUser', 'Edit');
        console.log('route: ', route);
        return route;
    },
},
barbedCoil commented 9 years ago

Ok, I figured out the issue. Turns out my new controller's did NOT have the routing attributes which I guess means they were not available to the generated .js on client side? Not sure but adding them ([RoutePrefix("AdminUser")] for the class and [Route("Index")] for the route that was failing) resolved the issue.

Daniel15 commented 9 years ago

Interesting, sounds like there's been some regression in v2.0. This behaviour shouldn't have changed between 1.x and 2.0, and all the unit tests are still passing. I'll reopen this and take a look.

Routes don't need the Route attribute, using attributes to specify routes is an optional feature of ASP.NET MVC (you can specify all your routes as attributes rather than having them in a single file). I'll take a look.

barbedCoil commented 9 years ago

Daniel, Let me add more info that may be helpful. Adding the attributing did in fact fix the issue for me. But, I am also using Identity 2.0 and have attributed Authorization as well. I do not think this makes a difference because I have been running authorization for the past week and everything worked fine until my mass update today.

If you need help looking at this let me know as this utility has been a life saver for me and I am happy to help if I can.

barbedCoil commented 9 years ago

REgarding routes as attributes and having them in a single file. I do have most of my routes attributed but the single file is for all of my javascript routes so I don't have a bunch of string spread out all over if/when I need to change a route

Daniel15 commented 9 years ago

Thanks for the info! Could you please make a small project that reproduces the issue (eg. sets up the routes the same way you do) so I can take a closer look?

barbedCoil commented 9 years ago

I think I can do that - I just have a lot of client specific stuff - let me see what I can do...

Daniel Lo Nigro mailto:notifications@github.com September 1, 2015 at 1:01 PM

Thanks for the info! Could you please make a small project that reproduces the issue (eg. sets up the routes the same way you do) so I can take a closer look?

— Reply to this email directly or view it on GitHub https://github.com/Daniel15/RouteJs/issues/42#issuecomment-136843590.

Daniel15 commented 9 years ago

Yeah, just a small project with the bare minimum should be fine - Doesn't need to include any of your actual app code, just the routes and some empty controllers. I'd like to compare what RouteJs v1 did compared to v2 :)

Regards, Daniel Lo Nigro http://dan.cx/ | Twitter http://twitter.com/Daniel15 | Facebook http://www.facebook.com/daaniel

On Tue, Sep 1, 2015 at 1:03 PM, trh notifications@github.com wrote:

I think I can do that - I just have a lot of client specific stuff - let me see what I can do...

Daniel Lo Nigro mailto:notifications@github.com September 1, 2015 at 1:01 PM

Thanks for the info! Could you please make a small project that reproduces the issue (eg. sets up the routes the same way you do) so I can take a closer look?

— Reply to this email directly or view it on GitHub https://github.com/Daniel15/RouteJs/issues/42#issuecomment-136843590.

— Reply to this email directly or view it on GitHub https://github.com/Daniel15/RouteJs/issues/42#issuecomment-136844687.

barbedCoil commented 9 years ago

Daniel,

Sorry to leave you hanging... I am not able to extract just the routing from my project - it's way to complicated to strip out the code I can't share with you.

If you have questions then fire away or if you want me to look at a build then let me know

Sorry took so long to get back to you.

Tab

Daniel Lo Nigro mailto:notifications@github.com September 1, 2015 at 1:05 PM Yeah, just a small project with the bare minimum should be fine - Doesn't need to include any of your actual app code, just the routes and some empty controllers. I'd like to compare what RouteJs v1 did compared to v2 :)

Regards, Daniel Lo Nigro http://dan.cx/ | Twitter http://twitter.com/Daniel15 | Facebook http://www.facebook.com/daaniel

On Tue, Sep 1, 2015 at 1:03 PM, trh notifications@github.com wrote:

I think I can do that - I just have a lot of client specific stuff - let me see what I can do...

Daniel Lo Nigro mailto:notifications@github.com September 1, 2015 at 1:01 PM

Thanks for the info! Could you please make a small project that reproduces the issue (eg. sets up the routes the same way you do) so I can take a closer look?

— Reply to this email directly or view it on GitHub

https://github.com/Daniel15/RouteJs/issues/42#issuecomment-136843590.

— Reply to this email directly or view it on GitHub https://github.com/Daniel15/RouteJs/issues/42#issuecomment-136844687.

— Reply to this email directly or view it on GitHub https://github.com/Daniel15/RouteJs/issues/42#issuecomment-136845105.

Regards, Tab R. Hockamierhttp://www.getpostbox.com

Daniel15 commented 8 years ago

Sorry it took me so long to reply. I tried looking into this today, but so far I've been unable to replicate the issue.

barbedCoil commented 8 years ago
                                                                                  Ignore. I re installed and works fine                                                                                                                                                                                                                                                                                                                                         Tab                                                                                                                                                                                                                From: Daniel Lo NigroSent: Sunday, October 25, 2015 2:48 PMTo: Daniel15/RouteJsReply To: Daniel15/RouteJsCc: trhSubject: Re: [RouteJs] Route .action - invalid route created (regression in v2.0) (#42)Sorry it took me so long to reply. I tried looking into this today, but so far I've been unable to replicate the issue.

—Reply to this email directly or view it on GitHub.