diff --git a/lib/router/router.js b/lib/router/router.js index ecbc0349..5f34f374 100644 --- a/lib/router/router.js +++ b/lib/router/router.js @@ -26,6 +26,7 @@ function Router(_options) { this.oldState = undefined; this.currentHandlerInfos = undefined; this.state = undefined; + this.currentSequence = 0; this.recognizer = new RouteRecognizer(); this.reset(); @@ -59,7 +60,7 @@ function getTransitionByIntent(intent, isIntermediate) { } // Create a new transition to the destination route. - newTransition = new Transition(this, intent, newState); + newTransition = new Transition(this, intent, newState, undefined, this.activeTransition); // Abort and usurp any previously active transition. if (this.activeTransition) { @@ -625,7 +626,28 @@ function updateURL(transition, state/*, inputUrl*/) { params.queryParams = transition._visibleQueryParams || state.queryParams; var url = router.recognizer.generate(handlerName, params); - if (urlMethod === 'replace') { + // transitions during the initial transition must always use replaceURL. + // When the app boots, you are at a url, e.g. /foo. If some handler + // redirects to bar as part of the initial transition, you don't want to + // add a history entry for /foo. If you do, pressing back will immediately + // hit the redirect again and take you back to /bar, thus killing the back + // button + var initial = transition.isCausedByInitialTransition; + + // say you are at / and you click a link to route /foo. In /foo's + // handler, the transition is aborted using replacewith('/bar'). + // Because the current url is still /, the history entry for / is + // removed from the history. Clicking back will take you to the page + // you were on before /, which is often not even the app, thus killing + // the back button. That's why updateURL is always correct for an + // aborting transition that's not the initial transition + var replaceAndNotAborting = ( + urlMethod === 'replace' && + !transition.isCausedByAbortingTransition + ); + + + if (initial || replaceAndNotAborting) { router.replaceURL(url); } else { router.updateURL(url); diff --git a/lib/router/transition.js b/lib/router/transition.js index 328f0c27..2e48d401 100644 --- a/lib/router/transition.js +++ b/lib/router/transition.js @@ -16,7 +16,7 @@ import { trigger, slice, log, promiseLabel } from './utils'; @param {Object} error @private */ -function Transition(router, intent, state, error) { +function Transition(router, intent, state, error, previousTransition) { var transition = this; this.state = state || router.state; this.intent = intent; @@ -40,6 +40,18 @@ function Transition(router, intent, state, error) { return; } + // if you're doing multiple redirects, need the new transition to know if it + // is actually part of the first transition or not. Any further redirects + // in the initial transition also need to know if they are part of the + // initial transition + this.isCausedByAbortingTransition = !!previousTransition; + this.isCausedByInitialTransition = ( + previousTransition && ( + previousTransition.isCausedByInitialTransition || + previousTransition.sequence === 0 + ) + ); + if (state) { this.params = state.params; this.queryParams = state.queryParams; @@ -58,7 +70,7 @@ function Transition(router, intent, state, error) { this.pivotHandler = handlerInfo.handler; } - this.sequence = Transition.currentSequence++; + this.sequence = router.currentSequence++; this.promise = state.resolve(checkForAbort, this)['catch'](function(result) { if (result.wasAborted || transition.isAborted) { return Promise.reject(logAbort(transition)); @@ -80,7 +92,6 @@ function Transition(router, intent, state, error) { } } -Transition.currentSequence = 0; Transition.prototype = { targetName: null, diff --git a/test/tests/router_test.js b/test/tests/router_test.js index f40458c6..517653b4 100644 --- a/test/tests/router_test.js +++ b/test/tests/router_test.js @@ -3327,6 +3327,660 @@ test("intermediateTransitionTo() forces an immediate intermediate transition tha counterAt(7, "original transition promise resolves"); }); + +test("Calling transitionTo during initial transition in validation hook should use replaceURL", function(assert) { + assert.expect(4); + map(assert, function(match) { + match("/foo").to('foo'); + match("/bar").to('bar'); + }); + + var fooModelCount = 0, barModelCount = 0; + + router.updateURL = function(updateUrl) { + url = updateUrl; + assert.ok(false, "The url was not correctly replaced on initial transition"); + }; + + router.replaceURL = function(replaceURL) { + url = replaceURL; + assert.ok(true, "The url was replaced correctly on initial transition"); + }; + + var fooHandler = { + model: function() { + fooModelCount++; + router.transitionTo('/bar'); + } + }; + + var barHandler = { + model: function() { + barModelCount++; + } + }; + + handlers = { + foo: fooHandler, + bar: barHandler + }; + + transitionTo(router, '/foo'); + + assert.equal(url, '/bar'); + assert.equal(fooModelCount, 1); + assert.equal(barModelCount, 1); +}); + +test("Calling transitionTo during initial transition in validation hook with multiple redirects should use replaceURL", function(assert) { + assert.expect(5); + map(assert, function(match) { + match("/foo").to('foo'); + match("/bar").to('bar'); + match("/baz").to('baz'); + }); + + var fooModelCount = 0, barModelCount = 0, bazModelCount = 0; + + router.updateURL = function(updateUrl) { + url = updateUrl; + assert.ok(false, "The url was not correctly replaced on initial transition"); + }; + + router.replaceURL = function(replaceURL) { + url = replaceURL; + assert.ok(true, "The url was replaced correctly on initial transition"); + }; + + var fooHandler = { + model: function() { + fooModelCount++; + router.transitionTo('/bar'); + } + }; + + var barHandler = { + model: function() { + barModelCount++; + router.transitionTo('/baz'); + } + }; + + var bazHandler = { + model: function() { + bazModelCount++; + } + }; + + handlers = { + foo: fooHandler, + bar: barHandler, + baz: bazHandler + }; + + transitionTo(router, '/foo'); + + assert.equal(url, '/baz'); + assert.equal(fooModelCount, 1); + assert.equal(barModelCount, 1); + assert.equal(bazModelCount, 1); +}); + +test("Calling transitionTo after initial transition in validation hook should use updateUrl", function(assert) { + assert.expect(8); + + map(assert, function(match) { + match("/foo").to('foo'); + match("/bar").to('bar'); + }); + + var fooModelCount = 0, barModelCount = 0; + + router.updateURL = function(updateUrl) { + url = updateUrl; + assert.ok(true, "updateURL should be used"); + }; + + router.replaceURL = function(replaceURL) { + url = replaceURL; + assert.ok(false, "replaceURL should not be used"); + }; + + var fooHandler = { + model: function() { + fooModelCount++; + router.transitionTo('/bar'); + } + }; + + var barHandler = { + model: function() { + barModelCount++; + } + }; + + handlers = { + foo: fooHandler, + bar: barHandler + }; + + transitionTo(router, '/bar'); + + assert.equal(url, '/bar'); + assert.equal(barModelCount, 1, 'Bar model should be called once'); + assert.equal(fooModelCount, 0, 'Foo model should not be called'); + + transitionTo(router, '/foo'); + + assert.equal(url, '/bar'); + assert.equal(barModelCount, 2, 'Bar model should be called twice'); + assert.equal(fooModelCount, 1, 'Foo model should be called once'); +}); + +test("Calling transitionTo after initial transition in validation hook with multiple redirects should use updateUrl", function(assert) { + assert.expect(10); + + map(assert, function(match) { + match('/foo').to('foo'); + match('/bar').to('bar'); + match('/baz').to('baz'); + }); + + var fooModelCount = 0, barModelCount = 0, bazModelCount = 0; + + router.updateURL = function(updateUrl) { + url = updateUrl; + assert.ok(true, "updateURL should be used"); + }; + + router.replaceURL = function(replaceURL) { + url = replaceURL; + assert.ok(false, "replaceURL should not be used"); + }; + + var fooHandler = { + model: function() { + fooModelCount++; + router.transitionTo('/bar'); + } + }; + + var barHandler = { + model: function() { + barModelCount++; + router.transitionTo('/baz'); + } + }; + + var bazHandler = { + model: function() { + bazModelCount++; + } + }; + + handlers = { + foo: fooHandler, + bar: barHandler, + baz: bazHandler + }; + + transitionTo(router, '/baz'); + + assert.equal(url, '/baz'); + assert.equal(bazModelCount, 1, 'Baz model should be called once'); + assert.equal(fooModelCount, 0, 'Foo model should not be called'); + assert.equal(barModelCount, 0, 'Bar model should not be called'); + + transitionTo(router, '/foo'); + + assert.equal(url, '/baz'); + assert.equal(bazModelCount, 2, 'Baz model should be called thrice'); + assert.equal(fooModelCount, 1, 'Foo model should be called once'); + assert.equal(barModelCount, 1, 'Bar model should be called once'); +}); + + +test("Calling replaceWith during initial transition in validation hook should use replaceURL", function(assert) { + assert.expect(4); + map(assert, function(match) { + match("/foo").to('foo'); + match("/bar").to('bar'); + }); + + var fooModelCount = 0, barModelCount = 0; + + router.updateURL = function(updateUrl) { + url = updateUrl; + assert.ok(false, "The url was not correctly replaced on initial transition"); + }; + + router.replaceURL = function(replaceURL) { + url = replaceURL; + assert.ok(true, "The url was replaced correctly on initial transition"); + }; + + var fooHandler = { + model: function() { + fooModelCount++; + router.replaceWith('/bar'); + } + }; + + var barHandler = { + model: function() { + barModelCount++; + } + }; + + handlers = { + foo: fooHandler, + bar: barHandler + }; + + transitionTo(router, '/foo'); + + assert.equal(url, '/bar'); + assert.equal(fooModelCount, 1); + assert.equal(barModelCount, 1); +}); + +test("Calling replaceWith during initial transition in validation hook with multiple redirects should use replaceURL", function(assert) { + assert.expect(5); + map(assert, function(match) { + match("/foo").to('foo'); + match("/bar").to('bar'); + match("/baz").to('baz'); + }); + + var fooModelCount = 0, barModelCount = 0, bazModelCount = 0; + + router.updateURL = function(updateUrl) { + url = updateUrl; + assert.ok(false, "The url was not correctly replaced on initial transition"); + }; + + router.replaceURL = function(replaceURL) { + url = replaceURL; + assert.ok(true, "The url was replaced correctly on initial transition"); + }; + + var fooHandler = { + model: function() { + fooModelCount++; + router.replaceWith('/bar'); + } + }; + + var barHandler = { + model: function() { + barModelCount++; + router.replaceWith('/baz'); + } + }; + + var bazHandler = { + model: function() { + bazModelCount++; + } + }; + + handlers = { + foo: fooHandler, + bar: barHandler, + baz: bazHandler + }; + + transitionTo(router, '/foo'); + + assert.equal(url, '/baz'); + assert.equal(fooModelCount, 1, 'should call foo model once'); + assert.equal(barModelCount, 1, 'should call bar model once'); + assert.equal(bazModelCount, 1, 'should call baz model once'); +}); + + +test("Calling replaceWith after initial transition in validation hook should use updateUrl", function(assert) { + assert.expect(8); + + map(assert, function(match) { + match("/foo").to('foo'); + match("/bar").to('bar'); + }); + + var fooModelCount = 0, barModelCount = 0; + + router.updateURL = function(updateUrl) { + url = updateUrl; + assert.ok(true, "updateURL should be used"); + }; + + router.replaceURL = function(replaceURL) { + url = replaceURL; + assert.ok(false, "replaceURL should not be used"); + }; + + var fooHandler = { + model: function() { + fooModelCount++; + router.replaceWith('/bar'); + } + }; + var barHandler = { + model: function() { + barModelCount++; + } + }; + + handlers = { + foo: fooHandler, + bar: barHandler + }; + + transitionTo(router, '/bar'); + + assert.equal(url, '/bar'); + assert.equal(barModelCount, 1, 'Bar model should be called once'); + assert.equal(fooModelCount, 0, 'Foo model should not be called'); + + transitionTo(router, '/foo'); + + assert.equal(url, '/bar'); + assert.equal(barModelCount, 2, 'Bar model should be called twice'); + assert.equal(fooModelCount, 1, 'Foo model should be called once'); +}); + +test("Calling replaceWith after initial transition in validation hook with multiple redirects should use updateUrl", function(assert) { + assert.expect(10); + + map(assert, function(match) { + match("/foo").to('foo'); + match("/bar").to('bar'); + match("/baz").to('baz'); + }); + + var fooModelCount = 0, barModelCount = 0, bazModelCount = 0; + + router.updateURL = function(updateUrl) { + url = updateUrl; + assert.ok(true, "updateURL should be used"); + }; + + router.replaceURL = function(replaceURL) { + url = replaceURL; + assert.ok(false, "replaceURL should not be used"); + }; + + var fooHandler = { + model: function() { + fooModelCount++; + router.replaceWith('/bar'); + } + }; + + var barHandler = { + model: function() { + barModelCount++; + router.replaceWith('/baz'); + } + }; + + var bazHandler = { + model: function() { + bazModelCount++; + } + }; + + handlers = { + foo: fooHandler, + bar: barHandler, + baz: bazHandler + }; + + transitionTo(router, '/baz'); + + assert.equal(url, '/baz'); + assert.equal(bazModelCount, 1, 'Bar model should be called once'); + assert.equal(fooModelCount, 0, 'Foo model should not be called'); + assert.equal(barModelCount, 0, 'Baz model should not be called'); + + transitionTo(router, '/foo'); + + assert.equal(url, '/baz'); + assert.equal(bazModelCount, 2, 'Baz model should be called twice'); + assert.equal(fooModelCount, 1, 'Foo model should be called once'); + assert.equal(barModelCount, 1, 'Bar model should be called once'); +}); + + +test("Mixing multiple types of redirect during initial transition should work", function(assert) { + assert.expect(10); + + map(assert, function(match) { + match("/foo").to('foo'); + match("/bar").to('bar'); + match("/baz").to('baz'); + }); + + var fooModelCount = 0, barModelCount = 0, bazModelCount = 0; + + router.updateURL = function(updateUrl) { + url = updateUrl; + assert.ok(true, "updateURL should be used"); + }; + + router.replaceURL = function(replaceURL) { + url = replaceURL; + assert.ok(false, "replaceURL should not be used"); + }; + + var fooHandler = { + model: function() { + fooModelCount++; + router.replaceWith('/bar'); + } + }; + + var barHandler = { + model: function() { + barModelCount++; + router.transitionTo('/baz'); + } + }; + + var bazHandler = { + model: function() { + bazModelCount++; + } + }; + + handlers = { + foo: fooHandler, + bar: barHandler, + baz: bazHandler + }; + + transitionTo(router, '/baz'); + + assert.equal(url, '/baz'); + assert.equal(bazModelCount, 1, 'Bar model should be called once'); + assert.equal(fooModelCount, 0, 'Foo model should not be called'); + assert.equal(barModelCount, 0, 'Baz model should not be called'); + + transitionTo(router, '/foo'); + + assert.equal(url, '/baz'); + assert.equal(bazModelCount, 2, 'Baz model should be called twice'); + assert.equal(fooModelCount, 1, 'Foo model should be called once'); + assert.equal(barModelCount, 1, 'Bar model should be called once'); +}); + +test("Mixing multiple types of redirects after initial transition should work", function(assert) { + assert.expect(12); + + map(assert, function(match) { + match("/foo").to('foo'); + match("/bar").to('bar'); + match("/baz").to('baz'); + }); + + var fooModelCount = 0, barModelCount = 0, bazModelCount = 0, updateUrlCount = 0, replaceUrlCount = 0; + + router.updateURL = function(updateUrl) { + url = updateUrl; + updateUrlCount++; + }; + + router.replaceURL = function(replaceURL) { + url = replaceURL; + replaceUrlCount++; + }; + + var fooHandler = { + model: function() { + fooModelCount++; + router.replaceWith('/bar'); + } + }; + + var barHandler = { + model: function() { + barModelCount++; + router.transitionTo('/baz'); + } + }; + + var bazHandler = { + model: function() { + bazModelCount++; + } + }; + + handlers = { + foo: fooHandler, + bar: barHandler, + baz: bazHandler + }; + + transitionTo(router, '/baz'); + // actually replaceURL probably makes more sense here, but it's an initial + // transition to a route that the page loaded on, so it's a no-op and doesn't + // cause a problem + assert.equal(replaceUrlCount, 0, 'replaceURL should not be used'); + assert.equal(updateUrlCount, 1, 'updateURL should be used for initial transition'); + assert.equal(url, '/baz'); + assert.equal(bazModelCount, 1, 'Baz model should be called once'); + assert.equal(fooModelCount, 0, 'Foo model should not be called'); + assert.equal(barModelCount, 0, 'Bar model should not be called'); + + transitionTo(router, '/foo'); + + assert.equal(replaceUrlCount, 0, 'replaceURL should not be used'); + assert.equal(updateUrlCount, 2, 'updateURL should be used for subsequent transition'); + assert.equal(url, '/baz'); + assert.equal(bazModelCount, 2, 'Baz model should be called once'); + assert.equal(fooModelCount, 1, 'Foo model should not be called'); + assert.equal(barModelCount, 1, 'Bar model should not be called'); +}); + + +test("Calling replaceWith after initial transition outside validation hook should use replaceURL", function(assert) { + assert.expect(7); + + map(assert, function(match) { + match("/foo").to('foo'); + match("/bar").to('bar'); + }); + + var fooModelCount = 0, barModelCount = 0; + + router.updateURL = function(updateUrl) { + url = updateUrl; + assert.equal(updateUrl, '/foo', "incorrect url for updateURL"); + }; + + router.replaceURL = function(replaceUrl) { + url = replaceUrl; + assert.equal(replaceUrl, '/bar', "incorrect url for replaceURL"); + }; + + var fooHandler = { + model: function() { + fooModelCount++; + } + }; + var barHandler = { + model: function() { + barModelCount++; + } + }; + + handlers = { + foo: fooHandler, + bar: barHandler + }; + + transitionTo(router, '/foo'); + + assert.equal(url, '/foo', "failed initial transition"); + assert.equal(fooModelCount, 1, 'Foo model should be called once'); + assert.equal(barModelCount, 0, 'Bar model should not be called'); + + router.replaceWith('/bar'); + flushBackburner(); + + assert.equal(fooModelCount, 1, 'Foo model should be called once'); + assert.equal(barModelCount, 1, 'Bar model should be called once'); +}); + +test("Calling transitionTo after initial transition outside validation hook should use updateUrl", function(assert) { + assert.expect(7); + + map(assert, function(match) { + match("/foo").to('foo'); + match("/bar").to('bar'); + }); + + var fooModelCount = 0, barModelCount = 0; + + router.updateURL = function(updateUrl) { + url = updateUrl; + assert.ok(true, "updateURL is used"); + }; + + router.replaceURL = function(replaceURL) { + url = replaceURL; + assert.ok(false, "replaceURL should not be used"); + }; + + var fooHandler = { + model: function() { + fooModelCount++; + } + }; + var barHandler = { + model: function() { + barModelCount++; + } + }; + + handlers = { + foo: fooHandler, + bar: barHandler + }; + + transitionTo(router, '/foo'); + + assert.equal(url, '/foo', "failed initial transition"); + assert.equal(fooModelCount, 1, 'Foo model should be called once'); + assert.equal(barModelCount, 0, 'Bar model should not be called'); + + transitionTo(router, '/bar'); + + assert.equal(fooModelCount, 1, 'Foo model should be called once'); + assert.equal(barModelCount, 1, 'Bar model should be called once'); +}); + + test("transitioning to the same route with different context should not reenter the route", function(assert) { map(assert, function(match) { match("/project/:project_id").to('project');