feat(transition): Ignore duplicate transitions (double clicks)

Previously, any calls to state.go would supersede the running transition.
If the user starts a transition, then immediately starts another identical transition,
the new transition supersedes the old transition.

Now, if the new transition is identical to the running transition, the new transition is ignored.
Identical basically means: same destination state and parameter values.

Closes #42

Author: christopherthielen

src/common/trace.ts

@@ -39,7 +39,7 @@ import {Transition}  from "../transition/transition";
 import {ActiveUIView, ViewConfig, ViewContext}  from "../view/interface";
 import {stringify, functionToString, maxLength, padString} from "./strings";
 import {Resolvable} from "../resolve/resolvable";
-import {PathNode} from "../path/node";
+import {PathNode} from "../path/pathNode";
 import {PolicyWhen} from "../resolve/interface";
 import {TransitionHook} from "../transition/transitionHook";
 import {HookResult} from "../transition/interface";

src/hooks/ignoredTransition.ts

@@ -3,6 +3,7 @@ import { trace } from '../common/trace';
 import { Rejection } from '../transition/rejectFactory';
 import { TransitionService } from '../transition/transitionService';
 import { Transition } from '../transition/transition';
+import {PathUtils} from '../path/pathFactory';
 
 /**
  * A [[TransitionHookFn]] that skips a transition if it should be ignored
@@ -13,10 +14,21 @@ import { Transition } from '../transition/transition';
  * then the transition is ignored and not processed.
  */
 function ignoredHook(trans: Transition) {
-  if (trans.ignored()) {
-    trace.traceTransitionIgnored(this);
-    return Rejection.ignored().toPromise();
+  const ignoredReason = trans._ignoredReason();
+  if (!ignoredReason) return;
+
+  trace.traceTransitionIgnored(trans);
+
+  const pending = trans.router.globals.transition;
+
+  // The user clicked a link going back to the *current state* ('A')
+  // However, there is also a pending transition in flight (to 'B')
+  // Abort the transition to 'B' because the user now wants to be back at 'A'.
+  if (ignoredReason === 'SameAsCurrent' && pending) {
+    pending.abort();
   }
+
+  return Rejection.ignored().toPromise();
 }
 
 export const registerIgnoredTransitionHook = (transitionService: TransitionService) =>

src/path/index.ts

@@ -1,3 +1,3 @@
 /** @module path */ /** for typedoc */
-export * from "./node";
+export * from "./pathNode";
 export * from "./pathFactory";

src/path/pathFactory.ts

@@ -1,6 +1,8 @@
 /** @module path */ /** for typedoc */
 
-import {extend, find, pick, omit, tail, mergeR, values, unnestR, Predicate, inArray} from "../common/common";
+import {
+  extend, find, pick, omit, tail, mergeR, values, unnestR, Predicate, inArray, arrayTuples,
+} from "../common/common";
 import {prop, propEq, not} from "../common/hof";
 
 import {RawParams} from "../params/interface";
@@ -10,13 +12,14 @@ import {_ViewDeclaration} from "../state/interface";
 
 import {StateObject} from "../state/stateObject";
 import {TargetState} from "../state/targetState";
-import {PathNode} from "../path/node";
+import {GetParamsFn, PathNode} from "./pathNode";
 import {ViewService} from "../view/view";
+import { Param } from '../params/param';
 
 /**
  * This class contains functions which convert TargetStates, Nodes and paths from one type to another.
  */
-export class PathFactory {
+export class PathUtils {
 
   constructor() { }
 
@@ -33,9 +36,9 @@ export class PathFactory {
 
   /** Given a fromPath: PathNode[] and a TargetState, builds a toPath: PathNode[] */
   static buildToPath(fromPath: PathNode[], targetState: TargetState): PathNode[] {
-    let toPath: PathNode[] = PathFactory.buildPath(targetState);
+    let toPath: PathNode[] = PathUtils.buildPath(targetState);
     if (targetState.options().inherit) {
-      return PathFactory.inheritParams(fromPath, toPath, Object.keys(targetState.params()));
+      return PathUtils.inheritParams(fromPath, toPath, Object.keys(targetState.params()));
     }
     return toPath;
   }
@@ -49,7 +52,7 @@ export class PathFactory {
     // Only apply the viewConfigs to the nodes for the given states
     path.filter(node => inArray(states, node.state)).forEach(node => {
       let viewDecls: _ViewDeclaration[] = values(node.state.views || {});
-      let subPath = PathFactory.subPath(path, n => n === node);
+      let subPath = PathUtils.subPath(path, n => n === node);
       let viewConfigs: ViewConfig[][] = viewDecls.map(view => $view.createViewConfig(subPath, view));
       node.views = viewConfigs.reduce(unnestR, []);
     });
@@ -97,15 +100,18 @@ export class PathFactory {
     return <PathNode[]> toPath.map(makeInheritedParamsNode);
   }
 
+  static nonDynamicParams = (node: PathNode): Param[] =>
+      node.state.parameters({ inherit: false })
+          .filter(param => !param.dynamic);
+
   /**
    * Computes the tree changes (entering, exiting) between a fromPath and toPath.
    */
   static treeChanges(fromPath: PathNode[], toPath: PathNode[], reloadState: StateObject): TreeChanges {
     let keep = 0, max = Math.min(fromPath.length, toPath.length);
-    const staticParams = (state: StateObject) =>
-        state.parameters({ inherit: false }).filter(not(prop('dynamic'))).map(prop('id'));
+
     const nodesMatch = (node1: PathNode, node2: PathNode) =>
-        node1.equals(node2, staticParams(node1.state));
+        node1.equals(node2, PathUtils.nonDynamicParams);
 
     while (keep < max && fromPath[keep].state !== reloadState && nodesMatch(fromPath[keep], toPath[keep])) {
       keep++;
@@ -132,6 +138,43 @@ export class PathFactory {
     return { from, to, retained, exiting, entering };
   }
 
+  /**
+   * Returns a new path which is: the subpath of the first path which matches the second path.
+   *
+   * The new path starts from root and contains any nodes that match the nodes in the second path.
+   * It stops before the first non-matching node.
+   *
+   * Nodes are compared using their state property and their parameter values.
+   * If a `paramsFn` is provided, only the [[Param]] returned by the function will be considered when comparing nodes.
+   *
+   * @param pathA the first path
+   * @param pathB the second path
+   * @param paramsFn a function which returns the parameters to consider when comparing
+   *
+   * @returns an array of PathNodes from the first path which match the nodes in the second path
+   */
+  static matching(pathA: PathNode[], pathB: PathNode[], paramsFn?: GetParamsFn): PathNode[] {
+    let done = false;
+    let tuples: PathNode[][] = arrayTuples(pathA, pathB);
+    return tuples.reduce((matching, [nodeA, nodeB]) => {
+      done = done || !nodeA.equals(nodeB, paramsFn);
+      return done ? matching : matching.concat(nodeA);
+    }, []);
+  }
+
+  /**
+   * Returns true if two paths are identical.
+   *
+   * @param pathA
+   * @param pathB
+   * @param paramsFn a function which returns the parameters to consider when comparing
+   * @returns true if the the states and parameter values for both paths are identical
+   */
+  static equals(pathA: PathNode[], pathB: PathNode[], paramsFn?: GetParamsFn): boolean {
+    return pathA.length === pathB.length &&
+        PathUtils.matching(pathA, pathB, paramsFn).length === pathA.length;
+  }
+
   /**
    * Return a subpath of a path, which stops at the first matching node
    *
@@ -149,5 +192,6 @@ export class PathFactory {
   }
 
   /** Gets the raw parameter values from a path */
-  static paramValues = (path: PathNode[]) => path.reduce((acc, node) => extend(acc, node.paramValues), {});
+  static paramValues = (path: PathNode[]) =>
+      path.reduce((acc, node) => extend(acc, node.paramValues), {});
 }

src/path/node.ts renamed to src/path/pathNode.ts

@@ -1,5 +1,5 @@
 /** @module path */ /** for typedoc */
-import {extend, applyPairs, find, allTrueR} from "../common/common";
+import {extend, applyPairs, find, allTrueR, pairs, arrayTuples} from "../common/common";
 import {propEq} from "../common/hof";
 import {StateObject} from "../state/stateObject";
 import {RawParams} from "../params/interface";
@@ -8,6 +8,8 @@ import {Resolvable} from "../resolve/resolvable";
 import {ViewConfig} from "../view/interface";
 
 /**
+ * @internalapi
+ *
  * A node in a [[TreeChanges]] path
  *
  * For a [[TreeChanges]] path, this class holds the stateful information for a single node in the path.
@@ -27,19 +29,19 @@ export class PathNode {
   public views: ViewConfig[];
 
   /** Creates a copy of a PathNode */
-  constructor(state: PathNode);
+  constructor(node: PathNode);
   /** Creates a new (empty) PathNode for a State */
   constructor(state: StateObject);
-  constructor(stateOrPath: any) {
-    if (stateOrPath instanceof PathNode) {
-      let node: PathNode = stateOrPath;
+  constructor(stateOrNode: any) {
+    if (stateOrNode instanceof PathNode) {
+      let node: PathNode = stateOrNode;
       this.state = node.state;
       this.paramSchema = node.paramSchema.slice();
       this.paramValues = extend({}, node.paramValues);
       this.resolvables = node.resolvables.slice();
       this.views = node.views && node.views.slice();
     } else {
-      let state: StateObject = stateOrPath;
+      let state: StateObject = stateOrNode;
       this.state = state;
       this.paramSchema = state.parameters({ inherit: false });
       this.paramValues = {};
@@ -63,42 +65,35 @@ export class PathNode {
    * @returns true if the state and parameter values for another PathNode are
    * equal to the state and param values for this PathNode
    */
-  equals(node: PathNode, keys = this.paramSchema.map(p => p.id)): boolean {
-    const paramValsEq = (key: string) =>
-        this.parameter(key).type.equals(this.paramValues[key], node.paramValues[key]);
-    return this.state === node.state && keys.map(paramValsEq).reduce(allTrueR, true);
-  }
-
-  /** Returns a clone of the PathNode */
-  static clone(node: PathNode) {
-    return new PathNode(node);
+  equals(node: PathNode, paramsFn?: GetParamsFn): boolean {
+    const diff = this.diff(node, paramsFn);
+    return diff && diff.length === 0;
   }
 
   /**
-   * Returns a new path which is a subpath of the first path which matched the second path.
+   * Finds Params with different parameter values on another PathNode.
    *
-   * 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.
+   * Given another node (of the same state), finds the parameter values which differ.
+   * Returns the [[Param]] (schema objects) whose parameter values differ.
    *
-   * @param pathA the first path
-   * @param pathB the second path
-   * @param ignoreDynamicParams don't compare dynamic parameter values
+   * Given another node for a different state, returns `false`
+   *
+   * @param node The node to compare to
+   * @param paramsFn A function that returns which parameters should be compared.
+   * @returns The [[Param]]s which differ, or null if the two nodes are for different states
    */
-  static matching(pathA: PathNode[], pathB: PathNode[], ignoreDynamicParams = true): PathNode[] {
-    let matching: PathNode[] = [];
-
-    for (let i = 0; i < pathA.length && i < pathB.length; i++) {
-      let a = pathA[i], b = pathB[i];
+  diff(node: PathNode, paramsFn?: GetParamsFn): Param[] | false {
+    if (this.state !== node.state) return false;
 
-      if (a.state !== b.state) break;
-
-      let changedParams = Param.changed(a.paramSchema, a.paramValues, b.paramValues)
-          .filter(param => !(ignoreDynamicParams && param.dynamic));
-      if (changedParams.length) break;
-
-      matching.push(a);
-    }
+    const params: Param[] = paramsFn ? paramsFn(this) : this.paramSchema;
+    return Param.changed(params, this.paramValues, node.paramValues);
+  }
 
-    return matching
+  /** Returns a clone of the PathNode */
+  static clone(node: PathNode) {
+    return new PathNode(node);
   }
-}
+}
+
+/** @hidden */
+export type GetParamsFn = (pathNode: PathNode) => Param[];

src/resolve/resolvable.ts

@@ -12,7 +12,7 @@ import {stringify} from "../common/strings";
 import {isFunction, isObject} from "../common/predicates";
 import {Transition} from "../transition/transition";
 import {StateObject} from "../state/stateObject";
-import {PathNode} from "../path/node";
+import {PathNode} from "../path/pathNode";
 
 
 // TODO: explicitly make this user configurable

src/resolve/resolveContext.ts

@@ -5,10 +5,10 @@ import { propEq, not } from "../common/hof";
 import { trace } from "../common/trace";
 import { services, $InjectorLike } from "../common/coreservices";
 import { resolvePolicies, PolicyWhen, ResolvePolicy } from "./interface";
-import { PathNode } from "../path/node";
+import { PathNode } from "../path/pathNode";
 import { Resolvable } from "./resolvable";
 import { StateObject } from "../state/stateObject";
-import { PathFactory } from "../path/pathFactory";
+import { PathUtils } from "../path/pathFactory";
 import { stringify } from "../common/strings";
 import { Transition } from "../transition/transition";
 import { UIInjector } from "../interface";
@@ -82,7 +82,7 @@ export class ResolveContext {
    * `let AB = ABCD.subcontext(a)`
    */
   subContext(state: StateObject): ResolveContext {
-    return new ResolveContext(PathFactory.subPath(this._path, node => node.state === state));
+    return new ResolveContext(PathUtils.subPath(this._path, node => node.state === state));
   }
 
   /**
@@ -164,7 +164,7 @@ export class ResolveContext {
     let node = this.findNode(resolvable);
     // Find which other resolvables are "visible" to the `resolvable` argument
     // subpath stopping at resolvable's node, or the whole path (if the resolvable isn't in the path)
-    let subPath: PathNode[] = PathFactory.subPath(this._path, x => x === node) || this._path;
+    let subPath: PathNode[] = PathUtils.subPath(this._path, x => x === node) || this._path;
     let availableResolvables: Resolvable[] = subPath
         .reduce((acc, node) => acc.concat(node.resolvables), []) //all of subpath's resolvables
         .filter(res => res !== resolvable); // filter out the `resolvable` argument

src/state/stateService.ts

@@ -8,8 +8,8 @@ import { isDefined, isObject, isString } from '../common/predicates';
 import { Queue } from '../common/queue';
 import { services } from '../common/coreservices';
 
-import { PathFactory } from '../path/pathFactory';
-import { PathNode } from '../path/node';
+import { PathUtils } from '../path/pathFactory';
+import { PathNode } from '../path/pathNode';
 
 import { HookResult, TransitionOptions } from '../transition/interface';
 import { defaultTransOpts } from '../transition/transitionService';
@@ -93,7 +93,7 @@ export class StateService {
    * @internalapi
    */
   private _handleInvalidTargetState(fromPath: PathNode[], toState: TargetState) {
-    let fromState = PathFactory.makeTargetState(fromPath);
+    let fromState = PathUtils.makeTargetState(fromPath);
     let globals = this.router.globals;
     const latestThing = () => globals.transitionHistory.peekTail();
     let latest = latestThing();
@@ -340,9 +340,11 @@ export class StateService {
      */
     const rejectedTransitionHandler = (transition: Transition) => (error: any): Promise<any> => {
       if (error instanceof Rejection) {
+        const isLatest = router.globals.lastStartedTransitionId === transition.$id;
+
         if (error.type === RejectType.IGNORED) {
+          isLatest && router.urlRouter.update();
           // Consider ignored `Transition.run()` as a successful `transitionTo`
-          router.urlRouter.update();
           return services.$q.when(globals.current);
         }
 
@@ -355,7 +357,7 @@ export class StateService {
         }
 
         if (error.type === RejectType.ABORTED) {
-          router.urlRouter.update();
+          isLatest && router.urlRouter.update();
           return services.$q.reject(error);
         }
       }
@@ -593,7 +595,7 @@ export class StateService {
     if (!state || !state.lazyLoad) throw new Error("Can not lazy load " + stateOrName);
 
     let currentPath = this.getCurrentPath();
-    let target = PathFactory.makeTargetState(currentPath);
+    let target = PathUtils.makeTargetState(currentPath);
     transition = transition || this.router.transitionService.create(currentPath, target);
 
     return lazyLoadState(transition, state);

src/transition/hookBuilder.ts

@@ -14,7 +14,7 @@ import {
 import {Transition} from "./transition";
 import {TransitionHook} from "./transitionHook";
 import {StateObject} from "../state/stateObject";
-import {PathNode} from "../path/node";
+import {PathNode} from "../path/pathNode";
 import {TransitionService} from "./transitionService";
 import {TransitionEventType} from "./transitionEventType";
 import {RegisteredHook} from "./hookRegistry";

src/transition/hookRegistry.ts

@@ -4,7 +4,7 @@
  */ /** for typedoc */
 import { extend, removeFrom, tail, values, identity, map } from "../common/common";
 import {isString, isFunction} from "../common/predicates";
-import {PathNode} from "../path/node";
+import {PathNode} from "../path/pathNode";
 import {
     TransitionStateHookFn, TransitionHookFn, TransitionHookPhase, TransitionHookScope, IHookRegistry, PathType
 } from "./interface"; // has or is using

src/transition/interface.ts

@@ -7,7 +7,7 @@ import {Predicate} from "../common/common";
 
 import {Transition} from "./transition";
 import {StateObject} from "../state/stateObject";
-import {PathNode} from "../path/node";
+import {PathNode} from "../path/pathNode";
 import {TargetState} from "../state/targetState";
 import {RegisteredHook} from "./hookRegistry";