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

Add chaining support to Router methods #892

Closed
wants to merge 1 commit into from
Closed

Conversation

hx
Copy link

@hx hx commented Jan 27, 2012

I like to add routes using regexes in batches. This patch allows chaining of route() and navigate() methods.

@jashkenas jashkenas closed this in ac580ca Jan 30, 2012
@jashkenas
Copy link
Owner

I've made .route chainable, as you suggest ... but making .navigate chainable doesn't make any sense. There's absolutely no reason why you'd ever go to one route, and then immediately to another without the user taking any action.

@hx
Copy link
Author

hx commented Feb 3, 2012

@jashkenas I have chained other calls onto .navigate calls before. Obviously not subsequent .navigate calls, but to other local methods that I wanted to run after triggering a route, such as updating an auxiliary history tracker (part of an undo stack) and clearing a cache of some description. These are only the two that come to mind, but even if it's only vaguely useful, where's the harm in including it?

@jashkenas
Copy link
Owner

Because it's a bizarre return value for a side-effect-ful function after which there's no reason to chain. If you're already extending your Router with extra functions that you do want to chain, feel free to override navigate and make it chainable at the same time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants