BREAKING CHANGE: Replace LocationServices.setUrl with LocationServices.url

christopherthielen · christopherthielen · commit 4c39dcbc6450 · 2016-12-22T17:27:13.000-06:00
This makes `url()` a getter/setter.  It also adds the optional `state` parameter to pass through to the browser history when using pushstate.
End users should not notice this change, but plugin authors may.
diff --git a/src/common/coreservices.ts b/src/common/coreservices.ts
@@ -49,19 +49,35 @@ export interface CoreServices {
 
 export interface LocationServices extends Disposable {
   /**
-   * Changes the url
+   * Gets the current url string
+   *
+   *
+   * #### Example:
+   * ```js
+   * locationServices.url(); // "/some/path?query=value#anchor"
+   * ```
+   *
+   * @returns the current value of the url, as a string.
+   */
+  url(): string;
+
+  /**
+   * Updates the url, or gets the current url
    *
    * Updates the url, changing it to the value in `newurl`
    *
    * #### Example:
    * ```js
-   * locationServices.setUrl("/some/path?query=value#anchor", true);
+   * locationServices.url("/some/path?query=value#anchor", true);
    * ```
    *
    * @param newurl The new value for the URL
    * @param replace When true, replaces the current history entry (instead of appending it) with this new url
+   * @param state The history's state object, i.e., pushState (if the LocationServices implementation supports it)
+   * @return the url (after potentially being processed)
    */
-  setUrl(newurl: string, replace?: boolean): void;
+  url(newurl: string, replace?: boolean, state?: any): string;
+
   /**
    * Gets the path portion of the current url
    *
diff --git a/src/url/urlRouter.ts b/src/url/urlRouter.ts
@@ -81,7 +81,7 @@ export class UrlRouter implements Disposable {
       if (!match) return false;
 
       let result = rule.handler(match, path, search, hash);
-      if (isString(result)) $url.setUrl(result, true);
+      if (isString(result)) $url.url(result, true);
       return true;
     }
 
@@ -118,7 +118,7 @@ export class UrlRouter implements Disposable {
     }
     if ($url.path() === this.location) return;
 
-    $url.setUrl(this.location, true);
+    $url.url(this.location, true);
   }
 
   /**
@@ -133,7 +133,7 @@ export class UrlRouter implements Disposable {
    */
   push(urlMatcher: UrlMatcher, params?: RawParams, options?: { replace?: (string|boolean) }) {
     let replace = options && !!options.replace;
-    this._router.urlService.setUrl(urlMatcher.format(params || {}), replace);
+    this._router.urlService.url(urlMatcher.format(params || {}), replace);
   }
 
   /**
diff --git a/src/url/urlService.ts b/src/url/urlService.ts
@@ -10,7 +10,7 @@ const makeStub = (keys: string[]): any =>
     keys.reduce((acc, key) => (acc[key] = notImplemented(key), acc), { dispose: noop });
 
 /** @hidden */
-const locationServicesFns = ["setUrl", "path", "search", "hash", "onChange"];
+const locationServicesFns = ["url", "path", "search", "hash", "onChange"];
 /** @hidden */
 const locationConfigFns = ["port", "protocol", "host", "baseHref", "html5Mode", "hashPrefix"];
 
@@ -31,7 +31,10 @@ export class UrlService implements LocationServices {
   static locationConfigStub: LocationConfig = makeStub(locationConfigFns);
 
   /** @inheritdoc */
-  setUrl(newurl: string, replace?: boolean): void { return };
+  url(): string;
+  /** @inheritdoc */
+  url(newurl: string, replace?: boolean, state?): void;
+  url(newurl?, replace?, state?): any { return };
   /** @inheritdoc */
   path(): string { return };
   /** @inheritdoc */
diff --git a/src/vanilla/hashLocation.ts b/src/vanilla/hashLocation.ts
@@ -4,7 +4,7 @@
  */ /** */
 import { isDefined } from "../common/index";
 import { LocationServices } from "../common/coreservices";
-import { splitHash, splitQuery, trimHashVal, getParams, locationPluginFactory } from "./utils";
+import { splitHash, splitQuery, trimHashVal, getParams, locationPluginFactory, buildUrl } from "./utils";
 import { UIRouter } from "../router";
 import { LocationPlugin } from "./interface";
 import { pushTo, deregAll } from "../common/common";
@@ -15,30 +15,21 @@ import { BrowserLocationConfig } from "./browserLocationConfig";
 export class HashLocationService implements LocationServices, Disposable {
   private _listeners: Function[] = [];
 
-  hash() {
-      return splitHash(trimHashVal(location.hash))[1];
-  }
-
-  path() {
-      return splitHash(splitQuery(trimHashVal(location.hash))[0])[0];
-  }
+  hash   = () => splitHash(trimHashVal(location.hash))[1];
+  path   = () => splitHash(splitQuery(trimHashVal(location.hash))[0])[0];
+  search = () => getParams(splitQuery(splitHash(trimHashVal(location.hash))[0])[1]);
 
-  search() {
-    return getParams(splitQuery(splitHash(trimHashVal(location.hash))[0])[1]);
-  }
-
-  setUrl(url: string, replace: boolean = true) {
+  url(url?: string, replace: boolean = true): string {
     if (isDefined(url)) location.hash = url;
+    return buildUrl(this);
   };
 
   onChange(cb: EventListener) {
     window.addEventListener('hashchange', cb, false);
     return pushTo(this._listeners, () => window.removeEventListener('hashchange', cb));
   }
 
-  dispose() {
-    deregAll(this._listeners);
-  }
+  dispose = () => deregAll(this._listeners);
 }
 
 /** A `UIRouterPlugin` uses the browser hash to get/set the current location */
diff --git a/src/vanilla/memoryLocation.ts b/src/vanilla/memoryLocation.ts
@@ -4,7 +4,7 @@
  */ /** */
 import { isDefined } from "../common/index";
 import { LocationConfig, LocationServices } from "../common/coreservices";
-import { splitQuery, getParams, splitHash, locationPluginFactory } from "./utils";
+import { splitQuery, getParams, splitHash, locationPluginFactory, buildUrl } from "./utils";
 import { removeFrom, unnestR, deregAll, noop } from "../common/common";
 import { UIRouter } from "../router";
 import { LocationPlugin } from "./interface";
@@ -44,51 +44,32 @@ export class MemoryLocationService implements LocationServices, Disposable {
     this._listeners.forEach(cb => cb(evt));
   }
 
-  url() {
-    let s = this._url.search;
-    let hash = this._url.hash;
-    let query = Object.keys(s).map(key => (isArray(s[key]) ? s[key] : [s[key]]) .map(val => key + "=" + val))
-        .reduce(unnestR, [])
-        .join("&");
+  hash   = () => this._url.hash;
+  path   = () => this._url.path;
+  search = () => this._url.search;
 
-    return this._url.path +
-        (query ? "?" + query : "") +
-        (hash ? "#" + hash : "");
-  }
-
-  hash() {
-    return this._url.hash;
-  }
-
-  path() {
-    return this._url.path;
-  }
-
-  search() {
-    return this._url.search;
-  }
-
-  setUrl(url: string, replace: boolean = false) {
+  url(url?: string, replace: boolean = false, state?): string {
     if (isDefined(url)) {
       let path = splitHash(splitQuery(url)[0])[0];
       let hash = splitHash(url)[1];
       let search = getParams(splitQuery(splitHash(url)[0])[1]);
 
       let oldval = this.url();
       this._url = { path, search, hash };
+
       let newval = this.url();
       this._urlChanged(newval, oldval);
     }
+
+    return buildUrl(this);
   }
 
   onChange(cb: EventListener) {
     this._listeners.push(cb);
     return () => removeFrom(this._listeners, cb);
   }
   
-  dispose() {
-    deregAll(this._listeners);
-  }
+  dispose = () => deregAll(this._listeners);
 }
 
 /** A `UIRouterPlugin` that gets/sets the current location from an in-memory object */
diff --git a/src/vanilla/pushStateLocation.ts b/src/vanilla/pushStateLocation.ts
@@ -4,7 +4,7 @@
  */ /** */
 import { isDefined } from "../common/index";
 import { LocationServices, LocationConfig } from "../common/coreservices";
-import { splitQuery, trimHashVal, getParams, locationPluginFactory } from "./utils";
+import { splitQuery, trimHashVal, getParams, locationPluginFactory, buildUrl } from "./utils";
 import { LocationPlugin } from "./interface";
 import { UIRouter } from "../router";
 import { pushTo, deregAll } from "../common/common";
@@ -44,12 +44,14 @@ export class PushStateLocationService implements LocationServices, Disposable {
     return getParams(splitQuery(this._location.search)[1]);
   }
 
-  setUrl(url: string, replace: boolean = false) {
+  url(url?: string, replace: boolean = false, state?: any): any {
     if (isDefined(url)) {
       let fullUrl = this._config.baseHref() + url;
-      if (replace) this._history.replaceState(null, null, fullUrl);
-      else this._history.pushState(null, null, fullUrl);
+      if (replace) this._history.replaceState(state, null, fullUrl);
+      else this._history.pushState(state, null, fullUrl);
     }
+
+    return buildUrl(this);
   }
 
   onChange(cb: EventListener) {
diff --git a/src/vanilla/utils.ts b/src/vanilla/utils.ts
@@ -3,9 +3,9 @@
  * @module vanilla
  */ /** */
 import {isArray} from "../common/index";
-import { LocationServices, LocationConfig, services } from "../common/coreservices";
+import { LocationServices, LocationConfig } from "../common/coreservices";
 import { UIRouter } from "../router";
-import { identity } from "../common/common";
+import { identity, unnestR } from "../common/common";
 
 const beforeAfterSubstr = (char: string) => (str: string): string[] => {
   if (!str) return ["", ""];
@@ -31,7 +31,21 @@ export const keyValsToObjectR = (accum, [key, val]) => {
 };
 
 export const getParams = (queryString: string): any =>
-  queryString.split("&").filter(identity).map(splitEqual).reduce(keyValsToObjectR, {});
+    queryString.split("&").filter(identity).map(splitEqual).reduce(keyValsToObjectR, {});
+
+export const buildUrl = (loc: LocationServices) => {
+  let path = loc.path();
+  let searchObject = loc.search();
+  let hash = loc.hash();
+
+  let search = Object.keys(searchObject).map(key => {
+    let param = searchObject[key];
+    let vals = isArray(param) ? param : [param];
+    return vals.map(val => key + "=" + val);
+  }).reduce(unnestR, []).join("&");
+
+  return path + (search ? "?" + search : "") + (hash ? "#" + hash : "");
+};
 
 export function locationPluginFactory(
     name: string,
diff --git a/test/lazyLoadSpec.ts b/test/lazyLoadSpec.ts
@@ -287,7 +287,7 @@ describe('future state', function () {
     });
 
     it('triggered by a URL sync should re-parse the URL to activate the lazy loaded state', (done) => {
-      router.urlService.setUrl('/a/def');
+      router.urlService.url('/a/def');
       $urlRouter.sync();
       $transitions.onSuccess({}, () => {
         expect($state.current.name).toBe('A');
@@ -333,7 +333,7 @@ describe('future state', function () {
     });
 
     it('should re-parse the URL to activate the final state', (done) => {
-      router.urlService.setUrl('/a/def/b');
+      router.urlService.url('/a/def/b');
       $urlRouter.sync();
       $transitions.onSuccess({}, () => {
         expect($state.current.name).toBe('A.B');
@@ -451,7 +451,7 @@ describe('future state', function () {
     });
 
     it('should load and activate a nested future state by url sync', (done) => {
-      router.urlService.setUrl('/a/aid/b/bid');
+      router.urlService.url('/a/aid/b/bid');
       $urlRouter.sync();
       $transitions.onSuccess({}, (trans) => {
         expect($state.current.name).toBe('A.B');
diff --git a/test/stateRegistrySpec.ts b/test/stateRegistrySpec.ts
@@ -64,7 +64,7 @@ describe("StateRegistry", () => {
       registry.register(state);
 
       spyOn($state, "transitionTo");
-      router.urlService.setUrl("/foo");
+      router.urlService.url("/foo");
       router.urlRouter.sync();
       expect($state.transitionTo['calls'].mostRecent().args[0]).toBe(state.$$state());
 
diff --git a/test/stateServiceSpec.ts b/test/stateServiceSpec.ts
@@ -115,12 +115,12 @@ describe('stateService', function () {
     });
 
     it("should not update the URL in response to synchronizing URL", ((done) => {
-      $loc.setUrl('/a/b/c');
-      var setUrl = spyOn($loc, 'setUrl').and.callThrough();
+      $loc.url('/a/b/c');
+      var url = spyOn($loc, 'url').and.callThrough();
 
       wait().then(() => {
         expect($state.current.name).toBe('C');
-        let pushedUrls = setUrl.calls.all().map(x => x.args[0]).filter(x => x !== undefined);
+        let pushedUrls = url.calls.all().map(x => x.args[0]).filter(x => x !== undefined);
         expect(pushedUrls).toEqual([]);
         expect($loc.path()).toBe('/a/b/c');
         done();
@@ -130,12 +130,12 @@ describe('stateService', function () {
     it("should update the URL in response to synchronizing URL then redirecting", ((done) => {
       $transitions.onStart({ to: 'C' }, () => $state.target('D'));
 
-      $loc.setUrl('/a/b/c');
-      var setUrl = spyOn($loc, 'setUrl').and.callThrough();
+      $loc.url('/a/b/c');
+      var url = spyOn($loc, 'url').and.callThrough();
 
       wait().then(() => {
         expect($state.current.name).toBe('D');
-        let pushedUrls = setUrl.calls.all().map(x => x.args[0]).filter(x => x !== undefined);
+        let pushedUrls = url.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();
@@ -176,7 +176,7 @@ describe('stateService', function () {
       var dynamicstate, childWithParam, childNoParam;
 
       beforeEach(async function (done) {
-        $loc.setUrl("asdfasfdasf");
+        $loc.url("asdfasfdasf");
         dynlog = paramsChangedLog = "";
         dynamicstate = {
           name: 'dyn',
@@ -368,7 +368,7 @@ describe('stateService', function () {
             done();
           });
 
-          $loc.setUrl('/dynstate/p1/pd1?search=s1&searchDyn=sd2');
+          $loc.url('/dynstate/p1/pd1?search=s1&searchDyn=sd2');
         });
 
         it('exits and enters a state when any non-dynamic params change (triggered via url)', (done) => {
@@ -377,7 +377,7 @@ describe('stateService', function () {
             done();
           });
 
-          $loc.setUrl('/dynstate/p1/pd1?search=s2&searchDyn=sd2');
+          $loc.url('/dynstate/p1/pd1?search=s2&searchDyn=sd2');
         });
 
         it('does not exit nor enter a state when only dynamic params change (triggered via $state transition)', async (done) => {
@@ -410,7 +410,7 @@ describe('stateService', function () {
             }
           });
 
-          $loc.setUrl('/dynstate/p1/pd1?search=s1&searchDyn=sd2'); // {search: 's1', searchDyn: 'sd2'});
+          $loc.url('/dynstate/p1/pd1?search=s1&searchDyn=sd2'); // {search: 's1', searchDyn: 'sd2'});
         });
 
         it('updates $stateParams and $location.search when only dynamic params change (triggered via $state transition)', async (done) => {
@@ -443,7 +443,7 @@ describe('stateService', function () {
         });
 
         it('doesn\'t re-enter state (triggered by url change)', function (done) {
-          $loc.setUrl($loc.path() + "?term=hello");
+          $loc.url($loc.path() + "?term=hello");
           awaitTransition(router).then(() => {
             expect($loc.search()).toEqual({term: 'hello'});
             expect(entered).toBeFalsy();
diff --git a/test/urlMatcherFactorySpec.ts b/test/urlMatcherFactorySpec.ts
diff --git a/test/urlRouterSpec.ts b/test/urlRouterSpec.ts

Original file line number	Diff line number	Diff line change
`@@ -81,7 +81,7 @@ export class UrlRouter implements Disposable {`
`81`	`81`	`if (!match) return false;`
`82`	`82`
`83`	`83`	`let result = rule.handler(match, path, search, hash);`
`84`		`- if (isString(result)) $url.setUrl(result, true);`
	`84`	`+ if (isString(result)) $url.url(result, true);`
`85`	`85`	`return true;`
`86`	`86`	`}`
`87`	`87`
`@@ -118,7 +118,7 @@ export class UrlRouter implements Disposable {`
`118`	`118`	`}`
`119`	`119`	`if ($url.path() === this.location) return;`
`120`	`120`
`121`		`- $url.setUrl(this.location, true);`
	`121`	`+ $url.url(this.location, true);`
`122`	`122`	`}`
`123`	`123`
`124`	`124`	`/**`
`@@ -133,7 +133,7 @@ export class UrlRouter implements Disposable {`
`133`	`133`	`*/`
`134`	`134`	`push(urlMatcher: UrlMatcher, params?: RawParams, options?: { replace?: (string\|boolean) }) {`
`135`	`135`	`let replace = options && !!options.replace;`
`136`		`- this._router.urlService.setUrl(urlMatcher.format(params \|\| {}), replace);`
	`136`	`+ this._router.urlService.url(urlMatcher.format(params \|\| {}), replace);`
`137`	`137`	`}`
`138`	`138`
`139`	`139`	`/**`