51

As I read through the Angular tutorials, I really like a lot of it, but isn't "ng-click" the equivalent of an inline onClick? My understanding was that the JavaScript community had determined inline JavaScript event handlers in your HTML was "bad practice."

<img ng-src="{{img}}" ng-click="setImage(img)">

It would be great to know why this is no longer considered "incorrect" when using Angular.

Source: http://docs.angularjs.org/tutorial/step_10

j08691
  • 204,283
  • 31
  • 260
  • 272
AndrewHenderson
  • 4,686
  • 3
  • 26
  • 33
  • i dont like anything inline personally and would avoid anything like that, though this is sort of an opinion question. Theoretically if someone uses inline if the code still works I guess someone could use it. If it were me I would avoid any HTML defined inline events though. – sajawikio Feb 10 '13 at 01:32
  • I never used angularjs but it looks like a templating framework so I'd have to look more to be sure. If it is rendered on HTML it may or may not have an "ng-click" attribute at all, are you sure it is really inline, or does it create a JS which binds the event when the template is "generated"? I'm not so familiar with Angularjs though, probably it just does the event inline, but supposedly it is doing templating for you which might be why one wants to use it (though personally I dont think I would like that library, again, opinion question so ppl may be downvoting this question). – sajawikio Feb 10 '13 at 01:36
  • 3
    It's not actually inline. Setting the attribute's content to ``setImage(img)`` will not actually perform the binding. A script parsing the template will. – nikola Feb 10 '13 at 01:48
  • Very similar question: http://stackoverflow.com/questions/14346073/angularjs-is-ng-click-a-good-practice-why-is-there-no-ng-event-in-angularj – Mark Rajcok Feb 10 '13 at 04:08
  • 1
    AngularJS (and ReactJS) are obtrusive javascript frameworks which couple the static data layer (HTML) to the logic (Javascript) needed to process that data. This is bad practice. You're essentially locking your developers into a Google (or Facebook) product. Example: You spent 2 years writing an AngularJS app, and now you want to move to ReactJS because it has faster performance. If you had decoupled the logic from the data, you could reuse your HTML templates. But, now you're stuck in continuing to use AngularJS or rewriting your front-end with ReactJS. – tim-montague Feb 08 '15 at 23:22

1 Answers1

60

Really, it all comes down to the fact that your view code has to be hooked into your application logic somehow. Best practices for AngularJS generally state that you should write your own models--objects that represent your business domain--and attach them to the scope. Imagine some code like this:

<img ng-src="{{img}}" ng-click="myProfile.setMainImage(img)">
myApp.controller("ProfileController", function($scope, myProfile) {
  $scope.myProfile = myProfile;
});

The view says "when this image is clicked, it will call setMainImage() on myProfile." The business logic is inside myProfile, where it can be tested, etc. The view is just a hook.

In a more "traditional" or "vanilla" jQuery setup, you'd have to write something like the following:

$("#someId img").on('click', function() {
  var img = $(this).attr('src');
  myProfile.setMainImage(img); // where does myProfile come from here?
                               // how does it update the view?
});

Of course, the JavaScript community has determined that writing large applications in this fashion isn't really tenable, in part because of the disconnect between the views and the model objects (as indicated by the comments in the code snippet), which is why we have frameworks like Angular in the first place.

So, we know this native jQuery code is not ideal, but we're still not sure about the whole ngClick thing. Let's compare it to another very popular JavaScript framework that provides an MV* architecture, Backbone. In a recent RailsCasts episode on AngularJS, someone asked a very similar question:

Is it just me, or AngularJS looks so a bad idea? Ryan, don't get me wrong, the episode was great, but I'm not convinced by the framework.

All that ng-show, ng-repeat, ng-class are looking like the old Java's JSF, and similar frameworks. It also enforces obtrusive JS with ng-submit and ng-click.

So my point is: your view will easily become cluttered and totally dependent on it. The advantage of other frameworks like Backbone, is to have a separation of concerns between the presentation and the behavior (less or no dependencies), and a structured client side application (MVVM).

My response is applicable here as well:

In a framework like Backbone, you'd have something like the following code (taken from the Backbone website, minus a few lines):

var DocumentView = Backbone.View.extend({

  events: {
    "dblclick"                : "open",
    "click .icon.doc"         : "select",
    "contextmenu .icon.doc"   : "showMenu",
    "click .show_notes"       : "toggleNotes",
    "click .title .lock"      : "editAccessLevel",
    "mouseover .title .date"  : "showTooltip"
  },

  open: function() {
    window.open(this.model.get("viewer_url"));
  },

  select: function() {
    this.model.set({selected: true});
  },

});

In this object which is a view, you are setting up event handlers on various elements. These event handlers call functions on the view object, which delegate to models. You also set up callbacks on various model events (such as change) which in turn call functions on the view object to update the view accordingly.

In Angular, the DOM is your view. When using ng-click, ng-submit, etc., you are setting up event handlers on these elements, which call functions that should delegate to model objects. When using ng-show, ng-repeat, etc. you are setting up callbacks on model events that change the view.

The fact that AngularJS sets up these [hooks and] callbacks behind the scenes for you is irrelevant; the only difference between this and something like Backbone is that Angular lets you write your view declaratively--you describe what your view is--rather than imperatively--describing what your view does.

So, in the end, <a ng-click="model.set({selected: true})"> really adds no more dependencies than

events: {
  'click a': 'select'
},

select: function() {
  this.model.set({selected: true});
}

...but it sure is a hell of a lot less code. ;)

(Note: really, the Angular version should be <a ng-click="select()">, and the select method on the scope would be like the select method in the view in the Backbone example.)

Now, perhaps a legitimate concern is that you don't like the event hooks in your markup. Personally, I greatly prefer the declarative nature of Angular's views, where your markup describes what the view is and you have two way binding between events (whether they be user generated or simply changes in the model) and your views--I find I write far less boilerplate code to hook up events (especially changes in the view driven by changes in the models), and I think it's easier to reason about the view in general.

Community
  • 1
  • 1
Michelle Tilley
  • 157,729
  • 40
  • 374
  • 311
  • Thanks for the thorough response. That definitely helps clear things up. You also touched upon many of the gripes I have with Backbone. I find myself writing a lot of boilerplate code. – AndrewHenderson Feb 10 '13 at 03:23
  • 14
    One other addition reason I feel safer with Angular's declarative event definition is that a method listing ng-click is typically located on the $scope of that view's controller, while an inline onClick in vanilla HTML/JS is a global method call. – checketts Feb 10 '13 at 03:59
  • Good point. I agree. That is an important distinction. – AndrewHenderson Feb 10 '13 at 04:36
  • 4
    The scope is the reason angular is OK and inline onclick is bad IMO – Guillaume86 Feb 13 '13 at 18:46
  • 2
    I might catch some flack for saying this but what happens when you have another item you want to reuse the code? Do you now have to go into each item and add ng-click="model.set({selected: true}) into every thing you want to use it. What happens if you want to change stuff now you have to edit the html to make changes to javascript instead of just editing the behavior as a separate issue. I want to go all in on angular but I still have some concerns about how overly dependent it is on the HTML. – Chris Aug 19 '13 at 19:53
  • @AdonisSMU Good question! Your Angular hooks shouldn't really be `model.set({selected: true})`, it should be more like `item.select()` (or whatever the appropriate verb is). You shouldn't have business logic directly on your scope, and you shouldn't access it directly from the DOM--your `ng-clicks` should indicate *what you want to do* (select the item, etc.), and what that *means* should be relegated to your model layer (via Angular services). I've updated the answer with a note. – Michelle Tilley Aug 19 '13 at 22:10
  • 6
    But if you end up having to bind that same event to multiple elements on the page then you need to attach that `ng-click` to all of them individually, right in the markup. I feel like I've spent the last 5 years learning that inline javascript like this (and don't try to convince me it's not - you are literally calling the method `select()` right from the element definition in your HTML) is bad practice, only to be told that "it's ok as long as it's a javascript framework encouraging it!". It just... feels *wrong*. What happened to separation of concerns? – nzifnab Sep 04 '13 at 23:42
  • @nzifnab Why is inline JS bad? (I'm not saying it's not, but answering this will help bring home how Angular is different. See first part of [this](http://stackoverflow.com/a/6015702/62082)). As for separation of concerns, I *personally* feel that separation of the template and view (to use Backbone terms) makes for interaction bugs that are non-obvious and difficult to debug; the events are too far from the template. I think *declarative interfaces are easier to use*. I *do* agree that the interaction and the business logic should be separated, which is the purpose of services and `$scope`. – Michelle Tilley Sep 05 '13 at 00:02
  • 2
    @nzifnab To use the Backbone example, what if every link shouldn't trigger the `select` event? You can add a class to the template and selector, but then it's quite easy to accidentally change the class in the template without realizing that you broke your view. You can add some other `data-` attribute, but then how is that different that having to copy-paste an `ng-click` everywhere when you want to add a new link? `ng-click` etc. are *bindings to methods on a non-global object* (`$scope`), written in an obvious and declarative fashion. (Copy/paste issues can also be helped via directives.) – Michelle Tilley Sep 05 '13 at 00:06
  • 1
    @AndréAlçadaPadez Most of the code in this post is only pseudocode to demonstrate ideas. If you have a specific question about a specific issue, please post it as a new question on Stack Overflow. (Unless I entirely misunderstood your question, in which case, please feel free to correct me.) – Michelle Tilley Feb 03 '14 at 17:01