newbie question about magic 'on' methods

[Jason FB]

Here's my application object:

https://gist.github.com/jasonfb/02c26b995dab2379d599

Shouldn't my binding in the initialize function be unneeded? Based on a video I watched on the Marionette docs page I thought that magic "onXYZ" methods were called for you automatically whenever events by those name get triggered?

So if I trigger navigate:category, then I Marionette should fire onNavigateCategory automatically. However, it doesn't work on this object (which incidentally is extended from Marionette.Application). Perhaps this magic 'on' method only works on Marionette Views?

 initialize: function(options) {
     // todo: this should be unneeded with marionette
     this.bind("navigate:category", this.onNavigateCategory);
     this.bind("navigate:home", this.onNavigateHome);
     this.bind("navigate:product_detail", this.onNavigateProductDetail);
  },

Thanks,

Jason

[Jeremy Mcleod]

The magic `on` methods only fire if the event was triggered using Marionette's `triggerMethod` function. AppRouter does contain `triggerMethod`, but as it delegates the actual routing to the original Backbone.Router (which doesn't have `triggerMethod`) then those magic `on` methods won't fire.  The easiest workaround for you would probably be to create an `onRoute` method with a `switch` statement inside that calls your `on` methods - onRoute is called whenever any route is triggered, and the route name, path, and arguments are passed to it as its parameters.  Alternatively, you could directly call `triggerMethod` in your controller methods and duplicate the 'navigate:category' event, but unnecessarily duplicating that event is probably not a good idea.  Finally, you could create your own router class that doesn't delegate the routing to Backbone.Router - you'd basically copy the Backbone.Router's methods into your own class and change all `trigger` calls to `triggerMethod` instead.  That would let you continue to use your existing code, but at the cost of higher complexity and maintenance effort.

[Jeremy Mcleod]

You also seem to be introducing unnecessary complexity.  I'm going to just add comments to the code in your gist to recommend what I would consider "best practices" in your situation.

// A general notes on communication between objects: use the Wreqr or Radio API.
// Create a standalone `radio` or `wreqr` and have all of your modules use it
// for any communication outside of the normal view heirarchy. That is, if you
// have two side-by-side views, A and B, and A has a sub-view A1 and B has a
// sub-view B1, and A1 needs to know when B1 has done something, then B1 should
// send a message out on that radio instance, and A1 should be listening for it.
// Because of the way events bubble in Backbone, A will get any event triggered
// on A1, and B will get any event triggered by B1, but since A1 and B1 are on
// different branches of the view heirarchy, they won't automatically get each
// others' events. That's when the radio/wreqr will come in handy. If you have
// the same radio/wreqr instance used by all of your other modules, you can
// have any component react to any other component with minimal effort.
// (If you were wondering, I don't see a need for this in any of this code, but
// once you start breaking things up into smaller modules - which you should -
// you'll find that using radio/wreqr gives your code far more flexibility and
// portability than using direct namespace references for everything).
MW.Application = Marionette.Application.extend({
  initialize: function(options) {
     // You don't need these as long as you construct your routes/callbacks correctly

     this.bind("navigate:category", this.onNavigateCategory);
     this.bind("navigate:home", this.onNavigateHome);
     this.bind("navigate:product_detail", this.onNavigateProductDetail);
  },

  start: function() {


    // This should really be in its own file. You're creating a router, controller,
    // and more all inside your application module, which leads to a single monolithic
    // structure that's much harder to maintain. Since it doesn't look like you're
    // using AMD or other dependency injection, the namespace scheme that you currently
    // have should be sufficient. If you use CapsCamelCase for constructors and
    // lowerCamelCase for instances, you can even save references to both using
    // your namespace object under (mostly) the same name.
    MW.catalog_controller = {
     // These methods are the PRIMARY place logic should be placed for when a route
     // is matched. Any listeners for `navigate` events should only be performing
     // SECONDARY logic, like closing UI elements that are no longer needed. Basically,
     // you should move the logic on your `on` methods directly into these router callbacks.
     // If for some reason you
     index: function() {
        console.log("index");
      },
     showCategory: function(permalink) {
        console.log("showCategory");
      },
 
      showProdcutDetail: function(slug) {
        console.log("showProdcutDetail");
      }
    }
 
    // Again, this should really be in its own file for modularity's sake
    MW.appRouter = new MW.Router({
      controller: MW.catalog_controller,
      appRoutes: {
        '': 'index',
        "t/:permalink": "showCategory",
        "products/:slug": "showProdcutDetail"
      }
    });
 
    Backbone.history.start({pushState: true});
 
    // Again, move into its own file. If there's only ever going to be one instance
    // of this view on screen at a time, that file should return an INSTANCE of the
    // view, rather than its constructor.
    MW.root_view = new MW.View.RootView({
      el: '#primary-container',
      regions: {
        mobile_nav: "#mobile-nav",
        desktop_nav: "#desktop-nav",
        page_content: "#page-content",
        first_footer: "#first-footer",
        second_footer: "#second-footer"
      }
    });
 
    // this should probably be in the initialize method of your root view
    MW.main_vew = new MW.View.MainNav({el: ".header-primary" })
 
    // And I'd put this call in the `onShow` method of the root view
    MW.root_view.getRegion("desktop_nav").show(MW.main_vew);
  },


  // These methods are pretty much unnecessary. You should move their logic into
  // the router callbacks defined in your controller.
  onNavigateCategory: function (dest) {
    // The router handles this part automatically. You don't need to call it as long
    // as the logic is in the controller methods, and you shouldn't need a reference
    // to the router in the controller itself.
    MW.appRouter.navigate(dest, {trigger: true});
    // Same comments as with the root view. If you don't actually need multiple
    // instances of this view, you should be using an instance reference created
    // directly after the view definition. You don't want to create a whole new
    // instance every time the route is triggered - that way lies memory leaks.
    MW.category_view = new MW.View.CategoryPageView({
      destination: dest
    });
    // Move this to the `onShow` method of the category_view
    MW.root_view.getRegion("page_content").show(MW.category_view);
  },
  onNavigateHome: function (dest) {
    // Same comments as above
    MW.appRouter.navigate(dest, {trigger: true});
    MW.home_view = new MW.View.HomePageView({
 
    });
    MW.root_view.getRegion("page_content").show(MW.home_view);
  },
  onNavigateProductDetail: function (dest) {
    // Same comments as above.
    console.log("onNavigateProductDetail");
    MW.appRouter.navigate(dest, {trigger: true});
    MW.pdp_view = new MW.View.ProductDetailPageView({
 
    });
    MW.root_view.getRegion("page_content").show(MW.pdp_view);
  }
 
});

On Wednesday, May 13, 2015 at 11:07:40 AM UTC-4, Jason FB wrote:

[Jeremy Mcleod]

One other thing you might consider doing is moving your route definitions into the router definition itself. I don't know if having an empty `routes` hash in your router definition is causing problems, but it could be.  I'd change your router to this:

MW.Router = Backbone.Marionette.AppRouter.extend({
  routes: {

    '': 'index',
    "t/:permalink": "showCategory",
    "products/:slug": "showProdcutDetail"

  },
  initialize: function() {
    this.bind('route', this.trackPageview);
  },
  onRoute: function() {
    console.log("onRoute");
  },
  trackPageview: function ()
  {
    console.log("trackPageview");
    var url = Backbone.history.getFragment();
 
    //prepend slash
    if (!/^\//.test(url) && url != "")
    {
        url = "/" + url;
    }
    //_gaq.push(['_trackPageview', url]);
  }
});

I'd also define your controller somewhere other than in the middle of your application definition. There's no reason you can't define it immediately below your router (as long as it's before you create an instance of that router). You also have a few missing semi-colons that probably aren't causing any problems, but better to be safe than sorry.

Finally, I'd make sure the route paths you have defined include the full path of the route. I don't know how you have your paths set up, but it's possible that Backbone doesn't think that the root path off of which all of your other routes are based is what YOU think it is.  That is, if your "index" path is actually at something like "myhost.com/my/app" then it's possible (since you're using pushstate) that Backbone wants your index route path to be "/my/app" and not just an empty string.