Ripple Client refactoring notes, May 6th 2013

gistfile1.md

Turning client classes into services/controllers

Old	New	Comment
src/js/client/model.js	src/js/controllers/app.js	The model stuff was moved to a global controller.
src/js/client/status.js	src/js/controllers/status.js	The status module is now a controller, see "Status controller" below.
src/js/client/network.js	src/js/services/network.js
src/js/client/id.js	src/js/services/id.js
src/js/client/blob.js	src/js/services/blob.js
src/js/client/angular.js	src/js/entry/desktop.js	Which modules are used might be target-specific, so the loading has been moved to entry/.
src/js/client/app.js	(removed)	Angular does that job for us now.

Global "rippleclient"

The rippleclient global variable still exists and works much the same way it used to.

However, it is no longer acceptable to use it in the code. It is for console debugging only!

rippleclient.$scope was renamed rippleclient.scope for easier typing.

Tab events

The tab events like afterrender, beforeshow, reset and retrigger are removed. This is in preparation of removing the Tab class altogether. (See "Remaining refactoring" below.)

$apply and $digest

There was a lot of unclean/random $digest-ing and $apply-ing going on. (Some of that stuff is still in there, feel free to fix when you encouter it.)

Whenever you handle an event from OUTSIDE of ripple-client (usually ripple-lib), always (!) add a $scope.$apply around the whole event handler, e.g.:

request.on('success', function (res) {
  $scope.$apply(function () {
  
  });
});

Anything happening INSIDE of ripple-client, e.g. $scope.$on('$appTxNotification, ...) should already be inside of an $apply. If it isn't that means someone broke the first rule above.

A special case are OUTSIDE MAYBE-SYNCHRONOUS events. Those are events that have an asynchronous API, but may call their callback synchronously. In that case, wrap the event callback in a setImmediate call. Example:

doSomethingMaybeSynchronous(myArg1, myArg2, function () {
  setImmediate(function () {
    $scope.$apply(function () {
    
    });
  });
});

The reason we need this is because if the call happenes synchronously, it is already wrapped inside of an $apply and we'll get an error. Forcing the callback to be asynchronous ensures that never happens.

As long as we follow this consistently, we should never ever have to write this:

if ($scope.$$phase) {
  $scope.$digest();
}

This should also make it so we rarely, if ever, need $digest. Note that $apply not only takes care of bindings, but of error handling as well, that's why it is preferred.

Example: Handling a transaction result

var tx = $network.remote.transaction();
tx
    .ripple_line_set($id.account, amount)
    .on('success', function(res){
      $scope.$apply(function () {
        // do stuff here
      });
    })
    .on('error', function(){
      setImmediate(function () {
        $scope.$apply(function () {
          // do stuff here
        });
      });
    })
    .submit()
;

Note that we use exactly one $scope.$apply per handler. No multiple $digests, no $$phase, etc.

The tricky part is the error event, because ripple-lib currently may trigger errors synchronously. That's why we need the setImmediate - this ensures that the callback is always asynchronous.

See also

• http://jimhoskins.com/2012/12/17/angularjs-and-apply.html
• http://stackoverflow.com/questions/9682092/databinding-in-angularjs/9693933#9693933
• http://docs.angularjs.org/api/ng.$rootScope.Scope

Event management

The client before the refactor made heavy use of webpack's version of Node.js' EventEmitter class. All such references have been removed. We now use Angular's $scope-based event system.

Before:

id.emit('accountload');

After:

$scope.$broadcast('$idAccountLoad');

Note that when handling events, you no longer need to worry about $apply or $digest at all. For example:

Before:

id.on('accountload', function () {
  // do stuff
  
  // Maybe need to $digest? Maybe not? We don't know! Big confusion!
  if (Math.random() > 0.5) $scope.$digest();
});

After:

$scope.$on('$idAccountLoad', function () {
  // do stuff, enjoy peace of mind
});

Obviously this change concerns only events coming from within the client, ripple-lib still uses EventEmitters as before.

More explicit dependency management

So far, we basically declared all modules globally in src/js/angular.js, so everything was available everywhere.

However that makes testing slow and painful. Instead, we'd like to declare only the dependencies that a controller, service or directive actually needs.

I've started to do that, basically what this means is that rather than adding validators as a module dependency in src/js/entry/desktop.js, we should add it to any tab's angular dependencies that uses a directive from that module.

Status controller

The status box (top right) is now being managed by a controller. This isn't the cleanest solution, but it should allow for unit testing. Eventually, we should probably make it a directive instead.

Remaining refactoring

There are some things that still aren't what they should be.

• The Tab class isn't really needed anymore, we should remove it and change all tabs to be pure Angular modules.
• The status controller should be a directive probably, not a controller.