Skip to content

Commit

Permalink
fix(Transition): Use { location: replace } when redirecting a transti…
Browse files Browse the repository at this point in the history
…tion 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 angular-ui/ui-router#3187
Closes #15
  • Loading branch information
christopherthielen committed Jan 6, 2017
1 parent 7e0a8af commit 23e2b78
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 5 deletions.
10 changes: 9 additions & 1 deletion src/transition/transition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
46 changes: 42 additions & 4 deletions test/transitionSpec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {

Expand All @@ -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;

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -749,4 +783,8 @@ describe('transition', function () {
}));
});
});
});

describe("initial url redirect", () => {

});

0 comments on commit 23e2b78

Please sign in to comment.