refactor(view): Put views on a ITransNode for use in Path manipulations

refactor(viewPath): Moved $view.register/reset from transitionSuccess to onStart/onEnter
fix($view): Check if ui-view is still in the active list before configuring it

Author: christopherthielen

src/path/interface.ts

@@ -3,6 +3,8 @@ import Path from "./../path/path";
 
 import {IState} from "../state/interface";
 
+import {ViewConfig} from "../view/view";
+
 import {IRawParams} from "../params/interface";
 import ParamValues from "../params/paramValues";
 
@@ -19,7 +21,7 @@ export interface IPath extends Path<INode> {}
 
 
 /** Contains INode base data plus raw params values for the node */
-export interface IParamsNode extends INode { 
+export interface IParamsNode extends INode {
   ownParams: IRawParams;
 }
 /** A Path of IParamsNode(s) */
@@ -33,10 +35,10 @@ export interface IResolveNode extends IParamsNode {
 /** A Path of IResolveNode(s) */
 export interface IResolvePath extends Path<IResolveNode> {}
 
-
 /** Contains IResolveNode data, plus a ResolveContext and ParamsValues (bound to a full path) for the node,  */
 export interface ITransNode extends IResolveNode {
   resolveContext: ResolveContext;
+  views: ViewConfig[];
   paramValues: ParamValues;
 }
 /**

src/path/pathFactory.ts

@@ -1,4 +1,6 @@
-import {map, extend, prop, pick, omit, not, objectKeys, curry} from "../common/common";
+import {map, extend, pairs, prop, pick, omit, not, objectKeys, curry} from "../common/common";
+
+import {runtime} from "../common/angular1";
 
 import {IRawParams} from "../params/interface";
 import ParamValues from "../params/paramValues";
@@ -13,6 +15,9 @@ import Path from "../path/path";
 
 import Resolvable from "../resolve/resolvable";
 import ResolveContext from "../resolve/resolveContext";
+import PathContext from "../resolve/pathContext";
+
+import {ViewConfig} from "../view/view";
 
 /**
  * This class contains functions which convert TargetStates, Nodes and Paths from one type to another.
@@ -99,16 +104,30 @@ export default class PathFactory {
   /**
    * Given an IResolvePath, upgrades the path to an ITransPath.  Each node is assigned a ResolveContext
    * and ParamValues object which is bound to the whole path, but closes over the subpath from root to the node.
+   * The views are also added to the node.
    */
   static bindTransNodesToPath(resolvePath: IResolvePath): ITransPath {
     let resolveContext = new ResolveContext(resolvePath);
     let paramValues = new ParamValues(resolvePath);
     let transPath = <ITransPath> resolvePath;
 
+    // TODO: this doesn't belong here.
+    // TODO: pass options to PathContext
+    // TODO: rename PathContext
+    function makeViews(node: ITransNode) {
+      let context = node.state, params = node.paramValues;
+      let locals = new PathContext(node.resolveContext, node.state, runtime.$injector, {});
+      const makeViewConfig = ([rawViewName, viewDeclarationObj]) =>
+          new ViewConfig({rawViewName, viewDeclarationObj, context, locals, params});
+      return pairs(node.state.views || {}).map(makeViewConfig);
+    }
+
     // Attach bound resolveContext and paramValues to each node
-    transPath.nodes().forEach(node => {
+    // Attach views to each node
+    transPath.nodes().forEach((node: ITransNode) => {
           node.resolveContext = resolveContext.isolateRootTo(node.state);
           node.paramValues = paramValues.$isolateRootTo(node.state.name);
+          node.views = makeViews(node);
         }
     );
 

src/state/state.ts

@@ -22,6 +22,7 @@ import PathFactory from "../path/pathFactory";
 
 import {IRawParams, IParamsOrArray} from "../params/interface";
 
+import {ViewConfig} from "../view/view";
 
 /**
  * @ngdoc object
@@ -692,10 +693,26 @@ function $StateProvider(   $urlRouterProvider,   $urlMatcherFactoryProvider) {
         return $q.reject(transition.error());
 
 
+      // TODO: Move the Transition instance hook registration to its own function
+      let enteringViews = transition.views("entering");
+      let exitingViews = transition.views("exiting");
+      const loadView = (viewConfig: ViewConfig) => $view.load(viewConfig);
+      const activateView = (viewConfig: ViewConfig) => $view.registerStateViewConfig(viewConfig);
+      const deactivateView = (viewConfig: ViewConfig) => $view.reset(viewConfig);
+
+      const loadAllEnteringViews = () => $q.all(enteringViews.map(loadView)).then(() => undefined);
+      const deactivateAllExitedViews = () => exitingViews.forEach(deactivateView);
+      const activateEnteringViews = ($state$: IState) => transition.views("entering", $state$).forEach(activateView);
+
       // Add hooks
       // TODO: Move this to its own fn
       let hookBuilder = transition.hookBuilder();
+
+      transition.onStart({}, loadAllEnteringViews, { priority: 100 });
+      transition.onStart({}, deactivateAllExitedViews, { priority: 50 });
       transition.onStart({}, hookBuilder.getEagerResolvePathFn(), { priority: 100 });
+
+      transition.onEnter({}, activateEnteringViews, { priority: 101 });
       transition.onEnter({}, hookBuilder.getLazyResolveStateFn(), { priority: 100 });
 
       let onEnterRegistration = (state) => transition.onEnter({to: state.name}, state.onEnter, { priority: -100 });

src/state/stateHandler.ts

@@ -30,9 +30,6 @@ export default class StateHandler {
 
   transitionSuccess(treeChanges: ITreeChanges, transition: Transition) {
     let {$view, $state, activeTransQ, changeHistory} = this;
-    // TODO: sync on entering/exiting state, not transition success?
-    transition.views("exiting", "from").forEach($view.reset.bind($view));
-    transition.views("entering", "to").forEach($view.registerStateViewConfig.bind($view));
     $view.sync();
 
     // Update globals in $state

src/transition/interface.ts

@@ -34,6 +34,7 @@ export interface ITransitionHookOptions {
 }
 
 export interface ITreeChanges {
+  [key: string]: ITransPath;
   from:     ITransPath;
   to:       ITransPath;
   retained: ITransPath;

src/transition/transition.ts

@@ -9,19 +9,17 @@ import {HookRegistry, matchState} from "./hookRegistry";
 import HookBuilder from "./hookBuilder";
 import {RejectFactory} from "./rejectFactory";
 
-import {IResolvePath, ITransPath} from "../path/interface";
+import {ITransPath} from "../path/interface";
 import PathFactory from "../path/pathFactory";
 
-import ResolveContext from "../resolve/resolveContext";
-import PathContext from "../resolve/pathContext";
-
 import TargetState from "../state/targetState";
-import {IState, IStateDeclaration, IStateViewConfig} from "../state/interface";
+import {IState, IStateDeclaration} from "../state/interface";
 
 import ParamValues from "../params/paramValues";
 
-import {extend, flatten, forEach, identity, isEq, isObject, map, not, prop, toJson, unnest, val,
-    pairs, abstractKey} from "../common/common";
+import {ViewConfig} from "../view/view";
+
+import {extend, flatten, forEach, identity, isEq, isObject, not, prop, toJson, val, abstractKey} from "../common/common";
 
 let transitionCount = 0, REJECT = new RejectFactory();
 const stateSelf: (_state: IState) => IStateDeclaration = prop("self");
@@ -222,29 +220,13 @@ export class Transition implements IHookRegistry {
     return this._treeChanges.retained.states().map(stateSelf);
   }
 
-  // TODO
-  context(pathname: string, state: IState): PathContext {
-    let path = this._treeChanges[pathname].pathFromRootTo(state);
-    return new PathContext(new ResolveContext(path), state, runtime.$injector, this._options);
-  }
-
   /**
-   * Returns a list of StateViewConfig objects;
-   * Returns one StateViewConfig for each view in each state in a named path of the transition's tree changes
+   * Returns a list of ViewConfig objects for a given path. Returns one ViewConfig for each view in
+   * each state in a named path of the transition's tree changes. Optionally limited to a given state in that path.
    */
-  views(pathname: string = "entering", contextPathname: string = "to") {
-    let path: IResolvePath = this._treeChanges[pathname];
-    let states: IState[] = path.states();
-    let params: ParamValues = this.params();
-
-    return unnest(map(states, (state: IState) => {
-      let context = state;
-      let locals: PathContext = this.context(contextPathname, state);
-      const makeViewConfig = ([rawViewName, viewDeclarationObj]) => { return <IStateViewConfig> {
-        rawViewName, viewDeclarationObj, context, locals, params};
-      };
-      return pairs(state.views).map(makeViewConfig);
-    }));
+  views(pathname: string = "entering", state?: IState): ViewConfig[] {
+    let path = this._treeChanges[pathname];
+    return state ? path.nodeForState(state).views : flatten(path.nodes().map(prop("views")));
   }
 
   /**

src/transition/transitionHook.ts

@@ -1,4 +1,4 @@
-import {defaults, noop, filter, not, isFunction, objectKeys, map, pattern, isEq, val, pipe, eq, is, isPromise, isObject, parse} from "../common/common";
+import {defaults, noop, filter, not, isFunction, isDefined, objectKeys, map, pattern, isEq, val, pipe, eq, is, isPromise, isObject, parse} from "../common/common";
 import trace from "../common/trace";
 import {RejectFactory} from "./rejectFactory";
 import {Transition} from "./transition";
@@ -76,16 +76,20 @@ export default class TransitionHook {
   mapNewResolves(resolves: IResolveDeclarations) {
     let invalid = filter(resolves, not(isFunction)), keys = objectKeys(invalid);
     if (keys.length)
-      throw new Error("Invalid resolve key/value: ${keys[0]}/${invalid[keys[0]]}");
+      throw new Error(`Invalid resolve key/value: ${keys[0]}/${invalid[keys[0]]}`);
 
     // If result is an object, it should be a map of strings to functions.
     const makeResolvable = (fn, name) => new Resolvable(name, fn, this.state);
     return map(resolves, makeResolvable);
   }
 
   handleHookResult(hookResult) {
+    if (!isDefined(hookResult)) return undefined;
+    if (this.options.trace) trace.traceHookResult(hookResult, undefined, this.options);
+
     let transitionResult = this.mapHookResult(hookResult);
-    if (this.options.trace) trace.traceHookResult(hookResult, transitionResult, this.options);
+    if (transitionResult && this.options.trace) trace.traceHookResult(hookResult, transitionResult, this.options);
+
     return transitionResult;
   }
 

src/view/templateFactory.ts

@@ -106,7 +106,7 @@ function $TemplateFactory(  $http,   $templateCache) {
    * for that string.
    */
   this.fromProvider = function (provider, params, locals) {
-    return locals(provider, { params: params });
+    return locals(provider, { $stateParams: params });
   };
 }
 

src/view/view.ts

@@ -196,7 +196,6 @@ function $View(   $rootScope,   $templateFactory,   $q,   $timeout) {
     return $q.all(viewConfig.promises).then((results) => {
       debug(`$view.ViewConfig: Loaded ${viewConfigString(viewConfig)}`);
       extend(viewConfig, results);
-      this.sync();
     });
   };
 
@@ -206,19 +205,14 @@ function $View(   $rootScope,   $templateFactory,   $q,   $timeout) {
    * @param {String} name The fully-qualified name of the view to reset.
    * @return {Boolean} Returns `true` if the view exists, otherwise `false`.
    */
-  this.reset = function reset (stateViewConfig) {
-    let viewConfig = new ViewConfig(stateViewConfig);
+  this.reset = function reset (viewConfig) {
     debug(`$view.ViewConfig: <- Removing ${viewConfigString(viewConfig)}`);
-    // TODO: Check if this code is doing the right thing
     viewConfigs.filter(match(viewConfig, "uiViewName", "context")).forEach(removeFrom(viewConfigs));
-    //uiViews.filter(match(viewConfig, "name", "parentContext")).forEach(uiView => uiView.configUpdated(null));
   };
 
-  this.registerStateViewConfig = function(stateViewConfig: IStateViewConfig) {
-    let viewConfig = new ViewConfig(stateViewConfig);
+  this.registerStateViewConfig = function(viewConfig: ViewConfig) {
     debug(`$view.ViewConfig: -> Registering ${viewConfigString(viewConfig)}`);
     viewConfigs.push(viewConfig);
-    this.load(viewConfig);
   };
 
   this.sync = () => {
@@ -318,7 +312,13 @@ function $View(   $rootScope,   $templateFactory,   $q,   $timeout) {
       return [uiView, matchingConfigs[0]];
     };
 
-    const configureUiView = ([uiView, viewConfig]) => uiView.configUpdated(viewConfig);
+    const configureUiView = ([uiView, viewConfig]) => {
+      // If a parent ui-view is reconfigured, it could destroy child ui-views.
+      // Before configuring a child ui-view, make sure it's still in the active uiViews array.
+      if (uiViews.indexOf(uiView) !== -1)
+        uiView.configUpdated(viewConfig);
+    };
+
     uiViews.sort(depthCompare(uiViewDepth, 1)).map(matchingConfigPair).forEach(configureUiView);
   };
 

src/view/viewDirective.ts

@@ -143,11 +143,7 @@ function $ViewDirective(   $view,   $animate,   $uiViewScroll,   $interpolate,
   }
 
   function configsEqual(config1, config2) {
-    return (config1 === config2) || (config1 && config2 && (
-      config1.controller    === config2.controller &&
-      config1.template      === config2.template &&
-      config1.controllerAs  === config2.controllerAs
-      ));
+    return config1 === config2;
   }
 
   let directive = {
@@ -163,7 +159,7 @@ function $ViewDirective(   $view,   $animate,   $uiViewScroll,   $interpolate,
             onloadExp     = attrs.onload || '',
             autoScrollExp = attrs.autoscroll,
             renderer      = getRenderer(attrs, scope),
-            viewConfig    = {},
+            viewConfig    = undefined,
             inherited     = $element.inheritedData('$uiView') || { context: $view.rootContext() },
             name          = $interpolate(attrs.uiView || attrs.name || '')(scope) || '$default';
 
@@ -179,10 +175,11 @@ function $ViewDirective(   $view,   $animate,   $uiViewScroll,   $interpolate,
 
         debug(`uiView tag: Linking '${viewData.fqn}#${viewData.id}'`);
 
-        function configUpdatedCallback(config: ViewConfig = <any> {}) {
+        function configUpdatedCallback(config?: ViewConfig) {
           if (configsEqual(viewConfig, config)) return;
-          debug(`uiView tag: Updating '${viewData.fqn}#${viewData.id}': (${uiViewString(viewData)}) with ViewConfig from context='${config.context}'`);
-          viewConfig = extend({}, config);
+          let context = config && config.context;
+          debug(`uiView tag: Updating '${viewData.fqn}#${viewData.id}': (${uiViewString(viewData)}) with ViewConfig from context='${context}'`);
+          viewConfig = config;
           updateView(config);
         }
 

test/stateDirectivesSpec.js

@@ -101,7 +101,7 @@ describe('uiStateRef', function() {
     }
   };
 
-  xdescribe('links', function() {
+  describe('links', function() {
     beforeEach(inject(buildDOM));
 
     it('should generate the correct href', function() {

test/viewDirectiveSpec.js

@@ -463,31 +463,31 @@ describe('uiView', function () {
       // Enter Animation
       animation = $animate.queue.shift();
       expect(animation.event).toBe('enter');
-      expect(animation.element.text()).toBe(content);
+      expect(animation.element.text() + "-1").toBe(content + "-1");
 
       $state.transitionTo(aState);
       $q.flush();
 
       // Enter Animation
       animation = $animate.queue.shift();
       expect(animation.event).toBe('enter');
-      expect(animation.element.text()).toBe(aState.template);
+      expect(animation.element.text() + "-2").toBe(aState.template + "-2");
       // Leave Animation
       animation = $animate.queue.shift();
       expect(animation.event).toBe('leave');
-      expect(animation.element.text()).toBe(content);
+      expect(animation.element.text() + "-3").toBe(content + "-3");
 
       $state.transitionTo(bState);
       $q.flush();
 
       // Enter Animation
       animation = $animate.queue.shift();
       expect(animation.event).toBe('enter');
-      expect(animation.element.text()).toBe(bState.template);
+      expect(animation.element.text() + "-4").toBe(bState.template + "-4");
       // Leave Animation
       animation = $animate.queue.shift();
       expect(animation.event).toBe('leave');
-      expect(animation.element.text()).toBe(aState.template);
+      expect(animation.element.text() + "-5").toBe(aState.template + "-5");
 
       // No more animations
       expect($animate.queue.length).toBe(0);