Adding routes with the same path causes problems

[glassresistor]

trafficstars

In my code im doing something about the same as this

router.addRoute('samePath', function1);
rouder.addRoute('samePath', function2);

I expected to be able to use match continuation so that both functions ran like function1 function2. It looks like the same your handling the routeMap as a associated array https://github.com/aaronblohowiak/routes.js/blob/master/index.js#L135 means that if something is added with the same path the 2nd one will override the first and also be run twice. I don't think this is the desired(by you) behaviour. I believe the easiest solution is to add the router to a list like [[path, function], [path, function]] and use the numeric index to look them back up.

[Raynos]

I'm not going to change the internals.

I recommend you do

router.addRoute('samePath', function () {
  // some magic.
  function1();
  // some more custom magic.
  function2();
});

[glassresistor]

Just pointing out that doing routeMap the way you are means that you can not use the same path twice and that I didn't expect that to be a behaviour you wanted specifically that it just overrites the first route. Meaning it has bad undocumented behaviour, if you don't want to change it atleast document the bug. I'll be forking and fixing this myself but don't plan on using a continuing to use routes as is.

[glassresistor]

Here it is working and with tests https://github.com/motherjones/routes.js

[Raynos]

Cool, nice work.

[glassresistor]

actually hit a weird issue with fn being dropped but the tests pass. ill message when i figure it out.

On Wed, Oct 29, 2014 at 4:21 PM, Raynos (Jake Verbaten) < [email protected]> wrote:

Cool, nice work.

— Reply to this email directly or view it on GitHub https://github.com/aaronblohowiak/routes.js/issues/31#issuecomment-61022255 .

Mikela

[glassresistor]

nvm it was just an issue with the dist folder it works great even in my project

On Wed, Oct 29, 2014 at 4:27 PM, Mikela Clemmons [email protected] wrote:

actually hit a weird issue with fn being dropped but the tests pass. ill message when i figure it out.

On Wed, Oct 29, 2014 at 4:21 PM, Raynos (Jake Verbaten) < [email protected]> wrote:

Cool, nice work.

— Reply to this email directly or view it on GitHub https://github.com/aaronblohowiak/routes.js/issues/31#issuecomment-61022255 .

Mikela

Mikela

[jhnns]

First of all: Thx for the cool module. :+1:

I've expected the behavior to be like @glassresistor described it (and I think that's the way how the connect router works). It imho defeats the purpose of a middleware-architecture where you compose small functions to a big application.

I'd be glad to see this feature in the "official" module.

[jhnns]

Independent of that feature: The current behavior definitely seems like a bug, because when I add three different functions for the same name the last functions gets called three times. I can't imagine a situation where this is useful.

[glassresistor]

@jhnns thats for the +1 the authors suggestion is great so I'll be taking his advice, go ahead and use my fork its totally stable and in use in a project of mine I'll be republishing this with a fix for afew other issues like match not having the Router properly bound to it so match can be used to pass down context and some cleanup. Any help naming it would be loved. So far its just moreRoutes

[greim]

Just blew my morning on this issue. It would be helpful if there was a warning visible somewhere in the documentation, or a change to fix the underlying probelm.

[jhnns]

I'd still like to see this feature in the original module because imho that's not a worthy reason to fork the module (especially if more people expected it the other way). But if that's @Raynos last word I'd name it something like signal-box or something (still available on npm :grinning:)

Or routes2 (like eventemitter2) to refer to the original api.

[glassresistor]

Here is the new library on github https://www.npmjs.org/package/i40 and npm https://www.npmjs.org/package/i40

I'll update credits and says its a fork asap. @Raynos if you could link to it with bug that would be cool. I can't find the thread where this was requested.

[glassresistor]

added PR to link to new library here https://github.com/aaronblohowiak/routes.js/pull/35

[Raynos]

Released v2.0.0

If you define path twice it now throws an exception.

[glassresistor]

@jhnns @greim please try the https://www.npmjs.org/package/i40 package and let me know if there is any issues, i'll not change anything except bugs on version 1.0.X but 1.1.X will probably add some features and change enough code something might change. Good luck sorry you ran into the same issue as me. I wasted a decent amount of time thinking it was me.

[jhnns]

Cool, I'll try that. Thx @glassresistor :+1:

[glassresistor]

I've updated i40 to version 1.3.0 and removed some of the awkward stuff and updated docs https://www.npmjs.com/package/i40 I'm going to eventually release a 2.0 which will probably not offer breaking changes but will move to using prototypes properly, use mocha tests, docs and add afew things like addRoutes and named route lookup similar to what django offers maybe even optional url helpers from an inner require.

cc: @jhnns

[jhnns]

Nice, thx for the update!

[dashed]

@glassresistor if you're maintaining a fork; mind enabling issues for i40 repo?

[glassresistor]

@Dashed ok I added an issues and changed the name to i40 from routes https://github.com/glassresistor/i40

[Raynos]

At this point, this module is no longer maintained.

I use:

• https://github.com/Matt-Esch/http-hash

and @glassresistor maintains

• https://github.com/glassresistor/i40

I should update the README.

[Raynos]

I updated the README with https://github.com/aaronblohowiak/routes.js#alternative-routers