refactor(routing): simplify the code, add comments

Author: vicb

lib/routing/ng_view.dart

@@ -65,36 +65,38 @@ class NgView implements DetachAware, RouteProvider {
   final DirectiveInjector _dirInjector;
   final Element _element;
   final Scope _scope;
-  RouteHandle _route;
+  RouteHandle _parentRoute;
 
   View _view;
   Scope _childScope;
   Route _viewRoute;
 
-  NgView(this._element, this._viewCache, DirectiveInjector dirInjector, this._appInjector,
-         Router router, this._scope)
-      : _dirInjector = dirInjector,
-        _locationService = dirInjector.getByKey(NG_ROUTING_HELPER_KEY)
+  NgView(this._element, this._viewCache, this._dirInjector, this._appInjector,
+         Router router, this._scope, this._locationService)
   {
-    RouteProvider routeProvider = dirInjector.getFromParentByKey(NG_VIEW_KEY);
-    _route = routeProvider != null ?
+    RouteProvider routeProvider = _dirInjector.getFromParentByKey(NG_VIEW_KEY);
+    // Get the parent route
+    // - from the parent `NgView` when it exists,
+    // - from the router root otherwise.
+    _parentRoute = routeProvider != null ?
         routeProvider.route.newHandle() :
         router.root.newHandle();
     _locationService._registerPortal(this);
     _maybeReloadViews();
   }
 
+  /// Reload the child views when the `_parentRoute` is active
   void _maybeReloadViews() {
-    if (_route.isActive) _locationService._reloadViews(startingFrom: _route);
+    if (_parentRoute.isActive) _locationService._reloadViews(startingFrom: _parentRoute);
   }
 
   void detach() {
-    _route.discard();
+    _parentRoute.discard();
     _locationService._unregisterPortal(this);
     _cleanUp();
   }
 
-  void _show(_View viewDef, Route route, List<Module> modules) {
+  void _show(_View viewDef, Route route) {
     assert(route.isActive);
 
     if (_viewRoute != null) return;
@@ -110,14 +112,15 @@ class NgView implements DetachAware, RouteProvider {
 
     Injector viewInjector = _appInjector;
 
-    if (modules != null) {
-      viewInjector = createChildInjectorWithReload(_appInjector, modules);
-    }
+    List<Module> modules = viewDef.modules;
+    if (modules != null) viewInjector = createChildInjectorWithReload(_appInjector, modules);
 
     var newDirectives = viewInjector.getByKey(DIRECTIVE_MAP_KEY);
+
     var viewFuture = viewDef.templateHtml != null ?
         new Future.value(_viewCache.fromHtml(viewDef.templateHtml, newDirectives)) :
         _viewCache.fromUrl(viewDef.template, newDirectives, Uri.base);
+
     viewFuture.then((ViewFactory viewFactory) {
       _cleanUp();
       _childScope = _scope.createChild(new PrototypeMap(_scope.context));
@@ -135,19 +138,22 @@ class NgView implements DetachAware, RouteProvider {
     _childScope = null;
   }
 
+  /// implements `RouteProvider.route`
   Route get route => _viewRoute;
 
+  /// implements `RouteProvider.routeName`
   String get routeName => _viewRoute.name;
 
+  /// implements `RouteProvider.parameters`
   Map<String, String> get parameters {
     var res = new HashMap<String, String>();
     var p = _viewRoute;
-    while (p != null) {
+    for (Route p = _viewRoute; p != null; p = p.parent) {
       res.addAll(p.parameters);
-      p = p.parent;
     }
     return res;
   }
+
   /**
    * Creates a child injector that allows loading new directives, formatters and
    * services from the provided modules.

lib/routing/routing.dart

@@ -13,8 +13,7 @@ class RouteViewFactory {
 
   void _enterHandler(RouteEnterEvent event, String templateUrl,
                      {List<Module> modules, String templateHtml}) {
-    locationService._route(event.route, templateUrl, fromEvent: true,
-        modules: modules, templateHtml: templateHtml);
+    locationService._route(event.route, templateUrl, templateHtml, modules);
   }
 
   void configure(Map<String, NgRouteCfg> config) {
@@ -32,8 +31,7 @@ class RouteViewFactory {
           dontLeaveOnParamChanges: cfg.dontLeaveOnParamChanges,
           enter: (RouteEnterEvent e) {
             if (cfg.view != null || cfg.viewHtml != null) {
-              _enterHandler(e, cfg.view,
-                  modules: newModules, templateHtml: cfg.viewHtml);
+              _enterHandler(e, cfg.view, modules: newModules, templateHtml: cfg.viewHtml);
             }
             if (cfg.enter != null) {
               cfg.enter(e);
@@ -56,11 +54,7 @@ class RouteViewFactory {
               cfg.preEnter(e);
             }
           },
-          preLeave: (RoutePreLeaveEvent e) {
-            if (cfg.preLeave != null) {
-              cfg.preLeave(e);
-            }
-          },
+          preLeave: cfg.preLeave,
           leave: cfg.leave,
           mount: (Route mountRoute) {
             if (cfg.mount != null) {
@@ -71,14 +65,31 @@ class RouteViewFactory {
   }
 }
 
-NgRouteCfg ngRoute({String path, String view, String viewHtml,
-    Map<String, NgRouteCfg> mount, modules(), bool defaultRoute: false,
-    RoutePreEnterEventHandler preEnter, RouteEnterEventHandler enter,
-    RoutePreLeaveEventHandler preLeave, RouteLeaveEventHandler leave,
-    dontLeaveOnParamChanges: false}) =>
-        new NgRouteCfg(path: path, view: view, viewHtml: viewHtml, mount: mount,
-            modules: modules, defaultRoute: defaultRoute, preEnter: preEnter, preLeave: preLeave,
-            enter: enter, leave: leave, dontLeaveOnParamChanges: dontLeaveOnParamChanges);
+/**
+ * Helper function to create a route configuration (`NgRouteCfg`):
+ * - `path`: url section (`/path`),
+ * - `view`: external template,
+ * - `viewHtml`: inline template,
+ * - `mount`: child routes,
+ * - `defaultRoute`: set to `true` for the default route,
+ * - `*EventHandler`: event handlers, see route.dart for details,
+ * - `dontLeaveOnParamChanges`: do not leave the route when only parameters change
+ */
+NgRouteCfg ngRoute({String path,
+                    String view,
+                    String viewHtml,
+                    Map<String, NgRouteCfg> mount,
+                    modules(),
+                    bool defaultRoute: false,
+                    RoutePreEnterEventHandler preEnter,
+                    RouteEnterEventHandler enter,
+                    RoutePreLeaveEventHandler preLeave,
+                    RouteLeaveEventHandler leave,
+                    dontLeaveOnParamChanges: false}) {
+  return new NgRouteCfg(path: path, view: view, viewHtml: viewHtml, mount: mount, modules: modules,
+      defaultRoute: defaultRoute, preEnter: preEnter, preLeave: preLeave, enter: enter,
+      leave: leave, dontLeaveOnParamChanges: dontLeaveOnParamChanges);
+}
 
 class NgRouteCfg {
   final String path;
@@ -125,12 +136,11 @@ typedef void RouteInitializerFn(Router router, RouteViewFactory viewFactory);
 @Injectable()
 class NgRoutingHelper {
   final Router router;
-  final Application _ngApp;
   final _portals = <NgView>[];
   final _templates = <String, _View>{};
 
   NgRoutingHelper(RouteInitializer initializer, Injector injector, this.router,
-                  this._ngApp) {
+                  Application ngApp) {
     // TODO: move this to constructor parameters when di issue is fixed:
     // https://github.com/angular/di.dart/issues/40
     RouteInitializerFn initializerFn = injector.getByKey(ROUTE_INITIALIZER_FN_KEY);
@@ -144,6 +154,8 @@ class NgRoutingHelper {
     } else {
       initializer.init(router, new RouteViewFactory(this));
     }
+
+    // Check if we need to update `ng-view`s when a new route is activated
     router.onRouteStart.listen((RouteStartEvent routeEvent) {
       routeEvent.completed.then((success) {
         if (success) {
@@ -152,34 +164,37 @@ class NgRoutingHelper {
       });
     });
 
-    router.listen(appRoot: _ngApp.element);
+    // Make the router listen to URL change events and click events
+    router.listen(appRoot: ngApp.element);
   }
 
   void _reloadViews({Route startingFrom}) {
-    var alreadyActiveViews = [];
-    var activePath = router.activePath;
+    var activeViews = <NgView>[];
+    Iterable<Route> activePath = router.activePath;
     if (startingFrom != null) {
+      // only consider child routes of the `startingFrom` route
       activePath = activePath.skip(_routeDepth(startingFrom));
     }
+
     for (Route route in activePath) {
-      var viewDef = _templates[_routePath(route)];
+      var path = _routePath(route);
+      var viewDef = _templates[path];
       if (viewDef == null) continue;
-      var templateUrl = viewDef.template;
-
-      NgView view = _portals.lastWhere((NgView v) {
-        return _routePath(route) != _routePath(v._route) &&
-            _routePath(route).startsWith(_routePath(v._route));
-      }, orElse: () => null);
-      if (view != null && !alreadyActiveViews.contains(view)) {
-        view._show(viewDef, route, viewDef.modules);
-        alreadyActiveViews.add(view);
+
+      NgView view = _portals.lastWhere(
+          (NgView v) => path != _routePath(v._parentRoute) &&
+                        path.startsWith(_routePath(v._parentRoute)),
+          orElse: () => null);
+
+      if (view != null && !activeViews.contains(view)) {
+        view._show(viewDef, route);
+        activeViews.add(view);
         break;
       }
     }
   }
 
-  void _route(Route route, String template, {bool fromEvent, List<Module> modules,
-      String templateHtml}) {
+  void _route(Route route, String template, String templateHtml, List<Module> modules) {
     _templates[_routePath(route)] = new _View(template, templateHtml, modules);
   }
 
@@ -200,22 +215,20 @@ class _View {
   _View(this.template, this.templateHtml, this.modules);
 }
 
+/// Returns the route full path (ie `grand-parent.parent.current`)
 String _routePath(Route route) {
-  final path = [];
-  var p = route;
-  while (p.parent != null) {
+  final path = <String>[];
+  for (Route p = route; p.parent != null; p = p.parent) {
     path.insert(0, p.name);
-    p = p.parent;
   }
   return path.join('.');
 }
 
+/// Returns the route depth (ie 3 for `grand-parent.parent.current`)
 int _routeDepth(Route route) {
   var depth = 0;
-  var p = route;
-  while (p.parent != null) {
+  for (Route p = route; p.parent != null; p = p.parent) {
     depth++;
-    p = p.parent;
   }
   return depth;
 }