Separate attributes into hook

[jridgewell]

A small change to allow subclasses access to the attributes that should be on the view. This is particularly handy when there are dynamic attributes...

Something like:

var View = Backbone.View.extend({
    id() { return this.model.get('id'); },
    attributes() {
        // some logic
    },

    render() {
        this.$el.html(this.template(this.model.attributes));

        var attrs = _.extend({}, _.result(this, 'attributes'));
        if (this.id) attrs.id = _.result(this, 'id');
        if (this.className) attrs['class'] = _.result(this, 'className');
        return attrs;

        this.$el.attr(attrs);
    }
});

Notice the attributes update code is just copied pasted from the Backbone source.

[akre54]

What's the use case?

[jridgewell]

I had a list collection view that allowed you to select multiple items. The selected state was toggled by a css class on the item view's el.

class ItemView {
  className() {
    return this.model.get('active') ? 'active' : '';
  }
}

I specifically want the className to update, but I think grabbing the updated attributes and id would be helpful to others as well.

[akre54]

But this changes the way the view attribute methods are called, from being only invoked once at instantiate-time to now potentially being invoked many times throughout the view lifecycle with different state.

In my experience with Backbone you almost never want render to be called multiple times when you've got nested views. It's much better to put fine-grained changes (like updating the className) in an event handler and manipulate the el directly.

I like it in theory, but I wonder if it opens the door to too big of a breaking change in the way View works.

[jridgewell]

It's much better to put fine-grained changes (like updating the className) in an event handler and manipulate the el directly.

I'm actually trying to move into the opposite direction. I see all of the event listeners with element specific update logic as duplication that should be in the template. I'm hoping with iDOM to never have to write another one. 😄

The only parts I can't put into the template are the root el attributes, and I was hoping to expose the "all attributes" logic so I can remove the copy-paste from my libraries.

[akre54]

Sure, but if you're getting rid of the jQuery parts of View in favor of iDOM, you don't need tagName, id, className, etc anyway, right? (or even much of the structure of View...)

In my experience with virtual DOMs you usually compute the dynamic attributes in your render method at runtime. You can move that logic to your own methods if you want, but there shouldn't be a need to put this on View.

Closing, in lieu of a better example.

[jridgewell]

Sure, but if you're getting rid of the jQuery parts of View in favor of iDOM, you don't need tagName, id, className, etc anyway, right? (or even much of the structure of View...)

They're still needed as the root element is managed by Backbone. Rendering with iDOM isn't any different than the way we render with jQuery, it's just much faster.

In my experience with virtual DOMs you usually compute the dynamic attributes in your render method at runtime.

That's for template based vDOMs, where the root element is defined in the render function. Can't do that with Backbone, unless I want a bunch of wrapper DIVs (ick).

Closing, in lieu of a better example.

#3255, and (if you ignore the discussion about Views without root elements) marionettejs/backbone.marionette#2357 (comment).

[akre54]

They're still needed as the root element is managed by Backbone

I don't understand why this would be. You're only managing a small part of your app with a virtual dom? Why?? This is backwards from most vdom solutions I've seen. Mostly it's VDOM top of tree, then mutable leaves when you need Google Maps or a jQuery plugin or something.

it's just much faster.

vdom isn't magic. It's faster for certain types of changes (particularly ones that cause reflows), but slower for others (ones where you know what you're going to change ahead of time). Throwing a subtree of your app in a vdom isn't going to magically speed it up. The biggest gains tend to come from change-heavy apps that modify many parts of the tree at once.

That's for template based vDOMs, where the root element is defined in the render function

I think we might be using different terminologies here. I'm talking about something like this:

export default App = React.createClass({
  render() {
    const {isActive} = props;
    return <div className={cx({active: isActive})} />
  }
});

Can't do that with Backbone, unless I want a bunch of wrapper DIVs (ick).

Agreed. That's what I'm arguing against.

#3255, and (if you ignore the discussion about Views without root elements) marionettejs/backbone.marionette#2357 (comment).

#3255 isn't super compelling, the comments are pretty spot on. I see Backbone Views as orthogonal to VDOM. Backbone Views have a two-stage creation process: instantiation (var view = new MyView) and render (view.render()). They're not idempotent. VDOM parents make no distinction between newly instantiated components and re-rendered ones. The child manages its own lifecycle state.

[paulfalgout]

But this changes the way the view attribute methods are called, from being only invoked once at instantiate-time to now potentially being invoked many times throughout the view lifecycle with different state.

I really don't see the issue with this PR. It does nothing to enforce a new rendering strategy, nor is it breaking.. Not adding this however does very little in preventing people from poor rendering strategies. A quick search on stack overflow and it's easy to find people reproducing the attributes code inside their renders to do just that anyways.

[jridgewell]

I think we're getting off topic here. I'd be happy to discuss how I'm trying to integrate iDOM on gitter.

Agreed. That's what I'm arguing against.

@jashkenas comment on #3255 is pushing towards a wrapper DIV. This PR gives the dev a way to get the attributes as they should be now.

And this isn't necessarily tied to a re-render.

class View {
  initialize() {
    this.listenTo(this.model, 'change', this.updateAttributes);
  }

  className() {
    return this.model.get('active') ? 'active' : '';
  }

  attributes() {
    return {
      data-image: this.model.get('image')
    };
  }

  updateAttributes() {
    this.$el.attr(this._attributes());
  }
}

---

Answering:

I don't understand why this would be. You're only managing a small part of your app with a virtual dom? Why??

Because it's a Backbone app. I'm using iDOM as a templating language, nothing more. All of the core philosophies of Backbone still apply, including el being created by the View so that events can be delegated to it.

I think we might be using different terminologies here. I'm talking about something like this:

Exactly. The App's root element has been defined in #render. That's not how Backbone works.

vdom isn't magic.

A lot of people use jQuery#html to re-render the view, which is very slow. iDOM is magic compared to that.

I see Backbone Views as orthogonal to VDOM

I'm definitely not trying to make Backbone more like React as in marionettejs/backbone.marionette#2357. But one of the solutions to it is to make the root el a little less static (tagName being the exception). If we can support dynamic attributes easily, I don't see why we shouldn't.

[akre54]

I really don't see the issue with this PR. It does nothing to enforce a new rendering strategy, nor is it breaking..

But it implies a use case that is different. Is this something we want to support? If it's towards a better vdom solution, let's figure out the answer holistically.

A lot of people use jQuery#html to re-render the view, which is very slow. iDOM is magic compared to that.

But it's certainly not faster (or magic ✨) compared to just adding a class directly.

[jridgewell]

If it's towards a better vdom solution, let's figure out the answer holistically.

This is wholly separate from vDOM. Backbone currently treats the root element as static, with attributes only being set at instantiation time. This pushes us to using wrapper DIVs, per #3255 (comment). I really dislike that.

This isn't trying to tackle mutating the tagName or supporting any kind of template defined root element. Just trying to push us away from wrappers.

compared to just adding a class directly.

Ignoring the magic discussion, this means we already support mutating the root element's attributes – just indirectly. This PR gives a generalized method to grab all of the attributes with our custom precedence rules (#id and #className override the same keys in #attributes), instead of requiring separate listeners to update className, id, and any arbitrary attributes or copy-pasting our library code.

[ianmstew]

But this changes the way the view attribute methods are called, from being only invoked once at instantiate-time to now potentially being invoked many times throughout the view lifecycle with different state.

The attributes are still only set by the library once, so I don't see how this could break an app.

In my experience with Backbone you almost never want render to be called multiple times when you've got nested views. It's much better to put fine-grained changes (like updating the className) in an event handler and manipulate the el directly.

change: render combined with an uncomplicated this.el.innerHtml = this.template(data) is a common pattern, and I don't see how improving flexibility to leverage other patterns is a contradiction to Backbone philosophy. Suggesting a particular approach to the opinion-less render() is what I find a contradiction.

@akre54, the Backbone wrapping element defined outside of the template presents its own set of challenges (for good reasons); why hold back a backwards-compatible improvement that could make dealing with that fundamental tenet of Backbone a little more elegant?