From 23e2b7895f57a0d5469ad9043e987d023d386b02 Mon Sep 17 00:00:00 2001 From: Chris Thielen Date: Fri, 6 Jan 2017 14:21:39 -0600 Subject: [PATCH] fix(Transition): Use { location: replace } when redirecting a transtition in response to a URL sync This fixes a problem that occurred when the url sync caused a redirection: - URL changes (to `/foo`) - Router synchronizes - Router redirects elsewhere (to `/bar`) - Url is updated Now the browser history has two history entries: - `/foo` - `/bar` --- Now, when the URL is updated, it uses `{ location: replace }` so the browser history only has one entry: - `/bar` Closes https://github.com/angular-ui/ui-router/issues/3187 Closes #15 --- src/transition/transition.ts | 10 +++++++- test/transitionSpec.ts | 46 ++++++++++++++++++++++++++++++++---- 2 files changed, 51 insertions(+), 5 deletions(-) diff --git a/src/transition/transition.ts b/src/transition/transition.ts index df743e2b..a9022d75 100644 --- a/src/transition/transition.ts +++ b/src/transition/transition.ts @@ -508,7 +508,15 @@ export class Transition implements IHookRegistry { if (++redirects > 20) throw new Error(`Too many consecutive Transition redirects (20+)`); } - let newOptions = extend({}, this.options(), targetState.options(), { redirectedFrom: this, source: "redirect" }); + let redirectOpts: TransitionOptions = { redirectedFrom: this, source: "redirect" }; + // If the original transition was caused by URL sync, then use { location: 'replace' } + // on the new transition (unless the target state explicitly specifies location) + if (this.options().source === 'url') { + redirectOpts.location = 'replace'; + } + + let newOptions = extend({}, this.options(), targetState.options(), redirectOpts); + targetState = new TargetState(targetState.identifier(), targetState.$state(), targetState.params(), newOptions); let newTransition = this.router.transitionService.create(this._treeChanges.from, targetState); diff --git a/test/transitionSpec.ts b/test/transitionSpec.ts index f7190381..5464c78d 100644 --- a/test/transitionSpec.ts +++ b/test/transitionSpec.ts @@ -2,8 +2,8 @@ import { PathNode } from "../src/path/node"; import { UIRouter, RejectType, Rejection, pluck, services, TransitionService, StateService, Resolvable, Transition } from "../src/index"; -import * as vanilla from "../src/vanilla"; import { tree2Array, PromiseResult } from "./_testUtils"; +import { TestingPlugin } from "./_testingPlugin"; describe('transition', function () { @@ -30,8 +30,7 @@ describe('transition', function () { beforeEach(() => { router = new UIRouter(); - router.plugin(vanilla.servicesPlugin); - router.plugin(vanilla.hashLocationPlugin); + router.plugin(TestingPlugin); $state = router.stateService; $transitions = router.transitionService; @@ -60,7 +59,7 @@ describe('transition', function () { states.forEach(state => router.stateRegistry.register(state)); }); - describe('provider', () => { + describe('service', () => { describe('async event hooks:', () => { it('$transition$.promise should resolve on success', (done) => { var result = new PromiseResult(); @@ -674,6 +673,41 @@ describe('transition', function () { .then(done, done); })); }); + + describe('redirected transition', () => { + let urlRedirect; + beforeEach(() => { + urlRedirect = router.stateRegistry.register({ name: 'urlRedirect', url: '/urlRedirect', redirectTo: 'redirectTarget' }); + router.stateRegistry.register({ name: 'redirectTarget', url: '/redirectTarget' }); + }); + + it("should not replace the current url when redirecting a state.go transition", async (done) => { + let spy = spyOn(router.urlService, "url").and.callThrough(); + + await $state.go("urlRedirect"); + expect(router.urlService.path()).toBe("/redirectTarget"); + expect(spy).toHaveBeenCalledWith("/redirectTarget", false); + done(); + }); + + it("should replace the current url when redirecting a url sync", (done) => { + let url = spyOn(router.urlService, "url").and.callThrough(); + let transitionTo = spyOn(router.stateService, "transitionTo").and.callThrough(); + + router.transitionService.onSuccess({}, () => { + expect(transitionTo).toHaveBeenCalledWith(urlRedirect, {}, { inherit: true, source: 'url' }); + + expect(url.calls.count()).toEqual(2); + expect(url.calls.argsFor(0)).toEqual(["/urlRedirect"]); + expect(url.calls.argsFor(1)).toEqual(["/redirectTarget", true]); + + done(); + }); + + router.urlService.url('/urlRedirect'); + }); + + }); }); describe('Transition() instance', function() { @@ -749,4 +783,8 @@ describe('transition', function () { })); }); }); +}); + +describe("initial url redirect", () => { + }); \ No newline at end of file