Use template as el

[designermonkey]

One thing I find increasingly annoying about Backbone is it's penchant for wrapping everything in a div. While using LayoutManager for a short time, I liked how a developer could set the el of a View to false, so as to tell LayoutManager to use the provided template as the Views el.

IMO, this should be a part of Marionette also. There is already the option to pass false to template to disable it, so would follow suit in that regard.

It would be optional and defaulted to the current behaviour to not break any backwards compatibility.

From what I can see of the source code, it's not easily something that can be monkey patched in an extension of View either, although I am really going to try.

Any thoughts?

[samccone]

xref https://github.com/marionettejs/backbone.marionette/issues/2041 https://github.com/marionettejs/backbone.marionette/issues/815 https://github.com/marionettejs/backbone.marionette/issues/431 https://github.com/jashkenas/backbone/issues/2329

I still stand by what I said in #2041

[designermonkey]

Thanks for replying so fast.

After reading all those issues (thanks for the links, I did try looking), I can see how they are related. I am also astounded at some of the responses regarding why it should be accepted to allow arbitrarily wrapping anything added by JavaScript in a div, because, well, just because.

I know this isn't a responsibility of Marionette, which goes a hell of a way to patch up the great simplicity of Backbone to be a useable framework for JavaScript, but I have to say, it is a problem that needs addressing. If it can be proven to work in an extension of Backbone, then maybe it can be back-ported into Backbone.

Frameworks like this, need to be as agnostic about how a developer chooses to implement their HTML and CSS as possible. Personally, I don't tag every element with a classname because it looks horrible, and as stated in one response, it looks auto-generated, and doesn't adhere to DRY and KISS.

Wrapping everything in divs forces a certain way of working, and when the JavaScript is the last piece of a very large project-puzzle to be done, having to go back through to re-write all of the CSS can ruin morale, budgets, deadlines, and client expectations, especially when the developers have written HTML and CSS in a very light manner to keep markup clean.

Also, think about this markup...

<ul>
    <div>
        <li></li>
    </div>
    <div>
        <li></li>
    </div>

    ...

</ul>

You would never allow this to be output by a server side script, so it should never become acceptable for JavaScript.

[samccone]

Frameworks like this, need to be as agnostic about how a developer chooses to implement their HTML and CSS as possible

I agree 100%

I wrote up a blog post about why the wrapping div gives a bunch of power to event binding and such http://blog.marionettejs.com/2015/02/12/understanding-the-event-hash/index.html

That said, you should take a stab at implementing a non wrapping version. I know some people have done this... But let me know

[designermonkey]

Will do, I'm looking at it now.

[designermonkey]

@samccone I just read your blog post. I thought that events were delegated to document until I read that.

I know I read a while ago that with jQuery, document was more performant at handling delegated events that delegating to loads of different nodes within the DOM. I also have read the opposite, so... Stumped.

If I were to use something like I am proposing, then I would more than likely have a CollectionView capture the events.

[samccone]

I know I read a while ago that with jQuery, document was more performant at handling delegated events that delegating to loads of different nodes within the DOM. I also have read the opposite, so... Stumped.

Interesting! I did not know that, sound like you should perf it!

Looking forward to what you come up with :)

[jamesplease]

@designermonkey, I think that you're exaggerating the problem here. Very rarely (never?) will the wrapping element create a situation where the wrapping div would mess up, say, an HTML parser. The list example you gave above (with the ul) would only ever happen if you are unaware of the tagName property of views.

As far as CSS goes, if a wrapping div messes up your CSS then you are potentially writing overly-specific CSS.

I am also astounded at some of the responses regarding why it should be accepted to allow arbitrarily wrapping anything added by JavaScript in a div, because, well, just because.

The reason that it's "allowable" is that many people think that exchanging a nice-to-have – HTML with no extraneous parts – for something that is actually useful – a Javascript library that does a lot of the heavy lifting for you, is worth it. I would take the latter over the former any day of the week. Nobody is looking at my websites saying, "You know, this is a really nice web application...but I wish there weren't so many unnecessary divs in there!"

Fwiw, it's actually Regions that produce most of the unnecessary wrapping elements in my own apps, not views.

I'm in favor of removing the wrapping elements – just because it's doable – but not because it solves the Javascript equivalent of world hunger.

If it can be proven to work in an extension of Backbone, then maybe it can be back-ported into Backbone.

I can say with a fair degree of confidence that it will never land in Backbone as long as the current team is in charge of the library.

[designermonkey]

Not exaggerating at all, there's a real use case where it isn't helping at all.

The template

<script id="route-item" type="text/template">
<li class="route item" id="entry-{{ id }}" data-handle="{{ handle }}">
  <h2>{{ name }}</h2>
  <dl class="stats">
    <dt class="distance">Distance</dt>
    <dd class="distance"><span class="value">{{ distance.value }}</span> <span class="unit">{{ distance.unit }}</span></dd>
    <dt class="duration">Duration</dt>
    <dd class="duration"><span class="value">
    {{ duration.hours }}
    {{ duration.minutes }}
    </span> <span class="unit">{{ duration.unit }}</span></dd>
    <dt class="attractions">Attractions</dt>
    <dd class="attractions"><span class="value">{{ attractions.count }}</span> <span class="unit">{{ attractions.unit }}</span></dd>
  </dl>
  <nav>
    <a href="/routes/{{ handle }}">
      <i></i>
      <span>View Route</span>
    </a>
  </nav>
</li>
</script>

The View

View = Backbone.Marionette.ItemView.extend({
    tagName: 'li', // If I omit this, I get a wrapping div, or I get an extra wrapping li
    template: '#route-item' // The template *must* be a single node, so content is already wrapped with an li
});

This just isn't right really.

I've been doing this for nearly 15 years and have learnt one thing in my time: standards. HTML was standardised, CSS was standardised, JS is following suit. I just believe very strongly that all aspects of an application I and my team develop, should follow the same standards.

But anyway, I'm not here to argue with you guys, and I never said anything about world hunger, so please don't push my concern to that proportion. I'm just trying to help contribute to an already great piece of software.

I would take the latter over the former any day of the week.

Last point, I totally agree with this, which is why I've been trying so many JS frameworks, and this is the only one I've found that follows good solid design principles.

Like I said earlier, it's not Marionette's fault, but it can help fix it.

[samccone]

Like I said earlier, it's not Marionette's fault, but it can help fix it.

:+1: :clap:

[ianmstew]

@designermonkey Looks like we are finally solving this, just with a slightly different implementation, in 2.5. Instead of relying on the template to define the el, we are adding the option for a view's el to replace the region's el. That being the case I'm closing this, but please feel free to join the discussion over at #2625!

[jamesplease]

I think this should stay open. It's a separate issue from regions

[ianmstew]

Okay, I see your point. A better approach would be to ask the following:

Given the ability to replace a region's element coming in #2625, where does this issue stand?

[jamesplease]

@designermonkey makes a lot of good points, I think, and @tbranyen has successfully implemented this feature in LayoutManager. I think it'd be a huge win if we added this to Marionette!

[ianmstew]

Hmm, it does solve a different problem and I think you're right... it would be an instant fan-favorite. It would allow designers to write wrapper-less HTML without need editing tagName, className, id, and attributes in JS files.

[designermonkey]

One of the other added benefits would be server side markup matching templates too. I wish I had the time to contribute to open source more to help achieve this.

[ianmstew]

Well, it sounds like LayoutManager probably has a great example for this. @marionettejs/marionette-core any concerns before we start moving toward a PR here?

[ahumphreys87]

Im :+1: for this

On Mon, Jun 29, 2015 at 10:43 PM, Ian Stewart notifications@github.com wrote:

Well, it sounds like LayoutManager probably has a great example for this. @marionettejs/marionette-core https://github.com/orgs/marionettejs/teams/marionette-core any concerns before we start moving toward a PR here?

— Reply to this email directly or view it on GitHub https://github.com/marionettejs/backbone.marionette/issues/2357#issuecomment-116856195 .

[paulfalgout]

I think this will likely introduce some magic involving calling setElement after a render. It does seem doable, but we will want to be careful to not end up re-binding a bunch of things or not binding things that need to be bound.

[jridgewell]

My only concern here is setElement is a very expensive method call, and getting even more expensive.

[samccone]

We should keep in mind marionette is an extension lib not a fit all purpose framework. Whatever decision we end up with it should follow this thinking.

[megawac]

Another thing to consider is this will likely break many (almost every?, all the major ones I've ever worked on) applications currently in Marionette and be a pain to upgrade

[paulfalgout]

Hmm is this a change or an addition? As I understood it, it would be some new view configuration, but any current view would go one using tagName and it's template like normal

[jamesplease]

This wouldn't change the default behavior, so it wouldn't break anyone's apps: it would be an option that you must configure, sorta like template: false.

[frankcortes]

I'm only talking about my own experience. I have implemented/watched Marionette in very different projects, and one of the first things that the team notices about the implementation is always the same. Although Marionette is Backbone-based - so it should extend and not replace the basic functionality - , I agree with @designermonkey's motivation. In my view, Marionette should fill what Backbone lacks.

[frankcortes]

@jmeas, as a suggestion tagName: false doesn't mean anything for now.

[megawac]

Oh if its supplementary that would be awesome

[jamiebuilds]

I really think it's a poor idea to diverge from something so core as this. The more variation between views the more it difficult it gets to reason about all of their relationships to one another. You quickly get into a position of if x or else y being added all over the place and it's difficult to maintain.

I've never been hindered by there being a view.el, generally when it's not creating the DOM I would like, then there is something I'm doing wrong. I'm having a lot of difficulty thinking of somewhere that this isn't the case.

The provided example of:

<ul>
    <div>
        <li></li>
    </div>
    <div>
        <li></li>
    </div>

    ...

</ul>

is no exception. All you have to do is set your ItemView's tagName to li and remove the <li>s from your template. There's no problem here.

[jamesplease]

I think we should investigate how complex it is to support. If it's not so bad, then we should merge it. If it's really ugly, and seems likely to cause us problems down the road, then we can scrap it.

Let's get a prototype PR up.

[trezy]

Just had a convo about this with @paulfalgout and @ianmstew in Gitter. I think the same logic from the replaceEl PR could be used in the view to accomplish this. As @jridgewell mentioned, we may be looking at some pretty nasty performance hits fro doing it that way. However I think there may be a better way to do our event handling that would alleviate at least some of those pain points.

I'm busy tomorrow but later this week I'll try to put together a prototype if nobody else has jumped on it.

[designermonkey]

All you have to do is set your ItemView's tagName to li and remove the

s from your template. There's no problem here.

@thejameskyle but yes there is, as I then can't use a template, as a template requires a single node. I present the example above again.

<script id="route-item" type="text/template">
<li class="route item" id="entry-{{ id }}" data-handle="{{ handle }}">
  <h2>{{ name }}</h2>
  <dl class="stats">
    <dt class="distance">Distance</dt>
    <dd class="distance"><span class="value">{{ distance.value }}</span> <span class="unit">{{ distance.unit }}</span></dd>
    <dt class="duration">Duration</dt>
    <dd class="duration"><span class="value">
    {{ duration.hours }}
    {{ duration.minutes }}
    </span> <span class="unit">{{ duration.unit }}</span></dd>
    <dt class="attractions">Attractions</dt>
    <dd class="attractions"><span class="value">{{ attractions.count }}</span> <span class="unit">{{ attractions.unit }}</span></dd>
  </dl>
  <nav>
    <a href="/routes/{{ handle }}">
      <i></i>
      <span>View Route</span>
    </a>
  </nav>
</li>
</script>

I've read soooo many SO posts, blog posts, twitter posts all complaining about how JS frameworks wrap in divs, so there is obviously a perceived problem here.

[frankcortes]

@thejameskyle Since you must have part of your template (the container element) inside of your code, it could be considered as an issue. I understand the reason behind doing this but it should have an alternative.

[jridgewell]

I think there are two issues here:

1. DOM attributes on the root node don't update after render
2. The root node is defined in code instead of in a template

Issue One

The first one is easily solvable while still following core Backbone, and I think might actually solve @designermonkey's problem (correct me if I'm wrong).

<li class="route item" id="entry-{{ id }}" data-handle="{{ handle }}">

If that's the root node we're trying to create using core Backbone, I'd write it like so:

class View extends Backbone.View {
  attributes() {
    return {
      data-handle: this.model.get('handle')
    }
  }

  id() {
    return `entry-${this.model.get('id')}`;
  }

  className() {
    return 'route item';
  }

  tagName() {
    return 'li';
  }
}

Initialization will perfectly create my root li, but any later update's won't update the data-handle and id attributes. This can be easily fixed inside #render by updating id, class, and arbitrary attributes from #id, #className, and #attributes. Additionally we can support (though I wouldn't recommend) changing the root element's tag name.

Issue Two

The second issue is what I see being talked about in this thread. That is, defining both content and the root element inside the template. Like I've already pointed out, this is going to help people write some very slow renders.

There are two performance issues with this approach:

1. UI elements are scoped to the root element
2. DOM events attach to the root element, and that now happens on initialization for all views.

Perf Issue one can be completely mitigated if we assume our users aren't idiots (they aren't defining multiple "root" elements in the template).

{{! Good }}
<li class="route item" id="entry-{{ id }}" data-handle="{{ handle }}"></li>

{{! Bad }}
<li class="route item" id="entry-{{ id }}" data-handle="{{ handle }}"></li>
<span>Multiple root elements!</span>

Perf Issue two can be mitigated only after the initial render by solving Issue One (not perf issue one). If we just update the view's content with everything inside the template defined "root" element and throw away the template root. But there's no helping the initial render, it's just gonna be damn slow.

{{! Gonna throw the root away on additional renders }}
<li class="route item" id="entry-{{ id }}" data-handle="{{ handle }}">
  {{! And just grab all it's content }}
  <span>Content</span>
</li>

[designermonkey]

The issue I have is simply the wrapping. I generate initial content from the server as it's sooo much quicker, then get pissed off that all new content from models and views is wrapped by divs.

<ul>
    <li>Already</li>
    <li>From</li>
    <li>Server</li>
    <div>
        <li>Generated</li>
    </div>
    <div>
        <li>From</li>
    </div>
    <div>
        <li>Javascript</li>
    </div>
</ul>

Now, this can cause issues when a developer has followed web standards expectations of semantic markup and targeted with css like ul>li, which does happen (and I don't want to hear answers like, stop them doing it. That's not very helpful).

The <div/> other than being used for an events node is just cruft. I don't know how to solve it, and you may already be pointing in the right direction, but your answer so far @jridgewell so far seems like so much more work for me than just being allowed to declare that my el is my template (or at least a clone of it).

(Thanks for responding with an idea though)

[jamesplease]

@designermonkey, are you familiar with the tagName property of Views? Your root node can be an li.

var View = Marionette.ItemView.extend({
  tagName: 'li'
});

[paulfalgout]

We're going in circles a bit here https://github.com/marionettejs/backbone.marionette/issues/2357#issuecomment-75448937

However in that example I'm not sure why "The template must be a single node" @designermonkey why can't you remove the <li> from that template?

[designermonkey]

@jmeas, as previously posted

View = Backbone.Marionette.ItemView.extend({
    tagName: 'li', // If I omit this, I get a wrapping div, or I get an extra wrapping li
    template: '#route-item' // The template *must* be a single node, so content is already wrapped with an li
});

@paulfalgout it has to be a single node, as you can't add multiple nodes into a template? It's in the documentation for Backbone.

Also, why, when I have a template language at my disposal to fill in properties, must I write a load more code per View to assign attributes to a View's el when I should be able to declare it in my template?

I should be able to say:

Here View, here's a template node to use, just use it's root node for all your magical stuff and things.

I should not have to say:

Here View, here's a template to use, and also here's me having to write a load more JavaScript to recreate all that attribute stuff from the template's root node onto a node you just created for me, and then copy the contents of my template every time I call it, and dump it's perfectly defined root node, that you can already use, but prefer to wrap in another node you create every time I use you for some reason.

Bonkers.

[samccone]

We need to be clear here; this behavior comes directly from backbone and the ensureEl method. Changing this behavior would be a fundamental override, and a change to our current "hands off" approach with Backbone.View.

If we think this is a good idea, we also have to realize that we will be changing the well rooted backbone view interface: id, attributes, className, tagName.

What happens with these properties, do we just ignore them if someone does tagName: false?

I would reiterate what I have said before, Marionette is an extension library. I can see someone making a standalone "extended" view layer that has this behavior in an isolated repo, when they feel like they have something working in a sane way that also passes Marionettes tests we can evaluate it (if we think it is a general usecase).

In the end however this is a backbone thing, I would rather see this land in Backbone than in Marionette.

[jamiebuilds]

re: <dl><dt>...</dt><dd>...</dd></dl>

Since this is the only example where view.el would be a problem I would much rather it suck for dealing with this edge case than change something so core to Backbone view. Just put the whole thing in your template and re-render the whole view, the performance implications are most likely tiny.

[paulfalgout]

@designermonkey

It has to be a single node, as you can't add multiple nodes into a template? It's in the documentation for Backbone.

No I don't think this is true.. maybe you've read that about the <script> id being a single node, but for your original example this would work without issue:

<script id="route-item" type="text/template">
  <h2>{{ name }}</h2>
  <dl class="stats">
    <dt class="distance">Distance</dt>
    <dd class="distance"><span class="value">{{ distance.value }}</span> <span class="unit">{{ distance.unit }}</span></dd>
    <dt class="duration">Duration</dt>
    <dd class="duration"><span class="value">
    {{ duration.hours }}
    {{ duration.minutes }}
    </span> <span class="unit">{{ duration.unit }}</span></dd>
    <dt class="attractions">Attractions</dt>
    <dd class="attractions"><span class="value">{{ attractions.count }}</span> <span class="unit">{{ attractions.unit }}</span></dd>
  </dl>
  <nav>
    <a href="/routes/{{ handle }}">
      <i></i>
      <span>View Route</span>
    </a>
  </nav>
</script>

View = Backbone.Marionette.ItemView.extend({
    className: 'route item',
    id: function() {
       return this.model.id;
    },
    attributes: function() {
      return {
        'data-handler': this.model.get('handler')
      };
    },
    tagName: 'li', 
    template: '#route-item'
});

Is there issue with this solution?

I really think there are very few HTML issues that can't be resolved with Bb + Mn. But from a complexity and having DOM stuff on the view, it would be nice, however likely not practical for the root to be derived from the template.. However I don't think it is necessary.

[jridgewell]

Related: Backbone rejected this feature a few years ago https://github.com/jashkenas/backbone/issues/546. Of particular interest is Jeremy's response.

[designermonkey]

I'm very close to giving up on this whole thing as it seems I can't seem to make people understand what is so fundamentally wrong with div wrapping.

Also, the solutions proposed do indeed work, but they have to be considered a hack. Why? Because details I can and should describe in my template are having to be pulled across into JavaScript, therefore separating out my single template into two places.

Just because the current idea is to use it as an event target for binding, and just because a template can indeed have more than one sub-node, and just because you can re-create my example in JavaScript and a template working together doesn't mean it is right.

I've not been asking for us to make any fundamental changes to the way Backbone is expected to function, or monkey-patch it permanently from Marionette's perspective of use; I just proposed that Marionette become more agnostic and give developers an option to use the template node.

Anyway. I give up.

[jamesplease]

@designermonkey, I'm sorry that you're frustrated! I understand that it can be annoying to try to prove your point. But I'm on your side here, so I'm gonna reopen this - it's worth thinking more about.

Do note that if you don't want to follow along you can unsubscribe to the right.

[samccone]

@jmeas do you have a recommendation as to an approach here?

[jridgewell]

Here's a minimal gist to accomplish template defined root elements.

I'm going to open a PR against core Backbone for #_attributes to easily allow devs to update the root element's attributes after a render (my issue one above). Unfortunately, nothing else is appropriate for core, since rendering logic is a dev issue.

[Tvrqvoise]

Wait a sec. Maybe I'm just not getting things here, but how would this be different from using the region as $el? If these two features accomplish the same end result, yet are incompatible and this one has the potential for serious performance impacts, I'm -1 for this feature.

[designermonkey]

Because a region is a region. It is not a view. This behaviour is needed on the lowest level views.

[ambischof]

@Tvrqvoise I myself have come across this solution, however inelegant.

It would be really nice if region el s could be leveraged similarly to a CompositeView's childViewContainer, however then you either have to rewrite the region's show/attachHtml code or manually bind the currentView to the view and then all your event bindings have to be manually .. and all the native region events don't... Excuse me while I have a flashback...

I guess my point is, this is totally a doable option but right now too much of the region's core functionality is too tightly embedded in the show function for this to be a no-brainer solution.

If you've come up with a SIMPLE way to use a region's el as the view el that doesn't conflict native features, I'm all ears.

[Tvrqvoise]

@ambishof, it looks like #2625 would provide that functionality.

[hashchange]

I have built a solution for Backbone which respects how the el is normally created – ie, early on, before initialize() is called. It plays nice with frameworks built on top of Backbone, like Marionette.

You can drop it into an existing project without changing a thing, and then turn it on for individual templates on a case-by-case basis. There isn't any negative impact on performance, unlike an approach based on setElement(). Have a look if you wish: Backbone.Inline.Template.

The plugin solves two of the three issues discussed here:

• Markup and Javascript are kept apart.
• You can use the same templates, client-side and server-side, when your backend expects fully formed templates.

The remaining problem which I didn't address:

• You can't use template variables for the el itself (as in the example by @designermonkey).

For that last one, there is no easy fix. I went with the "early el strategy". That early in the process, there is no generic, template-agnostic way to incorporate template variables for the el. (I can imagine extensions which would be able to handle that, but they would be specific to a project.)

So the plugin doesn't solve the problem for everyone, but I suppose it might be helpful for at least some of the people in this thread.

[paulfalgout]

If anyone wants to weigh in on this issue #3259 is a possible solution. Beyond that, I don't know that there's a good way to accomplish this outside of removing Backbone.