feat(ng2.UrlRouter): Implement { location: replace }

refactor(core.$location): BC-BREAK remove $location.replace() and $location.url(url) in favor of `$location.setUrl(url, replace)` Closes #2850
angular-ui · Sep 4, 2016 · b8c6146 · b8c6146
1 parent 7bca101
commit b8c6146
Show file tree

Hide file tree

Showing 9 changed files with 44 additions and 48 deletions.
diff --git a/src/common/coreservices.ts b/src/common/coreservices.ts
@@ -19,7 +19,7 @@ let services: CoreServices = {
   template: <any> {}
 };
 
-["replace", "url", "path", "search", "hash", "onChange"]
+["setUrl", "path", "search", "hash", "onChange"]
     .forEach(key => services.location[key] = notImplemented(key));
 
 ["port", "protocol", "host", "baseHref", "html5Mode", "hashPrefix" ]
@@ -58,8 +58,7 @@ export interface CoreServices {
 }
 
 export interface LocationServices {
-  replace(): void;
-  url(newurl: string): string;
+  setUrl(newurl: string, replace?: boolean): void;
   url(): string;
   path(): string;
   search(): string;

diff --git a/src/justjs.ts b/src/justjs.ts
@@ -86,8 +86,9 @@ let loc = <any> services.location;
 loc.hash = () => "";
 loc.path = () => location.hash.replace(/^#/, "");
 loc.search = () => location.search;
-loc.url = (url: string) => { if (url) location.hash = url; return loc.path(); };
-loc.replace = () => { console.log(new Error("not impl")); };
+loc.setUrl = (url: string, replace: boolean = true) => {
+  if (url) location.hash = url;
+};
 loc.onChange = (cb: (ev?: HashChangeEvent) => any) => {
   window.addEventListener("hashchange", cb, false);
 };

diff --git a/src/ng1/services.ts b/src/ng1/services.ts
@@ -197,6 +197,11 @@ function $uiRouter($locationProvider: ILocationProvider) {
       html5Mode = isObject(html5Mode) ? html5Mode.enabled : html5Mode;
       return html5Mode && $sniffer.history;
     };
+
+    services.location.setUrl = (newUrl: string, replace = false) =>  {
+      $location.url(newUrl)
+      if (replace) $location.replace();
+    };
 
     services.template.get = (url: string) =>
         $http.get(url, { cache: $templateCache, headers: { Accept: 'text/html' }}).then(prop("data"));

diff --git a/src/ng2/location.ts b/src/ng2/location.ts
@@ -24,7 +24,7 @@ export class UIRouterLocation {
   }
 
   init() {
-    let loc = <any> services.location;
+    let loc = services.location;
     let locSt = this.locationStrategy;
 
     if (this.isHashBang) {
@@ -43,24 +43,23 @@ export class UIRouterLocation {
     }
 
 
-    loc.search = () => {
+    loc.search = <any> (() => {
       let queryString = splitOnHash(splitOnQuestionMark(locSt.path())[1])[0];
       return queryString.split("&").map(kv => splitOnEquals(kv)).reduce(applyPairs, {});
-    };
+    });
 
-    loc.url = (url: string) => {
+    loc.setUrl = (url: string, replace: boolean = false) => {
       if(isDefined(url)) {
         let split = splitOnQuestionMark(url);
-        locSt.pushState(null, null, split[0], split[1]);
+        if (replace) {
+          locSt.replaceState(null, null, split[0], split[1]);
+        } else {
+          locSt.pushState(null, null, split[0], split[1]);
+        }
       }
-      return locSt.path()
-    };
-
-    loc.replace = () => {
-      console.log(new Error('$location.replace() not impl'))
     };
 
-    loc.onChange = (cb: LocationChangeListener) => locSt.onPopState(cb);
+    loc.onChange = (cb: LocationChangeListener) => locSt.onPopState(cb) as any;
 
     let locCfg = <any> services.locationConfig;
 

diff --git a/src/url/urlRouter.ts b/src/url/urlRouter.ts
@@ -49,8 +49,7 @@ function update(rules: Function[], otherwiseFn: Function, evt?: any) {
 
     if (!handled) return false;
     if (isString(handled)) {
-      $location.replace();
-      $location.url(handled);
+      $location.setUrl(handled, true);
     }
     return true;
   }
@@ -355,13 +354,12 @@ export class UrlRouter {
    */
   update(read?: boolean) {
     if (read) {
-      this.location = $location.url();
+      this.location = $location.path();
       return;
     }
-    if ($location.url() === this.location) return;
+    if ($location.path() === this.location) return;
 
-    $location.url(this.location);
-    $location.replace();
+    $location.setUrl(this.location, true);
   }
 
   /**
@@ -374,8 +372,8 @@ export class UrlRouter {
    * @param options
    */
   push(urlMatcher: UrlMatcher, params: StateParams, options: { replace?: (string|boolean) }) {
-    $location.url(urlMatcher.format(params || {}));
-    if (options && options.replace) $location.replace();
+    let replace = options && !!options.replace;
+    $location.setUrl(urlMatcher.format(params || {}), replace);
   }
 
   /**

diff --git a/test/core/stateRegistrySpec.ts b/test/core/stateRegistrySpec.ts
@@ -64,7 +64,7 @@ describe("StateRegistry", () => {
       registry.register(state);
 
       spyOn($state, "transitionTo");
-      services.location.url("/foo");
+      services.location.setUrl("/foo");
       router.urlRouter.sync();
       expect($state.transitionTo['calls'].mostRecent().args[0]).toBe(state.$$state());
 

diff --git a/test/core/stateServiceSpec.ts b/test/core/stateServiceSpec.ts
@@ -63,13 +63,13 @@ describe('stateService', function () {
     }));
 
     it("should not update the URL in response to synchronizing URL", ((done) => {
-      $loc.url('/a/b/c');
-      spyOn($loc, 'url').and.callThrough();
+      $loc.setUrl('/a/b/c');
+      spyOn($loc, 'setUrl').and.callThrough();
       router.urlRouter.sync();
 
       wait().then(() => {
         expect($state.current.name).toBe('C');
-        let pushedUrls = $loc.url.calls.all().map(x => x.args[0]).filter(x => x !== undefined);
+        let pushedUrls = $loc.setUrl.calls.all().map(x => x.args[0]).filter(x => x !== undefined);
         expect(pushedUrls).toEqual([]);
         expect($loc.path()).toBe('/a/b/c');
         done();
@@ -79,13 +79,13 @@ describe('stateService', function () {
     it("should update the URL in response to synchronizing URL then redirecting", ((done) => {
       $transitions.onStart({ to: 'C' }, () => $state.target('D'));
 
-      $loc.url('/a/b/c');
-      spyOn($loc, 'url').and.callThrough();
+      $loc.setUrl('/a/b/c');
+      spyOn($loc, 'setUrl').and.callThrough();
       router.urlRouter.sync();
 
       wait().then(() => {
         expect($state.current.name).toBe('D');
-        let pushedUrls = $loc.url.calls.all().map(x => x.args[0]).filter(x => x !== undefined);
+        let pushedUrls = $loc.setUrl.calls.all().map(x => x.args[0]).filter(x => x !== undefined);
         expect(pushedUrls).toEqual(['/a/b/c/d']);
         expect($loc.path()).toBe('/a/b/c/d');
         done();

diff --git a/test/ng1/stateSpec.js b/test/ng1/stateSpec.js
@@ -1456,12 +1456,13 @@ describe('state', function () {
 
     it('should replace browser history when "replace" enabled', inject(function ($state, $rootScope, $location, $q) {
 
-      spyOn(services.location, 'replace');
+      spyOn(services.location, 'setUrl');
 
       $state.transitionTo('about', {}, { location: 'replace' });
       $q.flush();
 
-      expect(services.location.replace).toHaveBeenCalled();
+      expect(services.location.setUrl).toHaveBeenCalled();
+      expect(services.location.setUrl.calls.argsFor(0)[1]).toBe(true);
     }));
 
     it('should not replace history normally', inject(function ($state, $rootScope, $location, $q) {

diff --git a/test/ng1/urlRouterSpec.ts b/test/ng1/urlRouterSpec.ts
@@ -194,28 +194,21 @@ describe("UrlRouter", function () {
 
     describe("location updates", function() {
       it('can push location changes', inject(function($urlRouter) {
-        spyOn(services.location, "url");
-        spyOn(services.location, "replace");
+        spyOn(services.location, "setUrl");
         $urlRouter.push(makeMatcher("/hello/:name"), { name: "world" });
-
-        expect(services.location.url).toHaveBeenCalledWith("/hello/world");
-        expect(services.location.replace).not.toHaveBeenCalled();
+        expect(services.location.setUrl).toHaveBeenCalledWith("/hello/world", undefined);
       }));
 
       it('can push a replacement location', inject(function($urlRouter, $location) {
-        spyOn(services.location, "url");
-        spyOn(services.location, "replace");
+        spyOn(services.location, "setUrl");
         $urlRouter.push(makeMatcher("/hello/:name"), { name: "world" }, { replace: true });
-
-        expect(services.location.url).toHaveBeenCalledWith("/hello/world");
-        expect(services.location.replace).toHaveBeenCalled();
+        expect(services.location.setUrl).toHaveBeenCalledWith("/hello/world", true);
       }));
 
       it('can push location changes with no parameters', inject(function($urlRouter, $location) {
-        spyOn(services.location, "url");
+        spyOn(services.location, "setUrl");
         $urlRouter.push(makeMatcher("/hello/:name", {params:{name: ""}}));
-
-        expect(services.location.url).toHaveBeenCalledWith("/hello/");
+        expect(services.location.setUrl).toHaveBeenCalledWith("/hello/", undefined);
       }));
 
       it('can push location changes that include a #fragment', inject(function($urlRouter, $location) {
@@ -237,9 +230,9 @@ describe("UrlRouter", function () {
       it('can read and sync a copy of location URL', inject(function($urlRouter, $location) {
         $location.url('/old');
 
-        spyOn(services.location, 'url').and.callThrough();
+        spyOn(services.location, 'path').and.callThrough();
         $urlRouter.update(true);
-        expect(services.location.url).toHaveBeenCalled();
+        expect(services.location.path).toHaveBeenCalled();
 
         $location.url('/new');
         $urlRouter.update();