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