How we made a

mess

of our Angular app

Dave Smith

@djsmith42

“It is not the critic who counts; not the man who points out how the strong man stumbles, or where the doer of deeds could have done them better. The credit belongs to the man who is actually in the arena, whose face is marred by dust and sweat and blood; who strives valiantly; who errs, who comes short again and again, because there is no effort without error and shortcoming"

Theodore Roosevelt

2012

2.5

months

50% less code!

unit tests!

2015

Forgive me?

This is my list.

 

You will have your own.

 

Let's learn together.

Abusing $scope inheritance

<div ng-controller="MainController">
  <!-- lots of code -->
  <div ng-controller="ChildController">
    <!-- lots of code -->
    <div ng-controller="GrandchildController">
      <!-- lots of code -->
    </div>
  </div>
</div>
function GrandchildController($scope) {
    $scope.newThing = {...};

    $scope.saveClicked = function() {
        $scope.$parent.$parent.things.push($scope.newThing);
    }
}
function MainController($scope) {
    $scope.$watch("things", function() {
        $http.put("/api/things", $scope.questions)
    }, true);
}

Why this is bad

<div ng-controller="MainController">
  <!-- lots of code -->
  <div ng-controller="ChildController">
    <!-- lots of code -->

    <div ng-if="foo"> <!-- ruh roh! -->

      <div ng-controller="GrandchildController">
        <!-- lots of code -->
      </div>

    </div>
  </div>
</div>

Why this is bad

  • Hard to reason about
  • Business logic in controller
  • How do you grep for this?

What we should have done

  • A shared data service
    • Responsible for updating and retrieving data via $http.
    • Controllers call into that service.
  • ​Why?
    • More direct and obvious
    • Centralized business logic

Global controller functions

function MyController($scope) {

}

Why this is bad

  • Naming collisions
  • Can't package controllers into modules
  • Encourages GLOF and GLOG
  • Not supported by default in 1.3

What we should have done

angular.module("MyControllers")
.controller("MyController", function($scope) {

});
  • Why?
    • More modular
    • The global scope is already polluted enough

What we should have really done

No ng-controllers at all

Angular controllers aren't really that modularizable and are dying in Angular 2.0 anyway

Scope shadowing

function MyController($scope) {
    $scope.favoriteFood = "stroopwafel";
}
<div ng-controller="MyController">
  <input type="text" ng-model="favoriteFood" />
</div>
<div ng-controller="MyController">
  <div ng-if="someVariable">
    <input type="text" ng-model="favoriteFood" />
  </div>
</div>

Some unfortunate developer makes a tiny change

Why this is bad

  • You will introduce bugs into your app by looking at it.
  • Subtle and very hard to find bugs
  • No unit test can save you.
  • We shed many Angular-shaped tears over this.

Why does this happen?



expect($scope.$parent === $scope.__proto__).toBeTrue();
{
  favoriteFood: "stroopwafel",
  someVariable: true
}
angular.module("MyModule")
.controller("MyController", function($scope) {
  $scope.favoriteFood = "stroopwafel";
  $scope.someVariable = true;
});
{
  favoriteFood: "herring"
}
<div ng-controller="MyController">
  <div ng-if="someVariable">
    <input type="text" ng-model="favoriteFood" />
  </div>
</div>

User enters

text "herring"

What we should have done

angular.module("MyControllers")
.controller("MyController", function() {
  this.favoriteFood = "stroopwafel";
  this.someVariable = true;
});
  • Why?
    • Eliminates scope shadowing
<div ng-controller="MyController as vm">
  <div ng-if="vm.someVariable">
    <input type="text" ng-model="vm.favoriteFood" />
  </div>
</div>

Alternatively

angular.module("MyControllers")
.controller("MyController", function($scope) {
  $scope.vm = {
    favoriteFood: "stroopwafel",
    someVariable: true
  }
});
<div ng-controller="MyController">
  <div ng-if="vm.someVariable">
    <input type="text" ng-model="vm.favoriteFood" />
  </div>
</div>

Why does this fix it?

angular.module("MyModule")
.controller("MyController", function() {
  this.favoriteFood = "stroopwafel";
  this.someVariable = true;
});
<div ng-controller="MyController as vm">
  <div ng-if="vm.someVariable">
    <input type="text" ng-model="vm.favoriteFood" />
  </div>
</div>

User enters

text "herring"

{
  vm: {
    favoriteFood: "stroopwafel",
    someVariable: true
  }
}

JavaScript tries to read

"vm" here. Not found.

JavaScript tries to read
"vm" here. Found.

{
  vm: {
    favoriteFood: "herring",
    someVariable: true
  }
}

In other words:



    <input type="text" ng-model="vm.favoriteFood" />


   $scope.vm.favoriteFood = "herring";

   var tmp = $scope.vm;
   tmp.favoriteFood = "herring";

User types "herring"

Read the prototype chain to find $scope.vm

Write the value to $scope.vm.favoriteFood.

$scope.$watch for user events

function MyController($scope, $http) {
    $scope.id = 42;
    $http.get("/api/employees/" + $scope.id)
        .success(function(employee) {
            $scope.employee = employee;
        });
    $scope.$watch("employee.title", function(newTitle, oldTitle) {
        if (angular.isDefined(newTitle) && 
               angular.isDefined(oldTitle)) {
            // User changed the title
            $http.put("/api/employees/" + $scope.id, {
                title: newTitle
            });
        } else {
            // Just initial load or first $http.get(). Ignore.
        }
    });
}

Why this is bad

  • Difficult to differentiate between user events and programmatic events
    • User click vs. $http response
  • Can lead to duplicate PUT requests on page load
    • Very frequent bug for us

What we should have done

  • Use ng-change
  • Use events
  • Flux pattern?

Why?

  • One-way data flow is simpler

Rich Hickey on simplicity: https://www.youtube.com/watch?v=rI8tNMsozo0

ng-include

<div ng-controller="MyController">
  <div ng-include="'/partials/bar.html'">
  </div>
</div>
<div>
  {{foo}}
</div>

/partials/bar.html:

  • Slow
  • Context is hard to determine
  • New scope
  • Undefined interface for callers

Why it's bad

What we should have done

Element directives

Why?

  • Well defined interface
  • Easy to grep and reason about
  • Components are the future

Passing $scope to a service

function MyController($scope, myService) {
    myService.setFoo($scope);
}

app.service('myService', function() {
    this.setFoo(scope) {
        scope.$watch('thing', function() {
            ...
        });
    }
});

Why it's bad?

  • Leaky abstraction
  • Your services get view knowledge

What we should have done

  • Pass values to services, not $scopes.
  • Services can accept callbacks and/or return promises to notify callers of changes.

Home grown build tools

  • The "cat" command is not a build system
  • GLOF:
      
    Giant List of Files
  • GLOG:
      Giant List of GLOFs

     

Why this is bad

  • Easy to break your app through manual dependency management.
  • It's difficult to integrate tools like traceur.
  • Managing <script> tags manually is painful and error prone.

The ES6 future can be now with the right build tools

What we should have done


   dsmith@laptop:~$ npm install tardis
   dsmith@laptop:~$ tardis travel --year 2015
   dsmith@laptop:~$ npm install webpack
   dsmith@laptop:~$ tardis travel --year 2012

Even then, we'd still have a GLOF.

 

How to solve this?

ng-controller

Most of our Angular mistakes stem from using ng-controller

What mistakes have you made?

Tweet them
with #ngoops.

Let's learn and grow together.

How we made a mess of our Angular app

By djsmith

How we made a mess of our Angular app

  • 8,105