fix($urlMatcherFactory): no longer generate unroutable urls

Previously, urlMatcherFactory would squash default param values and any double slashes from generated urls (by default).  This behaviour caused urls generated with default values in them to not route back to the state that generated them.  Change the default behaviour to squash only the param-value and not the URLs.

Closes #1487

feat($urlMatcherFactory): configurable url param squashing

Allow configuration of the squashing behavior of default-url-parameters on a per-param basis with reasonable default setting.

Author: christopherthielen

src/urlMatcherFactory.js

@@ -91,11 +91,15 @@ function UrlMatcher(pattern, config) {
     return params[id];
   }
 
-  function quoteRegExp(string, pattern, isOptional) {
-    var result = string.replace(/[\\\[\]\^$*+?.()|{}]/g, "\\$&");
+  function quoteRegExp(string, pattern, squashPolicy) {
+    var flags = ['',''], result = string.replace(/[\\\[\]\^$*+?.()|{}]/g, "\\$&");
     if (!pattern) return result;
-    var flag = isOptional ? '?' : '';
-    return result + flag + '(' + pattern + ')' + flag;
+    switch(squashPolicy) {
+      case "nosquash": flags = ['', ''];   break;
+      case "value":    flags = ['', '?'];  break;
+      case "slash":   flags = ['?', '?']; break;
+    }
+    return result + flags[0] + '(' + pattern + ')' + flags[1];
   }
 
   this.source = pattern;
@@ -122,7 +126,7 @@ function UrlMatcher(pattern, config) {
     if (p.segment.indexOf('?') >= 0) break; // we're into the search part
 
     param = addParameter(p.id, p.type, p.cfg);
-    compiled += quoteRegExp(p.segment, param.type.pattern.source, param.isOptional);
+    compiled += quoteRegExp(p.segment, param.type.pattern.source, param.squash);
     segments.push(p.segment);
     last = placeholder.lastIndex;
   }
@@ -290,37 +294,47 @@ UrlMatcher.prototype.validates = function (params) {
  */
 UrlMatcher.prototype.format = function (values) {
   var segments = this.segments, params = this.parameters();
-
-  if (!values) return segments.join('').replace('//', '/');
+  var paramset = this.params;
+  values = values || {};
 
   var nPath = segments.length - 1, nTotal = params.length,
-    result = segments[0], i, search, value, param, cfg, array;
+    result = segments[0], i, search, value, name, param, array, isDefaultValue;
 
   if (!this.validates(values)) return null;
 
   for (i = 0; i < nPath; i++) {
-    param = params[i];
-    value = values[param];
-    cfg   = this.params[param];
-
-    if (!isDefined(value) && (segments[i] === '/' && segments[i + 1] === '/')) continue;
-    if (value != null) result += encodeURIComponent(cfg.type.encode(value));
-    result += segments[i + 1];
+    name = params[i];
+    param = paramset[name];
+    value = param.value(values[name]);
+    isDefaultValue = param.isOptional && param.type.equals(param.value(), value);
+    var squash = isDefaultValue ? param.squash : "nosquash";
+    var encoded = param.type.encode(value);
+
+    var nextSegment = segments[i + 1];
+    if (squash === "nosquash") {
+      if (encoded != null) result += encodeURIComponent(encoded);
+      result += nextSegment;
+    } else if (squash === "value") {
+      result += nextSegment;
+    } else if (squash === "slash") {
+      var capture = result.match(/\/$/) ? /\/?(.*)/ : /(.*)/;
+      result += nextSegment.match(capture)[1];
+    }
   }
 
   for (/**/; i < nTotal; i++) {
-    param = params[i];
-    value = values[param];
+    name = params[i];
+    value = values[name];
     if (value == null) continue;
     array = isArray(value);
 
     if (array) {
-      value = value.map(encodeURIComponent).join('&' + param + '=');
+      value = value.map(encodeURIComponent).join('&' + name + '=');
     }
-    result += (search ? '&' : '?') + param + '=' + (array ? value : encodeURIComponent(value));
+    result += (search ? '&' : '?') + name + '=' + (array ? value : encodeURIComponent(value));
     search = true;
   }
-  return result.replace('//', '/');
+  return result;
 };
 
 /**
@@ -446,9 +460,9 @@ Type.prototype.pattern = /.*/;
 function $UrlMatcherFactory() {
   $$UMFP = this;
 
-  var isCaseInsensitive = false, isStrictMode = true;
+  var isCaseInsensitive = false, isStrictMode = true, defaultSquashPolicy = "nosquash";
 
-  function safeString(val) { return isDefined(val) ? val.toString() : val; }
+  function safeString(val) { return val != null ? val.toString() : val; }
   function coerceEquals(left, right) { return left == right; }
   function angularEquals(left, right) { return angular.equals(left, right); }
 // TODO:  function regexpMatches(val) { return isDefined(val) && this.pattern.test(val); }
@@ -569,6 +583,28 @@ function $UrlMatcherFactory() {
     isStrictMode = value;
   };
 
+  /**
+   * @ngdoc function
+   * @name ui.router.util.$urlMatcherFactory#defaultSquashPolicy
+   * @methodOf ui.router.util.$urlMatcherFactory
+   *
+   * @description
+   * Sets the default behavior when generating or matching URLs with default parameter values.
+   *
+   * @param {string} value A string that defines the default parameter URL squashing behavior.
+   *    `nosquash`: When generating an href with a default parameter value, do not squash the parameter value from the URL
+   *    `value`: When generating an href with a default parameter value, squash (remove) the parameter value from the URL
+   *    `slash`: When generating an href with a default parameter value, squash (remove) the parameter value, and, if the
+   *             parameter is surrounded by slashes, squash (remove) one slash from the URL
+   */
+  this.defaultSquashPolicy = function(value) {
+    if (!value) return defaultSquashPolicy;
+    if (value !== "nosquash" && value !== "value" && value !== "slash")
+      throw new Error("Invalid squash policy: " + value + ". Valid policies: 'nosquash', 'value', 'slash'");
+    defaultSquashPolicy = value;
+    return value;
+  };
+
   /**
    * @ngdoc function
    * @name ui.router.util.$urlMatcherFactory#compile
@@ -758,10 +794,12 @@ function $UrlMatcherFactory() {
     var defaultValueConfig = getDefaultValueConfig(config);
     config = config || {};
     type = getType(config, type);
+    var isOptional = defaultValueConfig.value !== undefined;
+    var squash = getSquashPolicy(config, isOptional);
 
     function getDefaultValueConfig(config) {
       var keys = isObject(config) ? objectKeys(config) : [];
-      var isShorthand = keys.indexOf("value") === -1 && keys.indexOf("type") === -1;
+      var isShorthand = keys.indexOf("value") === -1 && keys.indexOf("type") === -1 && keys.indexOf("squash") === -1;
       var configValue = isShorthand ? config : config.value;
       return {
         fn: isInjectable(configValue) ? configValue : function () { return configValue; },
@@ -776,6 +814,19 @@ function $UrlMatcherFactory() {
       return config.type instanceof Type ? config.type : new Type(config.type);
     }
 
+    /**
+     * returns "nosquash", "value", "slash" to indicate the "default parameter url squash policy".
+     * undefined aliases to urlMatcherFactory default. `false` aliases to "nosquash". `true` aliases to "slash".
+     */
+    function getSquashPolicy(config, isOptional) {
+      var squash = config.squash;
+      if (!isOptional || squash === false) return "nosquash";
+      if (!isDefined(squash)) return defaultSquashPolicy;
+      if (squash === true) return "slash";
+      if (squash === "nosquash" || squash === "value" || squash === "slash") return squash;
+      throw new Error("Invalid squash policy: '" + squash + "'. Valid policies: 'nosquash' (false), 'value', 'slash' (true)");
+    }
+
     /**
      * [Internal] Get the default value of a parameter, which may be an injectable function.
      */
@@ -796,8 +847,9 @@ function $UrlMatcherFactory() {
       id: id,
       type: type,
       config: config,
+      squash: squash,
       dynamic: undefined,
-      isOptional: defaultValueConfig.value !== undefined,
+      isOptional: isOptional,
       value: $value
     });
   };

test/urlMatcherFactorySpec.js

@@ -280,7 +280,7 @@ describe("urlMatcherFactory", function () {
   describe("optional parameters", function() {
     it("should match with or without values", function () {
       var m = new UrlMatcher('/users/{id:int}', {
-        params: { id: { value: null } }
+        params: { id: { value: null, squash: true } }
       });
       expect(m.exec('/users/1138')).toEqual({ id: 1138 });
       expect(m.exec('/users/').id).toBeNull();
@@ -289,7 +289,7 @@ describe("urlMatcherFactory", function () {
 
     it("should correctly match multiple", function() {
       var m = new UrlMatcher('/users/{id:int}/{state:[A-Z]+}', {
-        params: { id: { value: null }, state: { value: null } }
+        params: { id: { value: null, squash: true }, state: { value: null, squash: true } }
       });
       expect(m.exec('/users/1138')).toEqual({ id: 1138, state: null });
       expect(m.exec('/users/1138/NY')).toEqual({ id: 1138, state: "NY" });
@@ -314,7 +314,7 @@ describe("urlMatcherFactory", function () {
 
     it("should correctly format multiple", function() {
       var m = new UrlMatcher('/users/{id:int}/{state:[A-Z]+}', {
-        params: { id: { value: null }, state: { value: null } }
+        params: { id: { value: null, squash: true }, state: { value: null, squash: true } }
       });
 
       expect(m.format()).toBe("/users/");
@@ -325,7 +325,7 @@ describe("urlMatcherFactory", function () {
 
     it("should match in between static segments", function() {
       var m = new UrlMatcher('/users/{user:int}/photos', {
-        params: { user: 5 }
+        params: { user: { value: 5, squash: true } }
       });
       expect(m.exec('/users/photos').user).toBe(5);
       expect(m.exec('/users/6/photos').user).toBe(6);
@@ -334,20 +334,20 @@ describe("urlMatcherFactory", function () {
     });
 
     it("should correctly format with an optional followed by a required parameter", function() {
-      var m = new UrlMatcher('/:user/gallery/photos/:photo', {
+      var m = new UrlMatcher('/home/:user/gallery/photos/:photo', {
         params: { 
-          user: {value: null},
-          photo: {} 
+          user: {value: null, squash: true},
+          photo: undefined
         }
       });
-      expect(m.format({ photo: 12 })).toBe("/gallery/photos/12");
-      expect(m.format({ user: 1138, photo: 13 })).toBe("/1138/gallery/photos/13");
+      expect(m.format({ photo: 12 })).toBe("/home/gallery/photos/12");
+      expect(m.format({ user: 1138, photo: 13 })).toBe("/home/1138/gallery/photos/13");
     });
 
     describe("default values", function() {
       it("should populate if not supplied in URL", function() {
         var m = new UrlMatcher('/users/{id:int}/{test}', {
-          params: { id: { value: 0 }, test: { value: "foo" } }
+          params: { id: { value: 0, squash: true }, test: { value: "foo", squash: true } }
         });
         expect(m.exec('/users')).toEqual({ id: 0, test: "foo" });
         expect(m.exec('/users/2')).toEqual({ id: 2, test: "foo" });
@@ -360,7 +360,7 @@ describe("urlMatcherFactory", function () {
         var m = new UrlMatcher('/foo/:foo', {
           params: { foo: "bar" }
         });
-        expect(m.exec("/foo")).toEqual({ foo: "bar" });
+        expect(m.exec("/foo/")).toEqual({ foo: "bar" });
       });
 
       it("should populate query params", function() {
@@ -372,21 +372,19 @@ describe("urlMatcherFactory", function () {
       });
 
       it("should allow function-calculated values", function() {
+        function barFn() { return "Value from bar()"; }
         var m = new UrlMatcher('/foo/:bar', {
-          params: {
-            bar: function() {
-              return "Value from bar()";
-            }
-          }
+          params: { bar: barFn }
+        });
+        expect(m.exec('/foo/').bar).toBe("Value from bar()");
+
+        m = new UrlMatcher('/foo/:bar', {
+          params: { bar: { value: barFn, squash: true } }
         });
         expect(m.exec('/foo').bar).toBe("Value from bar()");
 
-        var m = new UrlMatcher('/foo?bar', {
-          params: {
-            bar: function() {
-              return "Value from bar()";
-            }
-          }
+        m = new UrlMatcher('/foo?bar', {
+          params: { bar: barFn }
         });
         expect(m.exec('/foo').bar).toBe("Value from bar()");
       });
@@ -402,7 +400,7 @@ describe("urlMatcherFactory", function () {
         var user = { name: "Bob" };
 
         $stateParams.user = user;
-        expect(m.exec('/users').user).toBe(user);
+        expect(m.exec('/users/').user).toBe(user);
       }));
     });
   });