fix(redirect): Do not update URL after redirect with { location: false }

Closes angular-ui/ui-router#3317

src/transition/interface.ts

@@ -23,7 +23,7 @@ export interface TransitionOptions {
    *
    * - If `true`, it will update the url in the location bar.
    * - If `false`, it will not update the url in the location bar.
-   * - If it is the string "`replace`", it will update the url and also replace the last history record.
+   * - If it is the string `"replace"`, it will update the url and also replace the last history record.
    *
    * @default `true`
    */

src/transition/transition.ts

@@ -8,7 +8,7 @@ import { services } from '../common/coreservices';
 import {
   map, find, extend, mergeR, tail, omit, toJson, arrayTuples, unnestR, identity, anyTrueR
 } from '../common/common';
-import { isObject } from '../common/predicates';
+import { isObject, isUndefined } from '../common/predicates';
 import { prop, propEq, val, not, is } from '../common/hof';
 import { StateDeclaration, StateOrName } from '../state/interface';
 import {
@@ -517,8 +517,10 @@ export class Transition implements IHookRegistry {
 
     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') {
+    // on the new transition (unless the target state explicitly specifies location: false).
+    // This causes the original url to be replaced with the url for the redirect target
+    // so the original url disappears from the browser history.
+    if (this.options().source === 'url' && targetState.options().location !== false) {
       redirectOpts.location = 'replace';
     }
 

test/transitionSpec.ts

@@ -681,9 +681,10 @@ describe('transition', function () {
     });
 
     describe('redirected transition', () => {
-      let urlRedirect;
+      let urlRedirect, requiresAuth;
       beforeEach(() => {
         urlRedirect = router.stateRegistry.register({ name: 'urlRedirect', url: '/urlRedirect', redirectTo: 'redirectTarget' });
+        requiresAuth = router.stateRegistry.register({ name: 'requiresAuth', url: '/requiresAuth' });
         router.stateRegistry.register({ name: 'redirectTarget', url: '/redirectTarget' });
       });
 
@@ -703,6 +704,9 @@ describe('transition', function () {
         router.transitionService.onSuccess({}, () => {
           expect(transitionTo).toHaveBeenCalledWith(urlRedirect, {}, { inherit: true, source: 'url' });
 
+          expect(router.stateService.current.name).toBe('redirectTarget');
+          expect(router.urlService.path()).toBe('/redirectTarget');
+
           expect(url.calls.count()).toEqual(2);
           expect(url.calls.argsFor(0)).toEqual(["/urlRedirect"]);
           expect(url.calls.argsFor(1)).toEqual(["/redirectTarget", true]);
@@ -713,6 +717,29 @@ describe('transition', function () {
         router.urlService.url('/urlRedirect');
       });
 
+      it("should not replace the current url when redirecting a url sync with { location: false }", (done) => {
+        router.transitionService.onBefore({ to: 'requiresAuth' }, trans => {
+          return router.stateService.target('redirectTarget', null, { location: false })
+        });
+
+        let url = spyOn(router.urlService, "url").and.callThrough();
+        let transitionTo = spyOn(router.stateService, "transitionTo").and.callThrough();
+
+        router.transitionService.onSuccess({}, () => {
+          expect(transitionTo).toHaveBeenCalledWith(requiresAuth, {}, { inherit: true, source: 'url' });
+
+          expect(router.globals.current.name).toBe("redirectTarget");
+          expect(router.urlService.path()).toBe('/requiresAuth');
+
+          expect(url.calls.count()).toEqual(1);
+          expect(url.calls.argsFor(0)).toEqual(["/requiresAuth"]);
+
+          done();
+        });
+
+        router.urlService.url('/requiresAuth');
+      });
+
     });
   });