Deduplicate m.route and m.redraw logic (#2453)

- Remove appropriate route change subcriptions when a root is removed
  via `m.mount(root, null)`.
- Don't pollute `onpopstate` and friends - use standard event listeners
  instead.
- Simplify and streamline subscriptions, in preparation of adding a
  `remove` parameter to `m.mount`.
- Change the redraw internals to redraw immediately, with ability to
  cancel via returning a sentinel.
- Change `"bleeding-edge"` for `m.version` in `next` to instead just be
  the latest `m.version`. (If you're using `next`, you should know what
  you're in for.)
- Update tests to be aware of these changes. (Some were failing for
  subtle reasons.)
- Drive-by: remove some uses of `string.charAt(n)` and use `string[n]`
  instead.

router/router.js

@@ -6,91 +6,107 @@ var compileTemplate = require("../pathname/compileTemplate")
 var assign = require("../pathname/assign")
 
 module.exports = function($window) {
-	var supportsPushState = typeof $window.history.pushState === "function"
 	var callAsync = typeof setImmediate === "function" ? setImmediate : setTimeout
+	var supportsPushState = typeof $window.history.pushState === "function"
+	var fireAsync
 
-	var asyncId
-	var router = {prefix: "#!"}
-	router.getPath = function() {
-		// Consider the pathname holistically. The prefix might even be invalid,
-		// but that's not our problem.
-		var prefix = $window.location.hash
-		if (router.prefix[0] !== "#") {
-			prefix = $window.location.search + prefix
-			if (router.prefix[0] !== "?") {
-				prefix = $window.location.pathname + prefix
-				if (prefix[0] !== "/") prefix = "/" + prefix
+	return {
+		prefix: "#!",
+
+		getPath: function() {
+			// Consider the pathname holistically. The prefix might even be invalid,
+			// but that's not our problem.
+			var prefix = $window.location.hash
+			if (this.prefix[0] !== "#") {
+				prefix = $window.location.search + prefix
+				if (this.prefix[0] !== "?") {
+					prefix = $window.location.pathname + prefix
+					if (prefix[0] !== "/") prefix = "/" + prefix
+				}
 			}
-		}
-		// This seemingly useless `.concat()` speeds up the tests quite a bit,
-		// since the representation is consistently a relatively poorly
-		// optimized cons string.
-		return prefix.concat()
-			.replace(/(?:%[a-f89][a-f0-9])+/gim, decodeURIComponent)
-			.slice(router.prefix.length)
-	}
+			// This seemingly useless `.concat()` speeds up the tests quite a bit,
+			// since the representation is consistently a relatively poorly
+			// optimized cons string.
+			return prefix.concat()
+				.replace(/(?:%[a-f89][a-f0-9])+/gim, decodeURIComponent)
+				.slice(this.prefix.length)
+		},
 
-	router.setPath = function(path, data, options) {
-		path = buildPathname(path, data)
-		if (supportsPushState) {
-			var state = options ? options.state : null
-			var title = options ? options.title : null
-			$window.onpopstate()
-			if (options && options.replace) $window.history.replaceState(state, title, router.prefix + path)
-			else $window.history.pushState(state, title, router.prefix + path)
-		}
-		else $window.location.href = router.prefix + path
-	}
-
-	router.defineRoutes = function(routes, resolve, reject, defaultRoute) {
-		var compiled = Object.keys(routes).map(function(route) {
-			if (route.charAt(0) !== "/") throw new SyntaxError("Routes must start with a `/`")
-			if ((/:([^\/\.-]+)(\.{3})?:/).test(route)) {
-				throw new SyntaxError("Route parameter names must be separated with either `/`, `.`, or `-`")
+		setPath: function(path, data, options) {
+			path = buildPathname(path, data)
+			if (fireAsync != null) {
+				fireAsync()
+				var state = options ? options.state : null
+				var title = options ? options.title : null
+				if (options && options.replace) $window.history.replaceState(state, title, this.prefix + path)
+				else $window.history.pushState(state, title, this.prefix + path)
 			}
-			return {
-				route: route,
-				component: routes[route],
-				check: compileTemplate(route),
+			else {
+				$window.location.href = this.prefix + path
 			}
-		})
+		},
 
-		if (defaultRoute != null) {
-			var defaultData = parsePathname(defaultRoute)
+		defineRoutes: function(routes, resolve, reject, defaultRoute, subscribe) {
+			var self = this
+			var compiled = Object.keys(routes).map(function(route) {
+				if (route[0] !== "/") throw new SyntaxError("Routes must start with a `/`")
+				if ((/:([^\/\.-]+)(\.{3})?:/).test(route)) {
+					throw new SyntaxError("Route parameter names must be separated with either `/`, `.`, or `-`")
+				}
+				return {
+					route: route,
+					component: routes[route],
+					check: compileTemplate(route),
+				}
+			})
+			var unsubscribe, asyncId
 
-			if (!compiled.some(function (i) { return i.check(defaultData) })) {
-				throw new ReferenceError("Default route doesn't match any known routes")
-			}
-		}
+			fireAsync = null
 
-		function resolveRoute() {
-			var path = router.getPath()
-			var data = parsePathname(path)
+			if (defaultRoute != null) {
+				var defaultData = parsePathname(defaultRoute)
 
-			assign(data.params, $window.history.state)
-
-			for (var i = 0; i < compiled.length; i++) {
-				if (compiled[i].check(data)) {
-					resolve(compiled[i].component, data.params, path, compiled[i].route)
-					return
+				if (!compiled.some(function (i) { return i.check(defaultData) })) {
+					throw new ReferenceError("Default route doesn't match any known routes")
 				}
 			}
 
-			reject(path, data.params)
-		}
+			function resolveRoute() {
+				var path = self.getPath()
+				var data = parsePathname(path)
 
-		if (supportsPushState) {
-			$window.onpopstate = function() {
-				if (asyncId) return
-				asyncId = callAsync(function() {
-					asyncId = null
-					resolveRoute()
-				})
+				assign(data.params, $window.history.state)
+
+				for (var i = 0; i < compiled.length; i++) {
+					if (compiled[i].check(data)) {
+						resolve(compiled[i].component, data.params, path, compiled[i].route)
+						return
+					}
+				}
+
+				reject(path, data.params)
 			}
-		}
-		else if (router.prefix.charAt(0) === "#") $window.onhashchange = resolveRoute
-		resolveRoute()
-	}
 
-	return router
+			if (supportsPushState) {
+				unsubscribe = function() {
+					$window.removeEventListener("popstate", fireAsync, false)
+				}
+				$window.addEventListener("popstate", fireAsync = function() {
+					if (asyncId) return
+					asyncId = callAsync(function() {
+						asyncId = null
+						resolveRoute()
+					})
+				}, false)
+			} else if (this.prefix[0] === "#") {
+				unsubscribe = function() {
+					$window.removeEventListener("hashchange", resolveRoute, false)
+				}
+				$window.addEventListener("hashchange", resolveRoute, false)
+			}
+
+			subscribe(unsubscribe)
+			resolveRoute()
+		},
+	}
 }

router/tests/test-defineRoutes.js

@@ -11,6 +11,10 @@ o.spec("Router.defineRoutes", function() {
 			o.spec("using prefix `" + prefix + "` starting on " + env.protocol + "//" + env.hostname, function() {
 				var $window, router, onRouteChange, onFail
 
+				function defineRoutes(routes, defaultRoute) {
+					router.defineRoutes(routes, onRouteChange, onFail, defaultRoute, function() {})
+				}
+
 				o.beforeEach(function() {
 					$window = pushStateMock(env)
 					router = new Router($window)
@@ -21,7 +25,10 @@ o.spec("Router.defineRoutes", function() {
 
 				o("calls onRouteChange on init", function(done) {
 					$window.location.href = prefix + "/a"
-					router.defineRoutes({"/a": {data: 1}}, onRouteChange, onFail)
+					var subscribe = o.spy()
+
+					router.defineRoutes({"/a": {data: 1}}, onRouteChange, onFail, null, subscribe)
+					o(subscribe.callCount).equals(1)
 
 					callAsync(function() {
 						o(onRouteChange.callCount).equals(1)
@@ -32,7 +39,7 @@ o.spec("Router.defineRoutes", function() {
 
 				o("resolves to route", function(done) {
 					$window.location.href = prefix + "/test"
-					router.defineRoutes({"/test": {data: 1}}, onRouteChange, onFail)
+					defineRoutes({"/test": {data: 1}})
 
 					callAsync(function() {
 						o(onRouteChange.callCount).equals(1)
@@ -45,7 +52,7 @@ o.spec("Router.defineRoutes", function() {
 
 				o("resolves to route w/ escaped unicode", function(done) {
 					$window.location.href = prefix + "/%C3%B6?%C3%B6=%C3%B6"
-					router.defineRoutes({"/ö": {data: 2}}, onRouteChange, onFail)
+					defineRoutes({"/ö": {data: 2}})
 
 					callAsync(function() {
 						o(onRouteChange.callCount).equals(1)
@@ -58,7 +65,7 @@ o.spec("Router.defineRoutes", function() {
 
 				o("resolves to route w/ unicode", function(done) {
 					$window.location.href = prefix + "/ö?ö=ö"
-					router.defineRoutes({"/ö": {data: 2}}, onRouteChange, onFail)
+					defineRoutes({"/ö": {data: 2}})
 
 					callAsync(function() {
 						o(onRouteChange.callCount).equals(1)
@@ -75,7 +82,7 @@ o.spec("Router.defineRoutes", function() {
 					router = new Router($window)
 					router.prefix = prefix
 
-					router.defineRoutes({"/test": {data: 1}}, onRouteChange, onFail)
+					defineRoutes({"/test": {data: 1}})
 
 					callAsync(function() {
 						o(onRouteChange.callCount).equals(1)
@@ -88,7 +95,7 @@ o.spec("Router.defineRoutes", function() {
 
 				o("handles parameterized route", function(done) {
 					$window.location.href = prefix + "/test/x"
-					router.defineRoutes({"/test/:a": {data: 1}}, onRouteChange, onFail)
+					defineRoutes({"/test/:a": {data: 1}})
 
 					callAsync(function() {
 						o(onRouteChange.callCount).equals(1)
@@ -101,7 +108,7 @@ o.spec("Router.defineRoutes", function() {
 
 				o("handles multi-parameterized route", function(done) {
 					$window.location.href = prefix + "/test/x/y"
-					router.defineRoutes({"/test/:a/:b": {data: 1}}, onRouteChange, onFail)
+					defineRoutes({"/test/:a/:b": {data: 1}})
 
 					callAsync(function() {
 						o(onRouteChange.callCount).equals(1)
@@ -114,7 +121,7 @@ o.spec("Router.defineRoutes", function() {
 
 				o("handles rest parameterized route", function(done) {
 					$window.location.href = prefix + "/test/x/y"
-					router.defineRoutes({"/test/:a...": {data: 1}}, onRouteChange, onFail)
+					defineRoutes({"/test/:a...": {data: 1}})
 
 					callAsync(function() {
 						o(onRouteChange.callCount).equals(1)
@@ -127,7 +134,7 @@ o.spec("Router.defineRoutes", function() {
 
 				o("handles route with search", function(done) {
 					$window.location.href = prefix + "/test?a=b&c=d"
-					router.defineRoutes({"/test": {data: 1}}, onRouteChange, onFail)
+					defineRoutes({"/test": {data: 1}})
 
 					callAsync(function() {
 						o(onRouteChange.callCount).equals(1)
@@ -140,7 +147,7 @@ o.spec("Router.defineRoutes", function() {
 
 				o("calls reject", function(done) {
 					$window.location.href = prefix + "/test"
-					router.defineRoutes({"/other": {data: 1}}, onRouteChange, onFail)
+					defineRoutes({"/other": {data: 1}})
 
 					callAsync(function() {
 						o(onFail.callCount).equals(1)
@@ -152,7 +159,7 @@ o.spec("Router.defineRoutes", function() {
 
 				o("handles out of order routes", function(done) {
 					$window.location.href = prefix + "/z/y/x"
-					router.defineRoutes({"/z/y/x": {data: 1}, "/:a...": {data: 2}}, onRouteChange, onFail)
+					defineRoutes({"/z/y/x": {data: 1}, "/:a...": {data: 2}})
 
 					callAsync(function() {
 						o(onRouteChange.callCount).equals(1)
@@ -164,7 +171,7 @@ o.spec("Router.defineRoutes", function() {
 
 				o("handles reverse out of order routes", function(done) {
 					$window.location.href = prefix + "/z/y/x"
-					router.defineRoutes({"/:a...": {data: 2}, "/z/y/x": {data: 1}}, onRouteChange, onFail)
+					defineRoutes({"/:a...": {data: 2}, "/z/y/x": {data: 1}})
 
 					callAsync(function() {
 						o(onRouteChange.callCount).equals(1)
@@ -180,7 +187,7 @@ o.spec("Router.defineRoutes", function() {
 					routes["/:a..."] = {data: 2}
 
 					$window.location.href = prefix + "/z/y/x"
-					router.defineRoutes(routes, onRouteChange, onFail)
+					defineRoutes(routes)
 
 					callAsync(function() {
 						o(onRouteChange.callCount).equals(1)
@@ -196,7 +203,7 @@ o.spec("Router.defineRoutes", function() {
 					routes["/z/y/x"] = {data: 1}
 
 					$window.location.href = prefix + "/z/y/x"
-					router.defineRoutes(routes, onRouteChange, onFail)
+					defineRoutes(routes)
 
 					callAsync(function() {
 						o(onRouteChange.callCount).equals(1)
@@ -211,7 +218,7 @@ o.spec("Router.defineRoutes", function() {
 					routes["/:a..."] = {data: 2}
 
 					$window.location.href = prefix + "/z/y/x"
-					router.defineRoutes(routes, onRouteChange, onFail)
+					defineRoutes(routes)
 
 					callAsync(function() {
 						o(onRouteChange.callCount).equals(1)
@@ -226,7 +233,7 @@ o.spec("Router.defineRoutes", function() {
 					routes["/z/y/x"] = {data: 12}
 
 					$window.location.href = prefix + "/z/y/x"
-					router.defineRoutes(routes, onRouteChange, onFail)
+					defineRoutes(routes)
 
 					callAsync(function() {
 						o(onRouteChange.callCount).equals(1)
@@ -238,7 +245,7 @@ o.spec("Router.defineRoutes", function() {
 
 				o("handles non-ascii routes", function(done) {
 					$window.location.href = prefix + "/ö"
-					router.defineRoutes({"/ö": "aaa"}, onRouteChange, onFail)
+					defineRoutes({"/ö": "aaa"})
 
 					callAsync(function() {
 						o(onRouteChange.callCount).equals(1)

router/tests/test-getPath.js

@@ -10,6 +10,10 @@ o.spec("Router.getPath", function() {
 			o.spec("using prefix `" + prefix + "` starting on " + env.protocol + "//" + env.hostname, function() {
 				var $window, router, onRouteChange, onFail
 
+				function defineRoutes(routes, defaultRoute) {
+					router.defineRoutes(routes, onRouteChange, onFail, defaultRoute, function() {})
+				}
+
 				o.beforeEach(function() {
 					$window = pushStateMock(env)
 					router = new Router($window)
@@ -20,25 +24,25 @@ o.spec("Router.getPath", function() {
 
 				o("gets route", function() {
 					$window.location.href = prefix + "/test"
-					router.defineRoutes({"/test": {data: 1}}, onRouteChange, onFail)
+					defineRoutes({"/test": {data: 1}})
 
 					o(router.getPath()).equals("/test")
 				})
 				o("gets route w/ params", function() {
 					$window.location.href = prefix + "/other/x/y/z?c=d#e=f"
-					router.defineRoutes({"/test": {data: 1}, "/other/:a/:b...": {data: 2}}, onRouteChange, onFail)
+					defineRoutes({"/test": {data: 1}, "/other/:a/:b...": {data: 2}})
 
 					o(router.getPath()).equals("/other/x/y/z?c=d#e=f")
 				})
 				o("gets route w/ escaped unicode", function() {
 					$window.location.href = prefix + "/%C3%B6?%C3%B6=%C3%B6#%C3%B6=%C3%B6"
-					router.defineRoutes({"/test": {data: 1}, "/ö/:a/:b...": {data: 2}}, onRouteChange, onFail)
+					defineRoutes({"/test": {data: 1}, "/ö/:a/:b...": {data: 2}})
 
 					o(router.getPath()).equals("/ö?ö=ö#ö=ö")
 				})
 				o("gets route w/ unicode", function() {
 					$window.location.href = prefix + "/ö?ö=ö#ö=ö"
-					router.defineRoutes({"/test": {data: 1}, "/ö/:a/:b...": {data: 2}}, onRouteChange, onFail)
+					defineRoutes({"/test": {data: 1}, "/ö/:a/:b...": {data: 2}})
 
 					o(router.getPath()).equals("/ö?ö=ö#ö=ö")
 				})

router/tests/test-setPath.js

@@ -11,6 +11,10 @@ o.spec("Router.setPath", function() {
 			o.spec("using prefix `" + prefix + "` starting on " + env.protocol + "//" + env.hostname, function() {
 				var $window, router, onRouteChange, onFail
 
+				function defineRoutes(routes, defaultRoute) {
+					router.defineRoutes(routes, onRouteChange, onFail, defaultRoute, function() {})
+				}
+
 				o.beforeEach(function() {
 					$window = pushStateMock(env)
 					router = new Router($window)
@@ -21,7 +25,7 @@ o.spec("Router.setPath", function() {
 
 				o("setPath calls onRouteChange asynchronously", function(done) {
 					$window.location.href = prefix + "/a"
-					router.defineRoutes({"/a": {data: 1}, "/b": {data: 2}}, onRouteChange, onFail)
+					defineRoutes({"/a": {data: 1}, "/b": {data: 2}})
 
 					callAsync(function() {
 						router.setPath("/b")
@@ -35,7 +39,7 @@ o.spec("Router.setPath", function() {
 				})
 				o("setPath calls onFail asynchronously", function(done) {
 					$window.location.href = prefix + "/a"
-					router.defineRoutes({"/a": {data: 1}, "/b": {data: 2}}, onRouteChange, onFail)
+					defineRoutes({"/a": {data: 1}, "/b": {data: 2}})
 
 					callAsync(function() {
 						router.setPath("/c")
@@ -49,7 +53,7 @@ o.spec("Router.setPath", function() {
 				})
 				o("sets route via API", function(done) {
 					$window.location.href = prefix + "/test"
-					router.defineRoutes({"/test": {data: 1}, "/other/:a/:b...": {data: 2}}, onRouteChange, onFail)
+					defineRoutes({"/test": {data: 1}, "/other/:a/:b...": {data: 2}})
 
 					callAsync(function() {
 						router.setPath("/other/x/y/z?c=d#e=f")
@@ -61,7 +65,7 @@ o.spec("Router.setPath", function() {
 				})
 				o("sets route w/ escaped unicode", function(done) {
 					$window.location.href = prefix + "/test"
-					router.defineRoutes({"/test": {data: 1}, "/ö/:a/:b...": {data: 2}}, onRouteChange, onFail)
+					defineRoutes({"/test": {data: 1}, "/ö/:a/:b...": {data: 2}})
 
 					callAsync(function() {
 						router.setPath("/%C3%B6?%C3%B6=%C3%B6#%C3%B6=%C3%B6")
@@ -73,7 +77,7 @@ o.spec("Router.setPath", function() {
 				})
 				o("sets route w/ unicode", function(done) {
 					$window.location.href = prefix + "/test"
-					router.defineRoutes({"/test": {data: 1}, "/ö/:a/:b...": {data: 2}}, onRouteChange, onFail)
+					defineRoutes({"/test": {data: 1}, "/ö/:a/:b...": {data: 2}})
 
 					callAsync(function() {
 						router.setPath("/ö?ö=ö#ö=ö")
@@ -90,7 +94,7 @@ o.spec("Router.setPath", function() {
 					router = new Router($window)
 					router.prefix = prefix
 
-					router.defineRoutes({"/test": {data: 1}, "/other/:a/:b...": {data: 2}}, onRouteChange, onFail)
+					defineRoutes({"/test": {data: 1}, "/other/:a/:b...": {data: 2}})
 
 					callAsync(function() {
 						router.setPath("/other/x/y/z?c=d#e=f")
@@ -102,7 +106,7 @@ o.spec("Router.setPath", function() {
 				})
 				o("sets route via pushState/onpopstate", function(done) {
 					$window.location.href = prefix + "/test"
-					router.defineRoutes({"/test": {data: 1}, "/other/:a/:b...": {data: 2}}, onRouteChange, onFail)
+					defineRoutes({"/test": {data: 1}, "/other/:a/:b...": {data: 2}})
 
 					callAsync(function() {
 						$window.history.pushState(null, null, prefix + "/other/x/y/z?c=d#e=f")
@@ -115,7 +119,7 @@ o.spec("Router.setPath", function() {
 				})
 				o("sets parameterized route", function(done) {
 					$window.location.href = prefix + "/test"
-					router.defineRoutes({"/test": {data: 1}, "/other/:a/:b...": {data: 2}}, onRouteChange, onFail)
+					defineRoutes({"/test": {data: 1}, "/other/:a/:b...": {data: 2}})
 
 					callAsync(function() {
 						router.setPath("/other/:a/:b", {a: "x", b: "y/z", c: "d", e: "f"})
@@ -127,7 +131,7 @@ o.spec("Router.setPath", function() {
 				})
 				o("replace:true works", function(done) {
 					$window.location.href = prefix + "/test"
-					router.defineRoutes({"/test": {data: 1}, "/other": {data: 2}}, onRouteChange, onFail)
+					defineRoutes({"/test": {data: 1}, "/other": {data: 2}})
 
 					callAsync(function() {
 						router.setPath("/other", null, {replace: true})
@@ -140,7 +144,7 @@ o.spec("Router.setPath", function() {
 				})
 				o("replace:false works", function(done) {
 					$window.location.href = prefix + "/test"
-					router.defineRoutes({"/test": {data: 1}, "/other": {data: 2}}, onRouteChange, onFail)
+					defineRoutes({"/test": {data: 1}, "/other": {data: 2}})
 
 					callAsync(function() {
 						router.setPath("/other", null, {replace: false})
@@ -155,7 +159,7 @@ o.spec("Router.setPath", function() {
 				})
 				o("state works", function(done) {
 					$window.location.href = prefix + "/test"
-					router.defineRoutes({"/test": {data: 1}, "/other": {data: 2}}, onRouteChange, onFail)
+					defineRoutes({"/test": {data: 1}, "/other": {data: 2}})
 
 					callAsync(function() {
 						router.setPath("/other", null, {state: {a: 1}})