swipely / aviator

Aviator is a single-page front-end router built for modularity.
MIT License
143 stars 6 forks source link

Allow routes to be specified as functions #66

Closed nahiluhmot closed 8 years ago

nahiluhmot commented 9 years ago

Anonymous Route Handlers

@hojberg @flahertyb @tlunter @AdamEdgett @barnabyc @ermanc

This pull request allows route handlers to be specified as anonymous functions in the arguments to Aviator.setRoutes.

Motivation

Aviator's current means of specifying route handlers, via route targets and method names, is useful for grouping the route handling logic of related routes. However, for simple applications it feels a little bit more cumbersome than need be. Consider this application:

var RootTarget = {
  beforeAll: function () {},
  home: function () {},
  about: function () {},
  contact: function () {},
  legal: function () {},
  menu: function () {},
  notFound: function () {}
};

Aviator.setRoutes({
  target: RootTarget,
  '/*': 'beforeAll',
  '/': 'home',
  '/about': 'about',
  '/contact': 'contact',
  '/legal': 'legal',
  '/menu': 'menu',
  notFound: 'notFound'
});

Aviator.dispatch();

The above example is not very DRY, since one essentially has to repeat the route name in the arguments to Aviator.setRoutes in the handler itself.

Description

This PR makes route targets optional by allowing the handlers to be specified as anonymous functions. Using these changes, the above example could be re-written like so:

Aviator.setRoutes({
  '/*': function () {},
  '/': function () {},
  '/about': function () {},
  '/contact': function () {},
  '/legal': function () {},
  '/menu': function () {},
  notFound: function () {}
});

Aviator.dispatch();

Although the above example is not modular, one can still break up related routes into separate objects or files. More complex applications may look something like this:

var MarketingRoutes = {
  '/*': function () {},
  '/': function () {},
  notFound: function () {}
};

var StoreRoutes = {
  '/:store': function () {}
};

var BrandRoutes = {
  '/*': function () {},
  '/': function () {},
  '/:brand_id': {
    '/': function () {},
    '/stores': StoreRoutes
  }
  notFound: function () {},
};

var Routes = {
  '/*': function () {},
  '/': function () {},
  notFound: function () {},
  '/marketing': MarketingRoutes,
  '/brands': BrandRoutes,
};

Aviator.setRoutes(Routes);

Aviator.dispatch();

Of course, there's no reason that this feature should conflict with the existing Aviator style. Below is an example of an application that uses both the route target/method style and anonymous function style.

var UsersTarget = {
  beforeAll: function () {},
  index: function () {},
  notFound: function () {}
};

var Routes = {
  '/*': function () {},
  '/': function () {},
  notFound: function () {},
  '/users': {
    target: UsersTarget,
    '/*': 'beforeAll',
    '/': 'index',
    notFound: 'notFound',
  }
};

Aviator.setRoutes(Routes);

Aviator.dispatch();

There should be good test coverage, but please call out anything I missed.

hojberg commented 9 years ago

I think the current approach of using a RouteTarget is the way we should present as the default option, since it encourages having a separate route file from the implementation (one of the main reasons we built Aviator), but perhaps at least giving the user the choice ok.

I guess the question is how opinionated do we want Aviator to be.

@barnabyc @flahertyb what's your take?

nahiluhmot commented 9 years ago

@hojberg Addressed some of your comments in my last two commits, will fix them up if/when we're good to merge.

nahiluhmot commented 9 years ago

Ping @barnabyc @flahertyb

nahiluhmot commented 9 years ago

Ping ping @barnabyc @flahertyb

barnabyc commented 9 years ago

Sorry @nahiluhmot!

barnabyc commented 9 years ago

:+1: Looks good, but I agree with @hojberg in that the RouteTarget pattern is the preferred, but I can see small projects starting as anonymous and then migrating.

Should also note in the docs that any route handler will receive an argument of the request object.

flahertyb commented 9 years ago

I totally agree the enforcement of route targets is cumbersome in tiny projects. Dislike complex applications using anonymous functions all over the place, but also dislike unnecessarily opinionated libraries.

:+1:

nahiluhmot commented 8 years ago

Sorry about the long silence period. I fixed up the commits and rebased off of master. Good to merge?

hojberg commented 8 years ago

@nahiluhmot đź‘Ť

nahiluhmot commented 8 years ago

Fast response!