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

issue with wired views #52

Closed mmikeyy closed 10 years ago

mmikeyy commented 10 years ago

There seems to be an error in the _wrapConstructor function.

Current is as follows:

        _wrapConstructor: function(OriginalConstructor, wiring) {

            var context = this._context;

            return OriginalConstructor.extend({
                initialize: function() {
                    context.resolver.resolve(this, wiring);
                    OriginalConstructor.prototype.initialize.call(this, arguments);
                }
            });
        },

I was wondering why my wired views cound no longer be initialized using the very simple coffeescript syntax such as:

# the view is created
newView = new @wiredView parentContext: currentContext, model:@model
#........

# view code:
define [
  'marionnette'
  'geppetto'
  'context_ref'
],(
  Marionnette
  Geppetto
  context
)->
  class wiredView extends Marionnette.ItemView
    initialize: ({@parentContext})->
      Geppetto.bindContext
          view: @
          context: context
          parentContext: @parentContext
#.....

Using Geppetto as it is (v .70), parentContext is not assigned to @parentContext in the view when this code is executed.

I thought that if I have to stop using valid syntax to circumvent Geppetto quirks, I might as well forget about Geppetto.

Finally I found the problem. Constructor wrapper CALLS the original constructor with arguments, instead of APPLYing the arguments to it. The view receives as initialization parameter the argument pseudo-array object and not the expect object originally sent.

So in my patched Geppetto now, the last statement of the _wrapConstructor fn is as follows:

OriginalConstructor.prototype.initialize.apply(this, arguments);

and the problem is gone.

creynders commented 10 years ago

We really need a :facepalm: emoji...

Good job on finding these bugs. I never pass values to my view constructors, so I don't encounter this situation.

How would you feel about putting this (and your other finds) into (a) PR(s)? I can help you out with it, if you haven't done that before.

mmikeyy commented 10 years ago

All willing but never done before.. 😶 Le 28 août 2014 02:13, "creynders" notifications@github.com a écrit :

We really need a :facepalm: emoji...

Good job on finding these bugs. I never pass values to my view constructors, so I don't encounter this situation.

How would you feel about putting this (and your other finds) into (a) PR(s)? I can help you out with it, if you haven't done that before.

— Reply to this email directly or view it on GitHub https://github.com/ModelN/backbone.geppetto/issues/52#issuecomment-53677523 .

mmikeyy commented 10 years ago

Ah.. ok I figured it out. Just did a first PR. Let me know if there ever is some kind of etiquette that I fail to comply with…

We really need a :facepalm: emoji...

Good job on finding these bugs. I never pass values to my view constructors, so I don't encounter this situation.

How would you feel about putting this (and your other finds) into (a) PR(s)? I can help you out with it, if you haven't done that before.

— Reply to this email directly or view it on GitHub.

geekdave commented 10 years ago

See https://github.com/ModelN/backbone.geppetto/pull/53

geekdave commented 10 years ago

Fixed by #53