refactor(Resolvable): make Node.resolvables an array, not an object

This is the first step in allowing arbitrary (non-string) keys for injecting Resolvables.

refactor(matching): When calculating Node.matching(), require the node parameter values to match.

refactor(redirect): During redirect, only copy resolveables for matching entering states (between the original transition and the redirect transition).

refactor(stringify): Defer creation of stringify `pattern`
This mitigates the possibility of modules loading in a different order and imported classes such as Rejection, Transition, Resolvable not being immediately available until

Author: christopherthielen

src/common/strings.ts

@@ -67,17 +67,22 @@ export function fnToString(fn: IInjectable) {
 
 const isTransitionRejectionPromise = Rejection.isTransitionRejectionPromise;
 
-let stringifyPattern = pattern([
-  [not(isDefined),                  val("undefined")],
-  [isNull,                          val("null")],
-  [isPromise,                       promiseToString],
-  [isTransitionRejectionPromise,    (x: any) => x._transitionRejection.toString()],
-  [is(Rejection),                   invoke("toString")],
-  [is(Transition),                  invoke("toString")],
-  [is(Resolvable),                  invoke("toString")],
-  [isInjectable,                    functionToString],
-  [val(true),                       identity]
-]);
+let stringifyPatternFn = null;
+let stringifyPattern = function(val) {
+  stringifyPatternFn = stringifyPatternFn || pattern([
+    [not(isDefined),                  val("undefined")],
+    [isNull,                          val("null")],
+    [isPromise,                       val("[Promise]")],
+    [isTransitionRejectionPromise,    (x: any) => x._transitionRejection.toString()],
+    [is(Rejection),                   invoke("toString")],
+    [is(Transition),                  invoke("toString")],
+    [is(Resolvable),                  invoke("toString")],
+    [isInjectable,                    functionToString],
+    [val(true),                       identity]
+  ]);
+
+  return stringifyPatternFn(val);
+};
 
 export function stringify(o: Object) {
   var seen: any[] = [];

src/path/node.ts

@@ -1,18 +1,17 @@
 /** @module path */ /** for typedoc */
-import {extend, applyPairs, map, find, allTrueR, values, mapObj} from "../common/common";
+import {extend, applyPairs, find, allTrueR} from "../common/common";
 import {prop, propEq} from "../common/hof";
 import {State} from "../state/module";
 import {RawParams} from "../params/interface";
 import {Param} from "../params/module";
 import {Resolvable, ResolveContext} from "../resolve/module";
 import {ViewConfig} from "../view/interface";
-import {Resolvables} from "../resolve/interface";
 
 export class Node {
   public state: State;
   public paramSchema: Param[];
   public paramValues: { [key: string]: any };
-  public resolves: Resolvables;
+  public resolvables: Resolvable[];
   public views: ViewConfig[];
   public resolveContext: ResolveContext;
 
@@ -24,14 +23,14 @@ export class Node {
       this.state = node.state;
       this.paramSchema = node.paramSchema.slice();
       this.paramValues = extend({}, node.paramValues);
-      this.resolves = extend({}, node.resolves);
+      this.resolvables = node.resolvables.slice();
       this.views = node.views && node.views.slice();
       this.resolveContext = node.resolveContext;
     } else {
       this.state = state;
       this.paramSchema = state.parameters({ inherit: false });
       this.paramValues = {};
-      this.resolves = mapObj(state.resolve, (fn: Function, name: string) => new Resolvable(name, fn));
+      this.resolvables = Object.keys(state.resolve || {}).map(key => new Resolvable(key, state.resolve[key]));
     }
   }
 
@@ -55,15 +54,22 @@ export class Node {
   }
 
   /**
-   * Returns a new path which is a subpath of the first path. The new path starts from root and contains any nodes
-   * that match the nodes in the second path. Nodes are compared using their state property.
-   * @param first {Node[]}
-   * @param second {Node[]}
-   * @returns {Node[]}
+   * Returns a new path which is a subpath of the first path which matched the second path.
+   *
+   * The new path starts from root and contains any nodes that match the nodes in the second path.
+   * Nodes are compared using their state property and parameter values.
    */
-  static matching(first: Node[], second: Node[]): Node[] {
-    let matchedCount = first.reduce((prev, node, i) =>
-      prev === i && i < second.length && node.state === second[i].state ? i + 1 : prev, 0);
-    return first.slice(0, matchedCount);
+  static matching(pathA: Node[], pathB: Node[]): Node[] {
+    let matching = [];
+    
+    for (let i = 0; i < pathA.length && i < pathB.length; i++) {
+      let a = pathA[i], b = pathB[i];
+      
+      if (a.state !== b.state) break;
+      if (!Param.equals(a.paramSchema, a.paramValues, b.paramValues)) break;
+      matching.push(a);
+    }
+    
+    return matching
   }
 }

src/resolve/resolvable.ts

@@ -22,21 +22,19 @@ import {ResolveContext} from "./resolveContext";
  * parameter to those fns.
  */
 export class Resolvable {
-  constructor(name: string, resolveFn: Function, preResolvedData?: any) {
-    extend(this, {
-     name, 
-     resolveFn, 
-     deps: services.$injector.annotate(resolveFn, services.$injector.strictDi), 
-     data: preResolvedData 
-   });
-  }
-
   name: string;
   resolveFn: Function;
   deps: string[];
 
   promise: Promise<any> = undefined;
   data: any;
+  
+  constructor(name: string, resolveFn: Function, preResolvedData?: any) {
+    this.name = name;
+    this.resolveFn = resolveFn;
+    this.deps = services.$injector.annotate(resolveFn, services.$injector.strictDi);
+    this.data = preResolvedData;
+  }
 
   // synchronous part:
   // - sets up the Resolvable's promise
@@ -92,13 +90,13 @@ export class Resolvable {
   }
 
   /**
-   * Validates the result map as a "resolve:" style object, then transforms the resolves into Resolvables
+   * Validates the result map as a "resolve:" style object, then transforms the resolves into Resolvable[]
    */
-  static makeResolvables(resolves: { [key: string]: Function; }): Resolvables {
+  static makeResolvables(resolves: { [key: string]: Function; }): Resolvable[] {
     // If a hook result is an object, it should be a map of strings to functions.
     let invalid = filter(resolves, not(isInjectable)), keys = Object.keys(invalid);
     if (keys.length)
       throw new Error(`Invalid resolve key/value: ${keys[0]}/${invalid[keys[0]]}`);
-    return map(resolves, (fn, name: string) => new Resolvable(name, fn));
+    return Object.keys(resolves).map(key => new Resolvable(key, resolves[key]));
   }
 }

src/resolve/resolveContext.ts

@@ -20,8 +20,8 @@ interface Promises { [key: string]: Promise<any>; }
 
 export class ResolveContext {
 
-  private _nodeFor: Function;
-  private _pathTo: Function;
+  private _nodeFor: (s: State) => Node;
+  private _pathTo: (s: State) => Node[];
 
   constructor(private _path: Node[]) {
     extend(this, {
@@ -62,9 +62,13 @@ export class ResolveContext {
     const path = (state ?  this._pathTo(state) : this._path);
     const last = tail(path);
 
-    return path.reduce((memo, node) => {
+    return path.reduce((memo, node: Node) => {
       let omitProps = (node === last) ? options.omitOwnLocals : [];
-      let filteredResolvables = omit(node.resolves, omitProps);
+
+      let filteredResolvables = node.resolvables
+          .filter(r => omitProps.indexOf(r.name) === -1)
+          .reduce((acc, r) => { acc[r.name] = r; return acc; }, {});
+
       return extend(memo, filteredResolvables);
     }, <Resolvables> {});
   }
@@ -79,13 +83,16 @@ export class ResolveContext {
     return new ResolveContext(this._pathTo(state));
   }
 
-  addResolvables(resolvables: Resolvables, state: State) {
-    extend(this._nodeFor(state).resolves, resolvables);
+  addResolvables(newResolvables: Resolvable[], state: State) {
+    var node = this._nodeFor(state);
+    var keys = newResolvables.map(r => r.name);
+    node.resolvables = node.resolvables.filter(r => keys.indexOf(r.name) === -1).concat(newResolvables);
   }
 
   /** Gets the resolvables declared on a particular state */
   getOwnResolvables(state: State): Resolvables {
-    return extend({}, this._nodeFor(state).resolves);
+    return this._nodeFor(state).resolvables
+        .reduce((acc, r) => { acc[r.name] = r; return acc; }, <Resolvables>{});
   }
 
   // Returns a promise for an array of resolved path Element promises

src/state/hooks/resolveHooks.ts

@@ -43,7 +43,7 @@ export class ResolveHooks {
       // Resolve all the LAZY resolves, then resolve the `$resolve$` object, then add `$resolve$` to the context
       return context.resolvePathElement(node.state, options)
           .then(() => $resolve$.resolveResolvable(context))
-          .then(() => context.addResolvables({$resolve$}, node.state));
+          .then(() => context.addResolvables([$resolve$], node.state));
     }
 
     // Resolve eager resolvables before when the transition starts

src/transition/transition.ts

@@ -2,11 +2,11 @@
 import {trace} from "../common/trace";
 import {services} from "../common/coreservices";
 import {
-    map, find, extend, filter, mergeR,  tail,
+    map, find, extend, mergeR,  tail,
     omit, toJson, abstractKey, arrayTuples, unnestR, identity, anyTrueR
 } from "../common/common";
 import { isObject } from "../common/predicates";
-import { not, prop, propEq, val } from "../common/hof";
+import { prop, propEq, val, not } from "../common/hof";
 
 import {StateDeclaration, StateOrName} from "../state/interface";
 import {TransitionOptions, TransitionHookOptions, TreeChanges, IHookRegistry, IHookRegistration, IHookGetter} from "./interface";
@@ -20,7 +20,6 @@ import {Resolvable} from "../resolve/module";
 import {TransitionService} from "./transitionService";
 import {ViewConfig} from "../view/interface";
 import {Rejection} from "./rejectFactory";
-import {Resolvables} from "../resolve/interface";
 
 
 let transitionCount = 0;
@@ -143,10 +142,10 @@ export class Transition implements IHookRegistry {
 
     PathFactory.bindResolveContexts(this._treeChanges.to);
 
-    let rootResolvables: Resolvables = {
-      "$transition$": new Resolvable('$transition$', () => this, this),
-      "$stateParams": new Resolvable('$stateParams', () => this.params(), this.params())
-    };
+    let rootResolvables: Resolvable[] = [
+      new Resolvable('$transition$', () => this, this),
+      new Resolvable('$stateParams', () => this.params(), this.params())
+    ];
     let rootNode: Node = this._treeChanges.to[0];
     rootNode.resolveContext.addResolvables(rootResolvables, rootNode.state)
   }
@@ -292,34 +291,45 @@ export class Transition implements IHookRegistry {
   treeChanges = () => this._treeChanges;
 
   /**
-   * @ngdoc function
-   * @name ui.router.state.type:Transition#redirect
-   * @methodOf ui.router.state.type:Transition
+   * Creates a new transition that is a redirection of the current one.
    *
-   * @description
-   * Creates a new transition that is a redirection of the current one. This transition can
-   * be returned from a `$transitionsProvider` hook, `$state` event, or other method, to
+   * This transition can be returned from a [[TransitionService]] hook to
    * redirect a transition to a new state and/or set of parameters.
    *
-   * @returns {Transition} Returns a new `Transition` instance.
+   * @returns Returns a new [[Transition]] instance.
    */
   redirect(targetState: TargetState): Transition {
     let newOptions = extend({}, this.options(), targetState.options(), { previous: this });
     targetState = new TargetState(targetState.identifier(), targetState.$state(), targetState.params(), newOptions);
 
-    let redirectTo = new Transition(this._treeChanges.from, targetState, this._transitionService);
-    let reloadState = targetState.options().reloadState;
+    let newTransition = new Transition(this._treeChanges.from, targetState, this._transitionService);
+    let originalEnteringNodes = this.treeChanges().entering;
+    let redirectEnteringNodes = newTransition.treeChanges().entering;
+
+    // --- Re-use resolve data from original transition ---
+    // When redirecting from a parent state to a child state where the parent parameter values haven't changed
+    // (because of the redirect), the resolves fetched by the original transition are still valid in the
+    // redirected transition.
+    //
+    // This allows you to define a redirect on a parent state which depends on an async resolve value.
+    // You can wait for the resolve, then redirect to a child state based on the result.
+    // The redirected transition does not have to re-fetch the resolve.
+    // ---------------------------------------------------------
+
+    const nodeIsReloading = (reloadState: State) => (node: Node) => {
+      return reloadState && reloadState.includes[node.state.name];
+    };
+
+    // Find any "entering" nodes in the redirect path that match the original path and aren't being reloaded
+    let matchingEnteringNodes: Node[] = Node.matching(redirectEnteringNodes, originalEnteringNodes)
+        .filter(not(nodeIsReloading(targetState.options().reloadState)));
 
-    // If the current transition has already resolved any resolvables which are also in the redirected "to path", then
-    // add those resolvables to the redirected transition.  Allows you to define a resolve at a parent level, wait for
-    // the resolve, then redirect to a child state based on the result, and not have to re-fetch the resolve.
-    let redirectedPath = this.treeChanges().to;
-    let copyResolvesFor: Node[] = Node.matching(redirectTo.treeChanges().to, redirectedPath)
-        .filter(node => !reloadState || !reloadState.includes[node.state.name]);
-    const includeResolve = (resolve, key) => ['$stateParams', '$transition$'].indexOf(key) === -1;
-    copyResolvesFor.forEach((node, idx) => extend(node.resolves, filter(redirectedPath[idx].resolves, includeResolve)));
+    // Use the existing (possibly pre-resolved) resolvables for the matching entering nodes.
+    matchingEnteringNodes.forEach((node, idx) => {
+      node.resolvables = originalEnteringNodes[idx].resolvables;
+    });
 
-    return redirectTo;
+    return newTransition;
   }
 
   /** @hidden If a transition doesn't exit/enter any states, returns any [[Param]] whose value changed */

test/ng1/stateSpec.js

@@ -1474,19 +1474,6 @@ describe('state', function () {
     }));
   });
 
-  describe('default properties', function() {
-    it('should always have a name', inject(function ($state, $q) {
-      $state.transitionTo(A);
-      $q.flush();
-      expect($state.$current.name).toBe('A');
-      expect($state.$current.toString()).toBe('A');
-    }));
-
-    it('should always have a resolve object', inject(function ($state) {
-      expect($state.$current.resolve).toEqual({});
-    }));
-  });
-
   describe('"data" property inheritance/override', function () {
     it('should stay immutable for if state doesn\'t have parent', inject(function ($state) {
       initStateTo(H);
@@ -1554,7 +1541,8 @@ describe('state', function () {
     }));
 
     it('should always have a resolve object', inject(function ($state) {
-      expect($state.$current.resolve).toEqual({});
+      expect($state.$current.resolve).toBeDefined();
+      expect(typeof $state.$current.resolve).toBe('object');
     }));
 
     it('should include itself and parent states', inject(function ($state, $q) {