Skip to content

Commit

Permalink
Unify handler hook logic into callHook util fn
Browse files Browse the repository at this point in the history
This gets rid of lots of 

    if (handler.foo) { handler.foo(); }

in favor of 

    callHook(handler, ‘foo’)

which will also try `_foo` as a hook first,
if one is provided, so as to not clobber any
other API on whatever object is being used
as a route handler.
  • Loading branch information
machty committed Aug 3, 2014
1 parent 785ad4b commit a666cc9
Show file tree
Hide file tree
Showing 16 changed files with 235 additions and 120 deletions.
6 changes: 3 additions & 3 deletions dist/commonjs/router/handler-info.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ var bind = require("./utils").bind;
var merge = require("./utils").merge;
var serialize = require("./utils").serialize;
var promiseLabel = require("./utils").promiseLabel;
var applyHook = require("./utils").applyHook;
var Promise = require("rsvp/promise")["default"];

function HandlerInfo(_props) {
Expand Down Expand Up @@ -89,14 +90,13 @@ HandlerInfo.prototype = {
}
args.push(payload);

var handler = this.handler;
var result = handler[hookName] && handler[hookName].apply(handler, args);
var result = applyHook(this.handler, hookName, args);

if (result && result.isTransition) {
result = null;
}

return Promise.resolve(result, null, this.promiseLabel("Resolve value returned from one of the model hooks"));
return Promise.resolve(result, this.promiseLabel("Resolve value returned from one of the model hooks"));
},

// overridden by subclasses
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
"use strict";
var HandlerInfo = require("../handler-info")["default"];
var resolveHook = require("router/utils").resolveHook;
var merge = require("router/utils").merge;
var subclass = require("router/utils").subclass;
var promiseLabel = require("router/utils").promiseLabel;
Expand All @@ -18,8 +19,9 @@ var UnresolvedHandlerInfoByParam = subclass (HandlerInfo, {
fullParams.queryParams = payload.queryParams;
}

var hookName = typeof this.handler.deserialize === 'function' ?
'deserialize' : 'model';
var handler = this.handler;
var hookName = resolveHook(handler, 'deserialize') ||
resolveHook(handler, 'model');

return this.runSharedModelHook(payload, hookName, [fullParams]);
}
Expand Down
33 changes: 17 additions & 16 deletions dist/commonjs/router/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ var serialize = require("./utils").serialize;
var extractQueryParams = require("./utils").extractQueryParams;
var getChangelist = require("./utils").getChangelist;
var promiseLabel = require("./utils").promiseLabel;
var callHook = require("./utils").callHook;
var TransitionState = require("./transition-state")["default"];
var logAbort = require("./transition").logAbort;
var Transition = require("./transition").Transition;
Expand Down Expand Up @@ -153,11 +154,9 @@ Router.prototype = {
*/
reset: function() {
if (this.state) {
forEach(this.state.handlerInfos, function(handlerInfo) {
forEach(this.state.handlerInfos.slice().reverse(), function(handlerInfo) {
var handler = handlerInfo.handler;
if (handler.exit) {
handler.exit();
}
callHook(handler, 'exit');
});
}

Expand Down Expand Up @@ -221,7 +220,7 @@ Router.prototype = {
},

intermediateTransitionTo: function(name) {
doTransition(this, arguments, true);
return doTransition(this, arguments, true);
},

refresh: function(pivotHandler) {
Expand Down Expand Up @@ -441,8 +440,9 @@ function setupContexts(router, newState, transition) {
forEach(partition.exited, function(handlerInfo) {
var handler = handlerInfo.handler;
delete handler.context;
if (handler.reset) { handler.reset(true, transition); }
if (handler.exit) { handler.exit(transition); }

callHook(handler, 'reset', true, transition);
callHook(handler, 'exit', transition);
});

var oldState = router.oldState = router.state;
Expand All @@ -452,7 +452,7 @@ function setupContexts(router, newState, transition) {
try {
forEach(partition.reset, function(handlerInfo) {
var handler = handlerInfo.handler;
if (handler.reset) { handler.reset(false, transition); }
callHook(handler, 'reset', false, transition);
});

forEach(partition.updatedContext, function(handlerInfo) {
Expand Down Expand Up @@ -483,15 +483,15 @@ function handlerEnteredOrUpdated(currentHandlerInfos, handlerInfo, enter, transi
var handler = handlerInfo.handler,
context = handlerInfo.context;

if (enter && handler.enter) { handler.enter(transition); }
callHook(handler, 'enter', transition);
if (transition && transition.isAborted) {
throw new TransitionAborted();
}

handler.context = context;
if (handler.contextDidChange) { handler.contextDidChange(); }
callHook(handler, 'contextDidChange');

if (handler.setup) { handler.setup(context, transition); }
callHook(handler, 'setup', context, transition);
if (transition && transition.isAborted) {
throw new TransitionAborted();
}
Expand Down Expand Up @@ -554,7 +554,7 @@ function partitionHandlers(oldState, newState) {
unchanged: []
};

var handlerChanged, contextChanged, i, l;
var handlerChanged, contextChanged = false, i, l;

for (i=0, l=newHandlers.length; i<l; i++) {
var oldHandler = oldHandlers[i], newHandler = newHandlers[i];
Expand Down Expand Up @@ -772,9 +772,10 @@ function notifyExistingHandlers(router, newState, newTransition) {
var oldHandlers = router.state.handlerInfos,
changing = [],
leavingIndex = null,
leaving, leavingChecker, i, oldHandler, newHandler;
leaving, leavingChecker, i, oldHandlerLen, oldHandler, newHandler;

for (i = 0; i < oldHandlers.length; i++) {
oldHandlerLen = oldHandlers.length;
for (i = 0; i < oldHandlerLen; i++) {
oldHandler = oldHandlers[i];
newHandler = newState.handlerInfos[i];

Expand All @@ -789,9 +790,9 @@ function notifyExistingHandlers(router, newState, newTransition) {
}

if (leavingIndex !== null) {
leaving = oldHandlers.slice(leavingIndex, oldHandlers.length);
leaving = oldHandlers.slice(leavingIndex, oldHandlerLen);
leavingChecker = function(name) {
for (var h = 0; h < leaving.length; h++) {
for (var h = 0, len = leaving.length; h < len; h++) {
if (leaving[h].name === name) {
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,15 @@ exports["default"] = subclass(TransitionIntent, {

applyToHandlers: function(oldState, handlers, getHandler, targetRouteName, isIntermediate, checkingIfActive) {

var i;
var i, len;
var newState = new TransitionState();
var objects = this.contexts.slice(0);

var invalidateIndex = handlers.length;

// Pivot handlers are provided for refresh transitions
if (this.pivotHandler) {
for (i = 0; i < handlers.length; ++i) {
for (i = 0, len = handlers.length; i < len; ++i) {
if (getHandler(handlers[i].handler) === this.pivotHandler) {
invalidateIndex = i;
break;
Expand Down
13 changes: 6 additions & 7 deletions dist/commonjs/router/transition-state.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
var ResolvedHandlerInfo = require("./handler-info").ResolvedHandlerInfo;
var forEach = require("./utils").forEach;
var promiseLabel = require("./utils").promiseLabel;
var callHook = require("./utils").callHook;
var Promise = require("rsvp/promise")["default"];

function TransitionState(other) {
Expand Down Expand Up @@ -46,13 +47,13 @@ TransitionState.prototype = {
.then(resolveOneHandlerInfo, null, this.promiseLabel('Resolve handler'))['catch'](handleError, this.promiseLabel('Handle error'));

function innerShouldContinue() {
return Promise.resolve(shouldContinue(), promiseLabel("Check if should continue"))['catch'](function(reason) {
return Promise.resolve(shouldContinue(), currentState.promiseLabel("Check if should continue"))['catch'](function(reason) {
// We distinguish between errors that occurred
// during resolution (e.g. beforeModel/model/afterModel),
// and aborts due to a rejecting promise from shouldContinue().
wasAborted = true;
return Promise.reject(reason);
}, promiseLabel("Handle abort"));
}, currentState.promiseLabel("Handle abort"));
}

function handleError(error) {
Expand Down Expand Up @@ -82,14 +83,12 @@ TransitionState.prototype = {
// routes don't re-run the model hooks for this
// already-resolved route.
var handler = resolvedHandlerInfo.handler;
if (handler && handler.redirect) {
handler.redirect(resolvedHandlerInfo.context, payload);
}
callHook(handler, 'redirect', resolvedHandlerInfo.context, payload);
}

// Proceed after ensuring that the redirect hook
// didn't abort this transition by transitioning elsewhere.
return innerShouldContinue().then(resolveOneHandlerInfo, null, promiseLabel('Resolve handler'));
return innerShouldContinue().then(resolveOneHandlerInfo, null, currentState.promiseLabel('Resolve handler'));
}

function resolveOneHandlerInfo() {
Expand All @@ -105,7 +104,7 @@ TransitionState.prototype = {
var handlerInfo = currentState.handlerInfos[payload.resolveIndex];

return handlerInfo.resolve(innerShouldContinue, payload)
.then(proceed, null, promiseLabel('Proceed'));
.then(proceed, null, currentState.promiseLabel('Proceed'));
}
}
};
Expand Down
2 changes: 1 addition & 1 deletion dist/commonjs/router/transition.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ function Transition(router, intent, state, error) {

var len = state.handlerInfos.length;
if (len) {
this.targetName = state.handlerInfos[state.handlerInfos.length-1].name;
this.targetName = state.handlerInfos[len-1].name;
}

for (var i = 0; i < len; ++i) {
Expand Down
26 changes: 24 additions & 2 deletions dist/commonjs/router/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,29 @@ exports.promiseLabel = promiseLabel;function subclass(parentConstructor, proto)
return C;
}

exports.subclass = subclass;exports.merge = merge;
exports.subclass = subclass;function resolveHook(obj, hookName) {
if (!obj) { return; }
var underscored = "_" + hookName;
return obj[underscored] && underscored ||
obj[hookName] && hookName;
}

function callHook(obj, hookName) {
var args = slice.call(arguments, 2);
return applyHook(obj, hookName, args);
}

function applyHook(obj, _hookName, args) {
var hookName = resolveHook(obj, _hookName);
if (hookName) {
return obj[hookName].apply(obj, args);
}
}

exports.merge = merge;
exports.slice = slice;
exports.isParam = isParam;
exports.coerceQueryParamsToString = coerceQueryParamsToString;
exports.coerceQueryParamsToString = coerceQueryParamsToString;
exports.callHook = callHook;
exports.resolveHook = resolveHook;
exports.applyHook = applyHook;

0 comments on commit a666cc9

Please sign in to comment.