teropa/build-your-own-angularjs

Source Code & Errata for the "Build Your Own AngularJS" book. http://teropa.info/build-your-own-angular

build-your-own-angularjs's Issues

Typo on page viii

If I'm not mistaken, this:

"We’re enabling the browser and devel JSHint environments, which will cause it to now raise errors when we refer to global variables commonly available in browsers, such as setTimeout and console."

Should be this:

"We’re enabling the browser and devel JSHint environments, which will cause it to not raise errors when we refer to global variables commonly available in browsers, such as setTimeout and console."

That is, "...cause it to now raise errors..." should be "...cause it to not raise errors...".

P11 - spec is failing with jasmine.createSpy().andReturn('something');

When I run this example I'm getting a typeError : 'undefined' is not a function (evaluating 'jasmine.createSpy().andReturn('something').

If I remove the andReturn call this test will pass.

Also, in the paragraph beneath the example you mention the following :
"When the scope is digested our current implementation throws an
exception" and you explain that this is occurring because it's trying to invoke a non-existing listener. However, if I leave off the no-op function addition and just remove the andReturn call the test still passes.

Pix Missing jshint config

When you set up the jasmine task you left out the jshint options. The other snippets show it.

Other possibility is that you left it out to make the the snippet shorter, but I would add a comment saying that the options are omitted.

EDIT: Ouch, 3 issues and I had to open one that already exist. Sorry.

Percentage sign and black background makes it difficult to copy command examples

In this screenshot I think I selected the command, but as you see it's hard to tell. In addition, if you triple click on Mac to select all, it includes the percentage sign. This makes copy pasting more difficult. This is using Preview on Mavericks.

My suggestion would be to leave out the percentage sign or somehow separate it from the command in a way that triple click would skip it. I would also use a lighter background.

P17 typo

In the $digest snippet:

lastDirtyWatch = null should be
this.$$lastDirtyWatch = null

cheers.

P24 Describe reasons for the existence of $evalAsync better

Gruntfile.js on page ix does not include everything it had in the previous version.

I expected the jshint options to include:

 browser: true,
 devel: true

These were added in the previous step.

Add illustration of watch depths

"$: false" should not be highlighted on page ix

After adding grunt-contrib-jasmine the line with $: false is highlighted suggesting it is new, but it was already added in the previous step.

Fix links in table of contents

Should always submit for the correct page.

Fix $$lastDirtyWatch shadowing issue

$$lastDirtyWatch is assigned on the current scope in $$digestOnce, which shadows the parent's attribute. There should be one $$lastDirtyWatch per scope hierarchy.

P53 Another missing highlight

In the first snippet there should be an highlight in:

child = new ChildScope();

Since it doesn't have var anymore.

P40 Better explain the reason to set lastDirtyWatch to null

Explain usage of setTimeout in $evalAsync

In the section $evalAsync - Deferred Execution you wrote:

The usual way to do this is by calling setTimeout() with a zero (or very small) delay parameter.
This pattern applies to Angular applications as well, though the preferred way to do it is by using the $timeout service that, among other things, integrates the delayed function to the digest cycle with $apply.

This is followed by an explanation of why using setTimeOut - or its derived $timeout - is not always a good idea:

The reason why $evalAsync is often preferrable to a $timeout with zero delay has to do with the browser event loop. When you use $timeout to schedule some work, you relinquish control to the browser, and let it decide when to run the scheduled work.

This makes sense, however in the implementation of $evalAsync you're using setTimeOut:

Scope.prototype.$evalAsync = function(expr) { 
  var self = this;
  if (!self.$$phase && !self.$$asyncQueue.length) {
    setTimeout(function() {
      if (self.$$asyncQueue.length) {
        self.$digest();
      }
    }, 0); 
  }
  this.$$asyncQueue.push({scope: this, expression: expr});
};

Can you explain why you're using setTimeOut here? Maybe I misunderstood something. If it's just a simplification, then you should probably mention that below this code.

Figure out if there's a better way to handle exceptions until exception handling / logging is introduced

The current logging is mixed with Jasmine test output

Move "Scheduling $evalAsync from Watch Functions" to appendix?

I haven't read beyond this yet, so I don't know if it's needed elsewhere in the book, but if not, I would move it away from chapter 1. It just makes things more complicated and the chapter is already pretty long.

Just keep the phrase "Granted, this is something one should not do, since watch function are supposed to be side-effect free." and refer readers to the appendix if they care.

P46 Mention difference in the storage of $$children to Angular proper

Initialize watches with a value guaranteed to be unique, not undefined

P8 Jasmine URL points to old Jasmine website

Should the "Jasmine Spy" link point to http://jasmine.github.io/ rather than old URL?

P13 Clarify watch ordering

Iterate watch array in reverse order during digest

To make the behavior identical with AngularJS in cases where watches are added or removed during the digest, add new watches to the beginning of the array and iterate it starting from the end.

P39 Reword Chapter 2 introduction

Clumsy phrasing used.

P48 Two changes not highlighted

In the last snippet, there are two changes that are not highlighted:

scope.$$lastDirtyWatch = watcher;

and

} else if (scope.$$last.DirtyWatch === watcher) {

AngularJS change: Add ability to handle NaNs to $watchCollection

Applying angular/angular.js#4605

Enable JSHint for tests

Migrate to Jasmine 2.0

P ix Gruntfile.js example missing jshint options

Gruntfile.js example on page viii shows jshint config as:

jshint: {
all: ['src/*/.js'],
options: {
globals: {
_: false,
$: false
}}}});

but on next page ix - the example of the same Gruntfile.js file shows jshint config as:
jshint: {
all: ['src/*/.js']
},

Looks like it is missing the options...

In value comparison, clone value before running listener function

If the value is changed by the listener function, that changed value is saved as the "last value". We should clone it before the listener to prevent this.

P146 typo on snippet comment

the last snippet: s/invlaid/invalid/

P99 Typo in title

The basics if $emit and $broadcast

should be

The basics of $emit and $broadcast

P105 typo on test name

On the last test (for $broadcast) you put $emit.

Damn copy & pastes, one can't be sure that they do the job. I found myself with broken tests because I forgot to change something after a paste :P

P18 wrong array index in test specification

Due to the fact, that you iterate reverse through $$watchers, the array index in the test specification of "ends the digest when the last watch is clean" should be 99 (in contrast to 0), so I suggest you write:

scope.array[99] = 420;

Split Chapter 1

Chapter 1 is quite long, and could be split into two:

1. Dirty checking
2. $eval/$apply/$evalAsync/$$postDigest/Phases

P127 wrong snippet header

The first snippet says src/parse.js instead of test/parse_spec.js

P33 moved line of code

The last snippet, the line where we assign the last value to the watcher is now outside the inner if and put after the else if. The last time we saw that code (page 20 or so) had it inside the inner if.

P140 parameter not needed yet

First snippet has the quote parameter on it, that is wrong :P

Enable strict mode in all files

P9 Suggestion: missed TDD opportunity

The sidebar at the top of page 9:

Notice that we’re growing the watchers array at the beginning and iterating it in reverse order. Doing this will make our lives a bit easier when we implement watch removal later in the chapter.

But TDD makes no assumptions about the API and implements the simplest production code. When you get to "watch removal later in the chapter," then you can illustrate how the pressure from the tests on the design leads to a nice refactoring to Array.prototype.unshift and while.

This kind of valuable teaching moment on TDD is rarely seen in books like yours that deal with authentic code and not toy examples. No books on JavaScript teach TDD. GOOS is the best example of this in Java.

Suggestion: remove the sidebar and use Array.prototype.push and Array.prototype.forEach, i.e., replace these snippets on page 8:

Scope.prototype.$watch = function(watchFn, listenerFn) {
  var watcher = {
    watchFn: watchFn,
    listenerFn: listenerFn
  };
  this.$$watchers.unshift(watcher);
};

Scope.prototype.$digest = function() {
  var length = this.$$watchers.length;
  var watcher;
  while(length--) {
    watcher = this.$$watchers[length];
    watcher.listenerFn();
  }
};

with these:

Scope.prototype.$watch = function(watchFn, listenerFn) {
  var watcher = {
    watchFn: watchFn,
    listenerFn: listenerFn
  };
  this.$$watchers.push(watcher);
};

Scope.prototype.$digest = function() {
  var that = this;
  this.$$watchers.forEach(function(watcher) {
    watcher.watchFn(that);
    watcher.listenerFn();
  });
};

Add links to JSBins?

Stop calling Lo-Dash Underscore

P22 Example Issue

Here's a description for the issue.

P11 Example Issue. Chapter1. Third line. "iself" instead of "itself"

More information on book updates

Hi Tero,

Your messages with the issues fixes are not clear enough, I mean, you know what you fixed, but we don't. In other words, your messages are really really short so when you fixed some stuff, I need to check all the scope.js file step by step to see what changed.

Add the pages you changed (if you changed code or tests) and where exactly. Sometimes you say, I changed XXX to be more angular-ish, but not sure if it was a bug in the text or you actually modified something in the code.

TL;DR; Make it easy to the early readers to keep track of changes.

P25 capitalisation consistency

From the start of the book your unit test descriptions have all been lower case (e.g. "correctly handles NaNs") but from page 25 onwards you have begun capitalising the first character in the description (e.g. "Executes $eval'ed function...").

Would be good to have it consistent.

Otherwise, great book so far!

P viii Typo

I believe the phrase "should now raise errors when we do so" should actually be "should (not) raise errors when we do so".

P44 Refer to the existence of the "dot rule"

Mention that it is a well-established pattern

P111 Typo on the test

You have en extra it on the test title.

Bonus for chapter 0: notify_hooks

Perhaps you can point readers to this library and other useful tools at the end of the chapter, without going into detail about them. This one pops up a notification on your desktop (depending on the OS) if any of the tests fail.

P26 Keep digesting while there is something in the async queue

Also prevent infinite loop, as in angular/angular.js#2622

Typo page 23 - "expection" should be "expectation"

That is, unless it's a word I've never heard of.