From 76ab22de78ea170056bda3ab4ec21bba70b05eb9 Mon Sep 17 00:00:00 2001 From: Chris Thielen Date: Wed, 30 Mar 2016 15:46:03 -0500 Subject: [PATCH] fix(ViewHooks): Fix problem with injecting uiCanExit feat(TransitionHook): Allow a transition hook's `this` to be bound Added `{bind: any}` to the HookRegOptions Closes #2661 --- package.json | 2 +- src/ng1/viewDirective.ts | 9 +- src/ng1/viewsBuilder.ts | 4 +- src/resolve/resolveContext.ts | 2 +- src/transition/hookBuilder.ts | 8 +- src/transition/hookRegistry.ts | 14 +-- src/transition/interface.ts | 67 ++++++++----- src/transition/transitionHook.ts | 5 +- test/hookBuilderSpec.ts | 2 +- test/transitionSpec.ts | 2 +- test/viewHookSpec.ts | 161 +++++++++++++++++++++++++++++++ 11 files changed, 233 insertions(+), 43 deletions(-) create mode 100644 test/viewHookSpec.ts diff --git a/package.json b/package.json index 2ecf309c7..3ccb459df 100644 --- a/package.json +++ b/package.json @@ -64,7 +64,7 @@ "jsdoc": "git://github.com/jsdoc3/jsdoc.git#v3.2.2", "karma": "~0.12.0", "karma-chrome-launcher": "~0.1.0", - "karma-jasmine": "~0.3.6", + "karma-jasmine": "^0.3.8", "karma-phantomjs-launcher": "~0.1.0", "karma-script-launcher": "~0.1.0", "karma-systemjs": "^0.7.2", diff --git a/src/ng1/viewDirective.ts b/src/ng1/viewDirective.ts index 19d44139f..88bd2fc8a 100644 --- a/src/ng1/viewDirective.ts +++ b/src/ng1/viewDirective.ts @@ -13,6 +13,7 @@ import {Transition} from "../transition/transition"; import {Node} from "../path/node"; import {Param} from "../params/param"; import {kebobString} from "../common/strings"; +import {HookRegOptions} from "../transition/interface"; export type UIViewData = { $cfg: Ng1ViewConfig; @@ -384,6 +385,7 @@ function registerControllerCallbacks($transitions: TransitionService, controller // Call $onInit() ASAP if (isFunction(controllerInstance.$onInit)) controllerInstance.$onInit(); + var hookOptions: HookRegOptions = { bind: controllerInstance }; // Add component-level hook for onParamsChange if (isFunction(controllerInstance.uiOnParamsChanged)) { // Fire callback on any successful transition @@ -412,18 +414,19 @@ function registerControllerCallbacks($transitions: TransitionService, controller controllerInstance.uiOnParamsChanged(filter(toParams, (val, key) => changedKeys.indexOf(key) !== -1), $transition$); } }; - $scope.$on('$destroy', $transitions.onSuccess({}, ['$transition$', paramsUpdated])); + $scope.$on('$destroy', $transitions.onSuccess({}, ['$transition$', paramsUpdated]), hookOptions); // Fire callback on any IGNORED transition let onDynamic = ($error$, $transition$) => { if ($error$.type === RejectType.IGNORED) paramsUpdated($transition$); }; - $scope.$on('$destroy', $transitions.onError({}, ['$error$', '$transition$', onDynamic])); + $scope.$on('$destroy', $transitions.onError({}, ['$error$', '$transition$', onDynamic]), hookOptions); } // Add component-level hook for uiCanExit if (isFunction(controllerInstance.uiCanExit)) { - $scope.$on('$destroy', $transitions.onBefore({exiting: cfg.node.state.name}, controllerInstance.uiCanExit.bind(controllerInstance))); + var criteria = {exiting: cfg.node.state.name}; + $scope.$on('$destroy', $transitions.onBefore(criteria, controllerInstance.uiCanExit, hookOptions)); } } diff --git a/src/ng1/viewsBuilder.ts b/src/ng1/viewsBuilder.ts index 865c67872..c2eba3172 100644 --- a/src/ng1/viewsBuilder.ts +++ b/src/ng1/viewsBuilder.ts @@ -70,13 +70,13 @@ export function ng1ViewsBuilder(state: State) { } // for ng 1.2 style, process the scope: { input: "=foo" } object -const scopeBindings = bindingsObj => Object.keys(bindingsObj) +const scopeBindings = bindingsObj => Object.keys(bindingsObj || {}) .map(key => [key, /^[=<](.*)/.exec(bindingsObj[key])]) .filter(tuple => isDefined(tuple[1])) .map(tuple => tuple[1][1] || tuple[0]); // for ng 1.3+ bindToController or 1.5 component style, process a $$bindings object -const bindToCtrlBindings = bindingsObj => Object.keys(bindingsObj) +const bindToCtrlBindings = bindingsObj => Object.keys(bindingsObj || {}) .filter(key => !!/[=<]/.exec(bindingsObj[key].mode)) .map(key => bindingsObj[key].attrName); diff --git a/src/resolve/resolveContext.ts b/src/resolve/resolveContext.ts index e912f74e8..694d5cd31 100644 --- a/src/resolve/resolveContext.ts +++ b/src/resolve/resolveContext.ts @@ -164,7 +164,7 @@ export class ResolveContext { let resolvables = this.getResolvablesForFn(fn); trace.tracePathElementInvoke(tail(this._path), fn, Object.keys(resolvables), extend({when: "Now "}, options)); let resolvedLocals = map(resolvables, prop("data")); - return services.$injector.invoke( fn, null, extend({}, locals, resolvedLocals)); + return services.$injector.invoke( fn, options.bind || null, extend({}, locals, resolvedLocals)); } } diff --git a/src/transition/hookBuilder.ts b/src/transition/hookBuilder.ts index da61d62dc..9d526c99a 100644 --- a/src/transition/hookBuilder.ts +++ b/src/transition/hookBuilder.ts @@ -48,8 +48,8 @@ export class HookBuilder { getOnRetainHooks = () => this._buildNodeHooks("onRetain", "retained", tupleSort(), (node) => ({ $state$: node.state })); getOnEnterHooks = () => this._buildNodeHooks("onEnter", "entering", tupleSort(), (node) => ({ $state$: node.state })); getOnFinishHooks = () => this._buildNodeHooks("onFinish", "to", tupleSort(), (node) => ({ $treeChanges$: this.treeChanges })); - getOnSuccessHooks = () => this._buildNodeHooks("onSuccess", "to", tupleSort(), undefined, {async: false, rejectIfSuperseded: false}); - getOnErrorHooks = () => this._buildNodeHooks("onError", "to", tupleSort(), undefined, {async: false, rejectIfSuperseded: false}); + getOnSuccessHooks = () => this._buildNodeHooks("onSuccess", "to", tupleSort(), undefined, { async: false, rejectIfSuperseded: false }); + getOnErrorHooks = () => this._buildNodeHooks("onError", "to", tupleSort(), undefined, { async: false, rejectIfSuperseded: false }); asyncHooks() { let onStartHooks = this.getOnStartHooks(); @@ -79,7 +79,7 @@ export class HookBuilder { matchingNodesProp: string, sortHooksFn: (l: HookTuple, r: HookTuple) => number, getLocals: (node: Node) => any = (node) => ({}), - options: TransitionHookOptions = {}): TransitionHook[] { + options?: TransitionHookOptions): TransitionHook[] { // Find all the matching registered hooks for a given hook type let matchingHooks = this._matchingHooks(hookType, this.treeChanges); @@ -93,7 +93,7 @@ export class HookBuilder { // Return an array of HookTuples return nodes.map(node => { - let _options = extend({ traceData: { hookType, context: node} }, this.baseHookOptions, options); + let _options = extend({ bind: hook.bind, traceData: { hookType, context: node} }, this.baseHookOptions, options); let transitionHook = new TransitionHook(hook.callback, getLocals(node), node.resolveContext, _options); return { hook, node, transitionHook }; }); diff --git a/src/transition/hookRegistry.ts b/src/transition/hookRegistry.ts index f9725ef64..c05bd8e9f 100644 --- a/src/transition/hookRegistry.ts +++ b/src/transition/hookRegistry.ts @@ -4,7 +4,7 @@ import {isString, isFunction} from "../common/predicates"; import {val} from "../common/hof"; import {Node} from "../path/node"; -import {IMatchCriteria, IStateMatch, IEventHook, IHookRegistry, IHookRegistration, TreeChanges, MatchCriterion, IMatchingNodes} from "./interface"; +import {HookRegOptions, HookMatchCriteria, IStateMatch, IEventHook, IHookRegistry, IHookRegistration, TreeChanges, HookMatchCriterion, IMatchingNodes} from "./interface"; import {Glob} from "../common/glob"; import {State} from "../state/stateObject"; @@ -18,7 +18,7 @@ import {State} from "../state/stateObject"; * - If a function, matchState calls the function with the state and returns true if the function's result is truthy. * @returns {boolean} */ -export function matchState(state: State, criterion: MatchCriterion) { +export function matchState(state: State, criterion: HookMatchCriterion) { let toMatch = isString(criterion) ? [criterion] : criterion; function matchGlobs(_state) { @@ -40,16 +40,18 @@ export function matchState(state: State, criterion: MatchCriterion) { export class EventHook implements IEventHook { callback: IInjectable; - matchCriteria: IMatchCriteria; + matchCriteria: HookMatchCriteria; priority: number; + bind: any; - constructor(matchCriteria: IMatchCriteria, callback: IInjectable, options: { priority?: number } = {}) { + constructor(matchCriteria: HookMatchCriteria, callback: IInjectable, options: HookRegOptions = {}) { this.callback = callback; this.matchCriteria = extend({ to: true, from: true, exiting: true, retained: true, entering: true }, matchCriteria); this.priority = options.priority || 0; + this.bind = options.bind || null; } - private static _matchingNodes(nodes: Node[], criterion: MatchCriterion): Node[] { + private static _matchingNodes(nodes: Node[], criterion: HookMatchCriterion): Node[] { if (criterion === true) return nodes; let matching = nodes.filter(node => matchState(node.state, criterion)); return matching.length ? matching : null; @@ -59,7 +61,7 @@ export class EventHook implements IEventHook { * Determines if this hook's [[matchCriteria]] match the given [[TreeChanges]] * * @returns an IMatchingNodes object, or null. If an IMatchingNodes object is returned, its values - * are the matching [[Node]]s for each [[MatchCriterion]] (to, from, exiting, retained, entering) + * are the matching [[Node]]s for each [[HookMatchCriterion]] (to, from, exiting, retained, entering) */ matches(treeChanges: TreeChanges): IMatchingNodes { let mc = this.matchCriteria, _matchingNodes = EventHook._matchingNodes; diff --git a/src/transition/interface.ts b/src/transition/interface.ts index 69720c888..952af4112 100644 --- a/src/transition/interface.ts +++ b/src/transition/interface.ts @@ -88,6 +88,7 @@ export interface TransitionHookOptions { hookType ?: string; target ?: any; traceData ?: any; + bind ?: any; } /** @@ -151,7 +152,27 @@ export interface ITransitionService extends IHookRegistry { } export type IHookGetter = (hookName: string) => IEventHook[]; -export type IHookRegistration = (matchCriteria: IMatchCriteria, callback: IInjectable, options?) => Function; +export type IHookRegistration = (matchCriteria: HookMatchCriteria, callback: IInjectable, options?: HookRegOptions) => Function; + +/** + * These options may be provided when registering a Transition Hook (such as `onStart`) + */ +export interface HookRegOptions { + /** + * Sets the priority of the registered hook + * + * Hooks of the same type (onBefore, onStart, etc) are invoked in priority order. A hook with a higher priority + * is invoked before a hook with a lower priority. + * + * The default hook priority is 0 + */ + priority?: number; + + /** + * Specifies what `this` is bound to during hook invocation. + */ + bind?: any; +} /** * This interface specifies the api for registering Transition Hooks. Both the @@ -255,7 +276,7 @@ export interface IHookRegistry { * @param callback the hook function which will be injected and invoked. * @returns a function which deregisters the hook. */ - onBefore(matchCriteria: IMatchCriteria, callback: IInjectable, options?): Function; + onBefore(matchCriteria: HookMatchCriteria, callback: IInjectable, options?: HookRegOptions): Function; /** * Registers a callback function as an `onStart` Transition Hook. @@ -325,7 +346,7 @@ export interface IHookRegistry { * @param callback the hook function which will be injected and invoked. * @returns a function which deregisters the hook. */ - onStart(matchCriteria: IMatchCriteria, callback: IInjectable, options?): Function; + onStart(matchCriteria: HookMatchCriteria, callback: IInjectable, options?: HookRegOptions): Function; /** * Registers a callback function as an `onEnter` Transition+State Hook. @@ -394,7 +415,7 @@ export interface IHookRegistry { * @param callback the hook function which will be injected and invoked. * @returns a function which deregisters the hook. */ - onEnter(matchCriteria: IMatchCriteria, callback: IInjectable, options?): Function; + onEnter(matchCriteria: HookMatchCriteria, callback: IInjectable, options?: HookRegOptions): Function; /** * Registers a callback function as an `onRetain` Transition+State Hook. @@ -432,7 +453,7 @@ export interface IHookRegistry { * @param callback the hook function which will be injected and invoked. * @returns a function which deregisters the hook. */ - onRetain(matchCriteria: IMatchCriteria, callback: IInjectable, options?): Function; + onRetain(matchCriteria: HookMatchCriteria, callback: IInjectable, options?: HookRegOptions): Function; /** * Registers a callback function as an `onExit` Transition+State Hook. @@ -471,7 +492,7 @@ export interface IHookRegistry { * @param callback the hook function which will be injected and invoked. * @returns a function which deregisters the hook. */ - onExit(matchCriteria: IMatchCriteria, callback: IInjectable, options?): Function; + onExit(matchCriteria: HookMatchCriteria, callback: IInjectable, options?: HookRegOptions): Function; /** * Registers a callback function as an `onFinish` Transition Hook. @@ -504,7 +525,7 @@ export interface IHookRegistry { * @param callback the hook function which will be injected and invoked. * @returns a function which deregisters the hook. */ - onFinish(matchCriteria: IMatchCriteria, callback: IInjectable, options?): Function; + onFinish(matchCriteria: HookMatchCriteria, callback: IInjectable, options?: HookRegOptions): Function; /** * Registers a callback function as an `onSuccess` Transition Hook. @@ -531,7 +552,7 @@ export interface IHookRegistry { * @param callback the hook function which will be injected and invoked. * @returns a function which deregisters the hook. */ - onSuccess(matchCriteria: IMatchCriteria, callback: IInjectable, options?): Function; + onSuccess(matchCriteria: HookMatchCriteria, callback: IInjectable, options?: HookRegOptions): Function; /** * Registers a callback function as an `onError` Transition Hook. @@ -557,7 +578,7 @@ export interface IHookRegistry { * @param callback the hook function which will be injected and invoked. * @returns a function which deregisters the hook. */ - onError(matchCriteria: IMatchCriteria, callback: IInjectable, options?): Function; + onError(matchCriteria: HookMatchCriteria, callback: IInjectable, options?: HookRegOptions): Function; /** * Returns all the registered hooks of a given `hookName` type @@ -571,13 +592,14 @@ export interface IHookRegistry { getHooks(hookName: string): IEventHook[]; } +/** A predicate type which takes a [[State]] and returns a boolean */ export type IStateMatch = Predicate /** * This object is used to configure whether or not a Transition Hook is invoked for a particular transition, * based on the Transition's "to state" and "from state". * * Each property (`to`, `from`, `exiting`, `retained`, and `entering`) can be state globs, a function that takes a - * state, or a boolean (see [[MatchCriterion]]) + * state, or a boolean (see [[HookMatchCriterion]]) * * All properties are optional. If any property is omitted, it is replaced with the value `true`, and always matches. * @@ -631,17 +653,17 @@ export type IStateMatch = Predicate * } * ``` */ -export interface IMatchCriteria { - /** A [[MatchCriterion]] to match the destination state */ - to?: MatchCriterion; - /** A [[MatchCriterion]] to match the original (from) state */ - from?: MatchCriterion; - /** A [[MatchCriterion]] to match any state that would be exiting */ - exiting?: MatchCriterion; - /** A [[MatchCriterion]] to match any state that would be retained */ - retained?: MatchCriterion; - /** A [[MatchCriterion]] to match any state that would be entering */ - entering?: MatchCriterion; +export interface HookMatchCriteria { + /** A [[HookMatchCriterion]] to match the destination state */ + to?: HookMatchCriterion; + /** A [[HookMatchCriterion]] to match the original (from) state */ + from?: HookMatchCriterion; + /** A [[HookMatchCriterion]] to match any state that would be exiting */ + exiting?: HookMatchCriterion; + /** A [[HookMatchCriterion]] to match any state that would be retained */ + retained?: HookMatchCriterion; + /** A [[HookMatchCriterion]] to match any state that would be entering */ + entering?: HookMatchCriterion; } export interface IMatchingNodes { @@ -658,10 +680,11 @@ export interface IMatchingNodes { * which should return a boolean to indicate if a state matches. * Or, `true` to match anything */ -export type MatchCriterion = (string|IStateMatch|boolean) +export type HookMatchCriterion = (string|IStateMatch|boolean) export interface IEventHook { callback: IInjectable; priority: number; + bind: any; matches: (treeChanges: TreeChanges) => IMatchingNodes; } \ No newline at end of file diff --git a/src/transition/transitionHook.ts b/src/transition/transitionHook.ts index 4125a5f25..8c6f141a6 100644 --- a/src/transition/transitionHook.ts +++ b/src/transition/transitionHook.ts @@ -13,12 +13,13 @@ import {ResolveContext} from "../resolve/module"; let REJECT = new RejectFactory(); -let defaultOptions = { +let defaultOptions: TransitionHookOptions = { async: true, rejectIfSuperseded: true, current: noop, transition: null, - traceData: {} + traceData: {}, + bind: null }; export class TransitionHook { diff --git a/test/hookBuilderSpec.ts b/test/hookBuilderSpec.ts index 1cdcb993b..9a50cecec 100644 --- a/test/hookBuilderSpec.ts +++ b/test/hookBuilderSpec.ts @@ -66,7 +66,7 @@ describe('HookBuilder:', function() { }); - describe('MatchCriteria', function() { + describe('HookMatchCriteria', function() { describe('.to', function() { it("should match a transition with same to state", function() { diff --git a/test/transitionSpec.ts b/test/transitionSpec.ts index 33fd847f2..9f71c7a5a 100644 --- a/test/transitionSpec.ts +++ b/test/transitionSpec.ts @@ -516,7 +516,7 @@ describe('transition', function () { }); }); - describe('Transition MatchCriterion', function() { + xdescribe('Transition HookMatchCriterion', function() { it("should", function() { }) diff --git a/test/viewHookSpec.ts b/test/viewHookSpec.ts new file mode 100644 index 000000000..e6aefa309 --- /dev/null +++ b/test/viewHookSpec.ts @@ -0,0 +1,161 @@ +export default ""; + +describe("view hooks", () => { + let app, ctrl, $state, $q, $timeout, log = ""; + let component = { + bindings: { cmpdata: '<' }, + template: '{{$ctrl.cmpdata}}', + }; + + let directive = { + restrict: 'E', + scope: { cmpdata: '=' }, + bindToController: true, + controller: function() {}, + controllerAs: '$ctrl', + template: '{{$ctrl.cmpdata}}', + }; + + beforeEach(() => { + app = angular.module('viewhooks', []); + }); + + beforeEach(module(($stateProvider) => { + ctrl = function controller() { + this.data = "DATA"; + }; + + if (angular.version.minor >= 5) { + app.component('foo', angular.extend({}, component, {controller: ctrl})); + app.component('bar', angular.extend({}, component)); + app.component('baz', angular.extend({}, component)); + } else if (angular.version.minor >= 2) { + app.directive('foo', () => angular.extend({}, directive, {controller: ctrl})); + app.directive('bar', () => angular.extend({}, directive)); + app.directive('baz', () => angular.extend({}, directive)); + } + + $stateProvider.state({ name: "foo", url: "/foo", component: 'foo' }); + $stateProvider.state({ name: "bar", url: "/bar", component: 'bar' }); + $stateProvider.state({ name: "baz", url: "/baz", component: 'baz' }); + })); + + beforeEach(module('viewhooks', 'ui.router')); + + beforeEach(inject((_$state_, _$q_, _$timeout_, $compile, $rootScope) => { + $state = _$state_; + $q = _$q_; + $timeout = _$timeout_; + $compile('
')($rootScope.$new()); + })); + + describe("uiCanExit", () => { + beforeEach(() => { + log = ""; + }); + + let initial = () => { + $state.go('foo'); $q.flush(); $timeout.flush(); + expect(log).toBe(''); + expect($state.current.name).toBe('foo'); + }; + + it("can cancel a transition that would exit the view's state by returning false", () => { + ctrl.prototype.uiCanExit = function() { log += "canexit;"; return false; }; + initial(); + + $state.go('bar'); $q.flush(); $timeout.flush(); + expect(log).toBe('canexit;'); + expect($state.current.name).toBe('foo'); + }); + + it("can allow the transition by returning true", () => { + ctrl.prototype.uiCanExit = function() { log += "canexit;"; return true; }; + initial(); + + $state.go('bar'); $q.flush(); $timeout.flush(); + expect(log).toBe('canexit;'); + expect($state.current.name).toBe('bar'); + }); + + it("can allow the transition by returning nothing", () => { + ctrl.prototype.uiCanExit = function() { log += "canexit;"; }; + initial(); + + $state.go('bar'); $q.flush(); $timeout.flush(); + expect(log).toBe('canexit;'); + expect($state.current.name).toBe('bar'); + }); + + it("can redirect the transition", () => { + ctrl.prototype.uiCanExit = function($state, $transition$) { + log += "canexit;"; + if ($transition$.to().name !== 'baz') { + return $state.target('baz'); + } + }; + initial(); + + $state.go('bar'); $q.flush(); $timeout.flush(); + expect(log).toBe('canexit;canexit;'); // first: redirects, second: allows the transition + expect($state.current.name).toBe('baz'); + }); + + it("can cancel the transition by returning a rejected promise", () => { + ctrl.prototype.uiCanExit = function($q) { log += "canexit;"; return $q.reject('nope'); }; + initial(); + + $state.go('bar'); $q.flush(); $timeout.flush(); + expect(log).toBe('canexit;'); + expect($state.current.name).toBe('foo'); + }); + + it("can wait for a promise and then reject the transition", inject(($timeout) => { + ctrl.prototype.uiCanExit = function() { + log += "canexit;"; + return $timeout(() => { log += "delay;"; return false; }, 1000); + }; + initial(); + + $state.go('bar'); $q.flush(); $timeout.flush(); + expect(log).toBe('canexit;delay;'); + expect($state.current.name).toBe('foo'); + })); + + it("can wait for a promise and then allow the transition", inject(($timeout) => { + ctrl.prototype.uiCanExit = function() { + log += "canexit;"; + return $timeout(() => { log += "delay;"; }, 1000); + }; + initial(); + + $state.go('bar'); $q.flush(); $timeout.flush(); + expect(log).toBe('canexit;delay;'); + expect($state.current.name).toBe('bar'); + })); + + it("has 'this' bound to the controller", () => { + ctrl.prototype.uiCanExit = function() { log += this.data; }; + initial(); + + $state.go('bar'); $q.flush(); $timeout.flush(); + expect(log).toBe('DATA'); + expect($state.current.name).toBe('bar'); + }); + + it("is injectable", () => { + let _state = $state; + ctrl.prototype.uiCanExit = function($state, $transition$) { + log += "canexit;"; + expect($state === _state).toBe(true); + expect(typeof $transition$.treeChanges).toBe("function"); + }; + initial(); + + $state.go('bar'); $q.flush(); $timeout.flush(); + expect(log).toBe('canexit;'); + expect($state.current.name).toBe('bar'); + }); + + }); +}); \ No newline at end of file