GeppettoJS / backbone.geppetto

Bring your Backbone applications to life with an event-driven Command framework.
http://geppettojs.github.com/backbone.geppetto/
MIT License
203 stars 28 forks source link

BUG: commands not executed when mapped through options object #43

Closed creynders closed 10 years ago

creynders commented 10 years ago

Hi there, it's me again :)

I have the following context class:

module.exports = Backbone.Geppetto.Context.extend( {
    wiring : {
        commands : {
            "app:startup.requested": require('./commands/RegisterHelpers.js')
        }
    },
    initialize : function(){
        this.dispatch('app:startup.requested');
    }
} );

This unfortunately won't run, since a contexts initialize method is run (https://github.com/ModelN/backbone.geppetto/blob/master/backbone.geppetto.js#L190) before the parsing of the wiring object (https://github.com/ModelN/backbone.geppetto/blob/master/backbone.geppetto.js#L198).

It seems we could move the initialize method invocation to the last line, but I'm not entirely sure?

BTW there's quite a few mistakes in the documentation: e.g. 'commands' > 'option2: using the commands map' shows

return Geppetto.Context.extend( {
    commands: {
        "appEventFoo":          FooCommand,
        "appEventBar":          BarCommand,
        "appEventBaz":          BazCommand,
        "appEventFooBarBaz":    [
            FooCommand,
            BarCommand,
            BazCommand
        ]
    }
});

but the commands object should be wrapped in a wiring object.

Also in 'Setting up wiring in the context' the example is:

return Geppetto.Context.extend( {
    initialize: function () {
        wiring: {
            singletons: {
                "userModel": Backbone.Model,
                "productModel": ProductModel,
                "loggingSvc": LoggingSvc
            },
            views: {
                "UserView": UserView,
                "ProductView": ProductView
            }
        }
    }
});

but the wiring object should be moved to outside the initialize function.

Sorry if I am to blame, my memory's so terrible I can't remember whether this is stuff I wrote or not?

geekdave commented 10 years ago

Ah yeah, I never hit this myself because I always use "pseudo-constants" for my event names, which don't work in object maps. So I always wire my commands programmatically using wireCommand() in the initialize method.

At first glance, I think it would be totally fine to move initialize to run before _configureWirings... but I haven't tested it. I'd be open to a PR if it would solve your problem, and you can verify that it works.

As for the docs, yeah they're still in bad shape. Feel free to sneak in any doc fixes into unrelated PRs.