marionettejs / backbone.marionette

The Backbone Framework
https://marionettejs.com
Other
7.07k stars 1.26k forks source link

Add View Constructor events #1560

Closed jasonLaster closed 10 years ago

jasonLaster commented 10 years ago

Disclaimer, I'm not sure if this is a great idea, but it is really powerful and helps a use case I'm currently running into.

 Marionette.View = Backbone.View.extend({
   constructor: function(options) {
+    this.triggerMethod('before:constructor', options);
     _.bindAll(this, 'render');

     Backbone.View.apply(this, arguments);
     Marionette.MonitorDOMRefresh(this);
     this.listenTo(this, 'show', this.onShowCalled);
+    this.triggerMethod('constructor', options);
   },
 });

The problem that I have is that I'd like all of my views to have a viewName which is added to the view's el attribute hash.

I want this API:

ItemView.extend({
  viewName: 'my-view'
})
<div data-view-name="my-view"></div>

I want to turn viewName into attributes.data-view-name, which is really easy with onBeforeConstructor

ItemView.extend({
  attribues: {
    data-view-name: 'my-view'
  } 
})
_.extend(View.prototype, {
   onBeforeConstructor: function() {
     if (this.viewName) {
       this.attributes['data-view-name'] = this.viewName
     }
  }
});
jamesplease commented 10 years ago

I've thought about this feautre, too. It would be a nice complement to the destroy event. WIth that said, I can see it being initialize instead of constructor, so that the name is a verb like all the other events.

For our lifecycle objects this would give us a nice set of lifecycle events:

start/stop initialize/destroy

Also, I'd want this on every Marionette Class...not just Views.

jasonLaster commented 10 years ago

yea, before:initialize would be rocking

Anachron commented 10 years ago

:+1: for all Marionette classes having this event.

jasonLaster commented 10 years ago

Yup, I think before:initialize would be rad. Would add it to Object, Module, etc...

jamiebuilds commented 10 years ago

Wouldn't before:initialize imply:

constructor: function() {
  // constructor stuf...
  this.triggerMethod('before:initialize');
  this.initialize.apply(this, arguments);
  this.triggerMethod('initialize');
}
// vs.
constructor: function() {
  this.triggerMethod('before:constructor');  
  // constructor stuf...
  this.initialize.apply(this, arguments);
  this.triggerMethod('constructor');
}
jamesplease commented 10 years ago

Good thought, @thejameskyle. That's possible. We should definitely go with a verb, so constructor just won't do. Perhaps before:instantiate is better?

jamiebuilds commented 10 years ago

I think instantiate sounds/reads too much like initialize.

Other verbs: construct, create

jamesplease commented 10 years ago

We might need to re-think our inheritance if we add this.

LayoutView calls ItemView's initialize, which calls View's initialize, so there's no clear way to make a before initialize that functions the same across all of the Classes.

If we include it in all 3, it'll be called three times. If it's in View, it'll be called in the middle of initialization, and if it's in anything else then the 'parent' Classes will miss out on the event.

I think instantiate sounds/reads too much like initialize.

Other verbs: construct, create

Mmm...I'm not sure if I think the two words being similar is an issue.

jamiebuilds commented 10 years ago

Mmm...I'm not sure if I think the two words being similar is an issue.

I would confuse the two every single time it came up

jasonLaster commented 10 years ago

hmm, maybe to be consistent with initialize we add preInitialize as a noop function to View.prototype and call it at the beginning of the constructor. That way, preInitialize and initialize are consistent sibling hooks.

If we use triggerMethod, I'd vote for triggerMethod('construct')

samccone commented 10 years ago

preInitialize* :)

samccone commented 10 years ago

but what about prepreinitialize

jamiebuilds commented 10 years ago

preInitialize... has science gone too far?

Anachron commented 10 years ago

Random stuff: beforeAssamble, beforeConstruct, beforeOriginate.

jamesplease commented 10 years ago

Appending pre makes no sense...every other event uses before:.

before:someVerb

^ consistency

jamiebuilds commented 10 years ago

Going back to the original reasoning for this. Is there not a better way to do it?

jamiebuilds commented 10 years ago

Ping @marionettejs/marionette-core. This issue has not been resolved for a long time.

samccone commented 10 years ago

i am cool with before:initialize and initialize

jamesplease commented 10 years ago

I think the problem here is the potential infeasability of the feature. How can we prevent multiple before:initializes with constructor inheritance?

samccone commented 10 years ago

I would assume we would only add it to marionette.view and marionette.object (and controller for now).

jasonLaster commented 10 years ago

Going to second what @samccone said. We'd add it to Marionette.View and all other Marionette classes like Object, Controller, etc...

jamesplease commented 10 years ago

Ehhh..I don't think I like that. For LayoutView, the order would be:

layoutView constructor
itemView constructor
before:initialize
view constructor
initialize

This seems really, really terrible to me. How could we even document this behavior in a way that makes it easy to understand?

before:initialize is called before the constructor method. But not all of the constructor methods. Just the Marionette.View's constructor. So crack open the source and figure out what each constructor does, just so you make sure you know when exactly the event is triggered.

jamiebuilds commented 10 years ago

Following what @jmeas said I'm :-1: to this idea.

jamesplease commented 10 years ago

A crazy idea I've been thinking about for some time might let us support this as our users intend it to work. Once I work it out more I'll write up the details

nvm.

samccone commented 10 years ago

:lock:

jasonLaster commented 10 years ago

hmm. interesting - will close the issue, but warning I might come back to this later.

I think there's a lot to be said for adding a hook for advanced users and we did just get rid of absolute constructor references.