From f9b358331e3276c47634bffc2d39adb3e794466f Mon Sep 17 00:00:00 2001 From: Pierre-Yves Gerardy Date: Thu, 1 Sep 2016 20:17:05 +0200 Subject: [PATCH 01/10] White space cleanup --- api/tests/test-router.js | 162 +++++++++++++++++++-------------------- 1 file changed, 81 insertions(+), 81 deletions(-) diff --git a/api/tests/test-router.js b/api/tests/test-router.js index d62c28b9..77098b9d 100644 --- a/api/tests/test-router.js +++ b/api/tests/test-router.js @@ -38,7 +38,7 @@ o.spec("route", function() { callAsync(function() { o(root.firstChild.nodeName).equals("DIV") - + done() }) }) @@ -55,11 +55,11 @@ o.spec("route", function() { setTimeout(function() { o(root.firstChild.nodeName).equals("DIV") - + $window.history.back() - + o($window.location.pathname).equals("/") - + done() }, FRAME_BUDGET) }) @@ -77,7 +77,7 @@ o.spec("route", function() { function init(vnode) { o(vnode.attrs.foo).equals(undefined) - + done() } }) @@ -219,11 +219,11 @@ o.spec("route", function() { root.firstChild.dispatchEvent(e) o($window.location.href).equals(env.protocol + "//" + (env.hostname === "/" ? "" : env.hostname) + slash + (prefix ? prefix + "/" : "") + "test") - + done() }) }) - + o("accepts RouteResolver", function(done) { var matchCount = 0 var renderCount = 0 @@ -232,37 +232,37 @@ o.spec("route", function() { return m("div") } } - + $window.location.href = prefix + "/" route(root, "/abc", { "/:id" : { onmatch: function(vnode, resolve) { matchCount++ - + o(vnode.attrs.id).equals("abc") o(route.get()).equals("/abc") - + resolve(Component) }, render: function(vnode) { renderCount++ - + o(vnode.attrs.id).equals("abc") - + return vnode }, }, }) - + setTimeout(function() { o(matchCount).equals(1) o(renderCount).equals(1) o(root.firstChild.nodeName).equals("DIV") - + done() }, FRAME_BUDGET) }) - + o("accepts RouteResolver without `render` method as payload", function(done) { var matchCount = 0 var Component = { @@ -270,30 +270,30 @@ o.spec("route", function() { return m("div") } } - + $window.location.href = prefix + "/" route(root, "/abc", { "/:id" : { onmatch: function(vnode, resolve) { matchCount++ - + o(vnode.attrs.id).equals("abc") o(route.get()).equals("/abc") - + resolve(Component) }, }, }) - + setTimeout(function() { o(matchCount).equals(1) - + o(root.firstChild.nodeName).equals("DIV") - + done() }, FRAME_BUDGET) }) - + o("accepts RouteResolver without `onmatch` method as payload", function(done) { var renderCount = 0 var Component = { @@ -301,37 +301,37 @@ o.spec("route", function() { return m("div") } } - + $window.location.href = prefix + "/" route(root, "/abc", { "/:id" : { render: function(vnode) { renderCount++ - + o(vnode.attrs.id).equals("abc") - + return m(Component) }, }, }) - + setTimeout(function() { o(root.firstChild.nodeName).equals("DIV") - + done() }, FRAME_BUDGET) }) - + o("RouteResolver `render` does not have component semantics", function(done, timeout) { timeout(60) - + var renderCount = 0 var A = { view: function() { return m("div") } } - + $window.location.href = prefix + "/" route(root, "/a", { "/a" : { @@ -345,16 +345,16 @@ o.spec("route", function() { }, }, }) - + setTimeout(function() { var dom = root.firstChild o(root.firstChild.nodeName).equals("DIV") - + route.set("/b") - + setTimeout(function() { o(root.firstChild).equals(dom) - + done() }, FRAME_BUDGET) }, FRAME_BUDGET) @@ -368,7 +368,7 @@ o.spec("route", function() { return m("div") } } - + $window.location.href = prefix + "/" route(root, "/", { "/" : { @@ -386,66 +386,66 @@ o.spec("route", function() { callAsync(function() { o(matchCount).equals(1) o(renderCount).equals(1) - + redraw.publish() setTimeout(function() { o(matchCount).equals(1) o(renderCount).equals(2) - + done() }, FRAME_BUDGET) }) }) - + o("onmatch can redirect to another route", function(done) { - var redirected = false + var redirected = false - $window.location.href = prefix + "/" - route(root, "/a", { - "/a" : { - onmatch: function() { - route.set("/b") - } - }, - "/b" : { - view: function(vnode){ - redirected = true - } - } - }) + $window.location.href = prefix + "/" + route(root, "/a", { + "/a" : { + onmatch: function() { + route.set("/b") + } + }, + "/b" : { + view: function(vnode){ + redirected = true + } + } + }) - setTimeout(function() { - o(redirected).equals(true) + setTimeout(function() { + o(redirected).equals(true) + + done() + }, FRAME_BUDGET) + }) - done() - }, FRAME_BUDGET) - }) - o("onmatch can redirect to another route that has RouteResolver", function(done) { - var redirected = false + var redirected = false - $window.location.href = prefix + "/" - route(root, "/a", { - "/a" : { - onmatch: function() { - route.set("/b") - } - }, - "/b" : { - render: function(vnode){ - redirected = true - } - } - }) + $window.location.href = prefix + "/" + route(root, "/a", { + "/a" : { + onmatch: function() { + route.set("/b") + } + }, + "/b" : { + render: function(vnode){ + redirected = true + } + } + }) - setTimeout(function() { - o(redirected).equals(true) + setTimeout(function() { + o(redirected).equals(true) + + done() + }, FRAME_BUDGET) + }) - done() - }, FRAME_BUDGET) - }) - o("onmatch resolution callback resolves at most once", function(done) { var resolveCount = 0 var resolvedComponent @@ -474,10 +474,10 @@ o.spec("route", function() { done() }, FRAME_BUDGET) }) - + o("calling route.set invalidates pending onmatch resolution", function(done, timeout) { timeout(100) - + var resolved $window.location.href = prefix + "/" route(root, "/a", { @@ -493,7 +493,7 @@ o.spec("route", function() { }) setTimeout(function() { route.set("/b") - + setTimeout(function() { o(resolved).equals("b") From 2e3a610a7816570397e9a2a368ba7c80afc8da92 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Gerardy Date: Fri, 2 Sep 2016 21:56:58 +0200 Subject: [PATCH 02/10] Tests for the bugs fixed by the mount-based router --- api/tests/test-router.js | 108 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 108 insertions(+) diff --git a/api/tests/test-router.js b/api/tests/test-router.js index 77098b9d..66275448 100644 --- a/api/tests/test-router.js +++ b/api/tests/test-router.js @@ -475,6 +475,114 @@ o.spec("route", function() { }, FRAME_BUDGET) }) + o("the previous view redraws while onmatch resolution is pending (#1268)", function(done, timeout) { + timeout(FRAME_BUDGET * 5) + var view = o.spy() + var onmatch = o.spy() + + $window.location.href = prefix + "/" + route(root, "/", { + "/": {view: view}, + "/2": {onmatch: onmatch} + }) + + setTimeout(function() { + o(view.callCount).equals(1) + o(onmatch.callCount).equals(0) + + route.set("/2") + + setTimeout(function(){ + o(view.callCount).equals(1) + o(onmatch.callCount).equals(1) + + redraw.publish() + + setTimeout(function() { + o(view.callCount).equals(2) + o(onmatch.callCount).equals(1) + + done() + }, FRAME_BUDGET) + }, FRAME_BUDGET) + }, FRAME_BUDGET) + }) + + o("routed mount points can redraw synchronoulsy (#1275)", function(done) { + var view = o.spy() + + $window.location.href = prefix + "/" + route(root, "/", {"/":{view:view}}) + + setTimeout(function() { + o(view.callCount).equals(1) + + redraw.publish(true) + + o(view.callCount).equals(2) + + done() + }, FRAME_BUDGET) + }) + + o("m.route.set(m.route.get()) re-runs the resolution logic (#1180)", function(done, timeout){ + timeout(FRAME_BUDGET * 3) + + var onmatch = o.spy(function(vnode, resolve){resolve()}) + + $window.location.href = prefix + "/" + route(root, '/', { + "/":{ + onmatch: onmatch, + render: function(){return m("div")} + } + }) + + setTimeout(function() { + o(onmatch.callCount).equals(1) + + route.set(route.get()) + + setTimeout(function() { + o(onmatch.callCount).equals(2) + + done() + }, FRAME_BUDGET) + }, FRAME_BUDGET) + }) + + o("routing with RouteResolver works more than once (#1286)", function(done, timeout){ + timeout(FRAME_BUDGET * 4) + + $window.location.href = prefix + "/a" + route(root, '/a', { + '/a': { + render: function() { + return m("a", "a") + } + }, + '/b': { + render: function() { + return m("b", "b") + } + } + }) + + setTimeout(function(){ + route.set('/b') + + setTimeout(function(){ + route.set('/a') + + setTimeout(function(){ + o(root.firstChild.nodeName).equals("A") + + done() + }, FRAME_BUDGET) + }, FRAME_BUDGET) + }, FRAME_BUDGET) + }) + o("calling route.set invalidates pending onmatch resolution", function(done, timeout) { timeout(100) From 09578197a193c956e0c47865bafb2b92a33b9905 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Gerardy Date: Fri, 2 Sep 2016 21:59:05 +0200 Subject: [PATCH 03/10] Mount-based api/router.js Fix #1180 Fix #1268 Fix #1275 Fix #1286 --- api/router.js | 56 ++++++++++++++++++++++++++-------------- api/tests/test-router.js | 6 +++-- route.js | 5 ++-- 3 files changed, 43 insertions(+), 24 deletions(-) diff --git a/api/router.js b/api/router.js index 92731e77..b1226d55 100644 --- a/api/router.js +++ b/api/router.js @@ -2,38 +2,56 @@ var Vnode = require("../render/vnode") var coreRouter = require("../router/router") -var autoredraw = require("../api/autoredraw") -module.exports = function($window, renderer, pubsub) { +module.exports = function($window, mount) { var router = coreRouter($window) + var globalId, currentComponent, currentRender, currentArgs + + var RouteComponent = {view: function() { + return currentRender(Vnode(currentComponent, null, currentArgs, undefined, undefined, undefined)) + }} + function defaultRender(vnode){ + return vnode + } var route = function(root, defaultRoute, routes) { - var current = {path: null, component: "div", resolver: null}, currentResolutionIdentifier = null - var replay = router.defineRoutes(routes, function(payload, args, path, route) { - var resolutionIdentifier = currentResolutionIdentifier = {} - function resolve(component) { - if (resolutionIdentifier !== currentResolutionIdentifier) return - resolutionIdentifier = null - current.path = path, current.component = component - renderer.render(root, payload.render(Vnode(component, null, args, undefined, undefined, undefined))) + currentComponent = "div" + currentRender = defaultRender + currentArgs = null + + mount(root, RouteComponent) + + router.defineRoutes(routes, function(payload, args, path, route) { + var resolutionIdentifier = globalId = {} + var isResolver = typeof payload.view !== "function" + var render = defaultRender + + function resolve (component) { + if (resolutionIdentifier !== globalId) return + globalId = null + + currentComponent = component != null ? component: isResolver ? "div" : payload + currentRender = render + currentArgs = args + + root.redraw(true) } - if (typeof payload.view !== "function") { - if (typeof payload.render !== "function") payload.render = function(vnode) {return vnode} - if (typeof payload.onmatch !== "function") payload.onmatch = function() {resolve(current.component)} - if (path !== current.path) payload.onmatch(Vnode(payload, null, args, undefined, undefined, undefined), resolve) - else resolve(current.component) + function onmatch() { + resolve() } - else { - renderer.render(root, Vnode(payload, null, args, undefined, undefined, undefined)) + if (isResolver) { + if (typeof payload.render === "function") render = payload.render.bind(payload) + if (typeof payload.onmatch === "function") onmatch = payload.onmatch } + + onmatch.call(payload, {attrs: args}, resolve) }, function() { router.setPath(defaultRoute, null, {replace: true}) }) - autoredraw(root, renderer, pubsub, replay) } route.link = router.link route.prefix = router.setPrefix route.set = router.setPath route.get = router.getPath - + return route } diff --git a/api/tests/test-router.js b/api/tests/test-router.js index 66275448..d3829bb5 100644 --- a/api/tests/test-router.js +++ b/api/tests/test-router.js @@ -8,13 +8,14 @@ var m = require("../../render/hyperscript") var coreRenderer = require("../../render/render") var apiPubSub = require("../../api/pubsub") var apiRouter = require("../../api/router") +var apiMounter = require("../../api/mount") o.spec("route", function() { void [{protocol: "http:", hostname: "localhost"}, {protocol: "file:", hostname: "/"}].forEach(function(env) { void ["#", "?", "", "#!", "?!", "/foo"].forEach(function(prefix) { o.spec("using prefix `" + prefix + "` starting on " + env.protocol + "//" + env.hostname, function() { var FRAME_BUDGET = Math.floor(1000 / 60) - var $window, root, redraw, route + var $window, root, redraw, mount, route o.beforeEach(function() { $window = browserMock(env) @@ -22,7 +23,8 @@ o.spec("route", function() { root = $window.document.body redraw = apiPubSub() - route = apiRouter($window, coreRenderer($window), redraw) + mount = apiMounter(coreRenderer($window), redraw) + route = apiRouter($window, mount) route.prefix(prefix) }) diff --git a/route.js b/route.js index 94c7eec8..d41c606b 100644 --- a/route.js +++ b/route.js @@ -1,4 +1,3 @@ -var renderService = require("./render") -var redrawService = require("./redraw") +var mount = require("./mount") -module.exports = require("./api/router")(window, renderService, redrawService) \ No newline at end of file +module.exports = require("./api/router")(window, mount) \ No newline at end of file From 419897f72c7d49cd6a98af7af9afa9812ca13995 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Gerardy Date: Fri, 2 Sep 2016 22:00:59 +0200 Subject: [PATCH 04/10] Add test for #1276 --- api/tests/test-router.js | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/api/tests/test-router.js b/api/tests/test-router.js index d3829bb5..85f977e6 100644 --- a/api/tests/test-router.js +++ b/api/tests/test-router.js @@ -553,6 +553,26 @@ o.spec("route", function() { }, FRAME_BUDGET) }) + o("m.route.get() returns the last fully resolved route (#1276)", function(done){ + $window.location.href = prefix + "/" + + route(root, "/", { + "/": {view: function(){}}, + "/2": {onmatch: function(){}} + }) + + + setTimeout(function() { + o(route.get()).equals("/") + + route.set("/2") + + o(route.get()).equals("/") + + done() + }, FRAME_BUDGET) + }) + o("routing with RouteResolver works more than once (#1286)", function(done, timeout){ timeout(FRAME_BUDGET * 4) From a34685d7a4316dd94b3afa9b67f6e6e668e272f5 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Gerardy Date: Fri, 2 Sep 2016 22:03:27 +0200 Subject: [PATCH 05/10] m.route.get() returns the last fully resolved route (fix #1276), change RouteResolver.onmatch() signature according to #1277 --- api/router.js | 9 +++++---- api/tests/test-router.js | 20 ++++++++++---------- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/api/router.js b/api/router.js index b1226d55..d8496c30 100644 --- a/api/router.js +++ b/api/router.js @@ -5,7 +5,7 @@ var coreRouter = require("../router/router") module.exports = function($window, mount) { var router = coreRouter($window) - var globalId, currentComponent, currentRender, currentArgs + var globalId, currentComponent, currentRender, currentArgs, currentPath var RouteComponent = {view: function() { return currentRender(Vnode(currentComponent, null, currentArgs, undefined, undefined, undefined)) @@ -32,6 +32,7 @@ module.exports = function($window, mount) { currentComponent = component != null ? component: isResolver ? "div" : payload currentRender = render currentArgs = args + currentPath = path root.redraw(true) } @@ -42,8 +43,8 @@ module.exports = function($window, mount) { if (typeof payload.render === "function") render = payload.render.bind(payload) if (typeof payload.onmatch === "function") onmatch = payload.onmatch } - - onmatch.call(payload, {attrs: args}, resolve) + + onmatch.call(payload, resolve, args, path) }, function() { router.setPath(defaultRoute, null, {replace: true}) }) @@ -51,7 +52,7 @@ module.exports = function($window, mount) { route.link = router.link route.prefix = router.setPrefix route.set = router.setPath - route.get = router.getPath + route.get = function() {return currentPath} return route } diff --git a/api/tests/test-router.js b/api/tests/test-router.js index 85f977e6..43db3493 100644 --- a/api/tests/test-router.js +++ b/api/tests/test-router.js @@ -238,11 +238,11 @@ o.spec("route", function() { $window.location.href = prefix + "/" route(root, "/abc", { "/:id" : { - onmatch: function(vnode, resolve) { + onmatch: function(resolve, args, requestedPath) { matchCount++ - o(vnode.attrs.id).equals("abc") - o(route.get()).equals("/abc") + o(args.id).equals("abc") + o(requestedPath).equals("/abc") resolve(Component) }, @@ -276,11 +276,11 @@ o.spec("route", function() { $window.location.href = prefix + "/" route(root, "/abc", { "/:id" : { - onmatch: function(vnode, resolve) { + onmatch: function(resolve, args, requestedPath) { matchCount++ - o(vnode.attrs.id).equals("abc") - o(route.get()).equals("/abc") + o(args.id).equals("abc") + o(requestedPath).equals("/abc") resolve(Component) }, @@ -374,7 +374,7 @@ o.spec("route", function() { $window.location.href = prefix + "/" route(root, "/", { "/" : { - onmatch: function(vnode, resolve) { + onmatch: function(resolve) { matchCount++ resolve(Component) }, @@ -458,7 +458,7 @@ o.spec("route", function() { $window.location.href = prefix + "/" route(root, "/", { "/": { - onmatch: function(vnode, resolve) { + onmatch: function(resolve) { resolve(A) resolve(B) callAsync(function() {resolve(C)}) @@ -530,7 +530,7 @@ o.spec("route", function() { o("m.route.set(m.route.get()) re-runs the resolution logic (#1180)", function(done, timeout){ timeout(FRAME_BUDGET * 3) - var onmatch = o.spy(function(vnode, resolve){resolve()}) + var onmatch = o.spy(function(resolve){resolve()}) $window.location.href = prefix + "/" route(root, '/', { @@ -612,7 +612,7 @@ o.spec("route", function() { $window.location.href = prefix + "/" route(root, "/a", { "/a": { - onmatch: function(vnode, resolve) { + onmatch: function(resolve) { setTimeout(resolve, 20) }, render: function(vnode) {resolved = "a"} From 2e6a2ae5d9b875c63f9a5abf3b6f2d25ffd5b4e9 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Gerardy Date: Fri, 2 Sep 2016 22:05:58 +0200 Subject: [PATCH 06/10] Docs for the #1276 fix --- docs/route.md | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/docs/route.md b/docs/route.md index 4931c07c..e60f2d3e 100644 --- a/docs/route.md +++ b/docs/route.md @@ -52,13 +52,13 @@ Argument | Type | Required | Description ##### route.get -Returns the current routing path, without the prefix. +Returns the last fully resolved routing path, without the prefix. It may differ from the path displayed in the location bar while an asynchronous route is [pending resolution](#code-splitting). `path = m.route.get()` Argument | Type | Required | Description ----------------- | --------- | -------- | --- -**returns** | String | | Returns the current path +**returns** | String | | Returns the last fully resolved path ##### route.prefix @@ -94,14 +94,12 @@ This method also allows you to asynchronously define what component will be rend `routeResolver.onmatch(vnode, resolve)` -Argument | Type | Description -------------------- | --------------------- | --- -`vnode` | `Vnode` | A [vnode](vnodes.md) whose attributes object contains routing parameters. If the routeResolver does not have a `resolve` method, the vnode's `tag` field defaults to a `div` -`vnode.attrs` | `Object` | The [routing parameters](#routing-parameters) -`vnode.attrs.path` | `String` | The current router path, including interpolated routing parameter values, but without the prefix. Same value as `m.route.get()` -`vnode.attrs.route` | `String` | The matched route -`resolve` | `Function(Component)` | Call this function with a component as the first argument to use it as the route's component -**returns** | | Returns `undefined` +Argument | Type | Description +--------------- | --------------------- | --- +`resolve` | `Function(Component)` | Call this function with a component as the first argument to use it as the route's component +`args` | `Object` | The [routing parameters](#routing-parameters) +`requestedPath` | `String` | The router path requested by the last routing action, including interpolated routing parameter values, but without the prefix. When `onmatch` is called, the resolution for this path is not complete and `m.route.get()` still returns the previous path. +**returns** | | Returns `undefined` ##### routeResolver.render @@ -113,8 +111,6 @@ Argument | Type | Description ------------------- | --------------- | ----------- `vnode` | `Object` | A [vnode](vnodes.md) whose attributes object contains routing parameters. If the routeResolver does not have a `resolve` method, the vnode's `tag` field defaults to a `div` `vnode.attrs` | `Object` | A [vnode](vnodes.md) whose attributes object contains routing parameters. If the routeResolver does not have a `resolve` method, the vnode defaults to a `div` -`vnode.attrs.path` | `String` | The current router path, including interpolated routing parameter values, but without the prefix. Same value as `m.route.get()` -`vnode.attrs.route` | `String` | The matched route **returns** | `Vnode` | Returns a vnode --- @@ -266,12 +262,12 @@ m.route.prefix("/my-app") ### Advanced component resolution -Instead of mapping a component to a route, you can specify a RouteResolver object. A RouteResolver object contains a `onmatch()` method and a optionally a `render()` method. +Instead of mapping a component to a route, you can specify a RouteResolver object. A RouteResolver object contains a `onmatch()` method and/or a `render()` method. ```javascript m.route(document.body, "/", { "/": { - onmatch: function(vnode, resolve) { + onmatch: function(resolve, args, requestedPath) { resolve(Home) }, render: function(vnode) { @@ -329,7 +325,7 @@ var Login = { m.route(document.body, "/secret", { "/secret": { - onmatch: function(vnode, resolve) { + onmatch: function(resolve) { if (isLoggedIn) resolve(Home) else m.route.set("/login") }, @@ -377,7 +373,7 @@ function load(file, done) { m.route(document.body, "/", { "/": { - onmatch: function(vnode, resolve) { + onmatch: function(resolve) { load("Home.js", resolve) }, }, @@ -391,7 +387,7 @@ Fortunately, there are a number of tools that facilitate the task of bundling mo ```javascript m.route(document.body, "/", { "/": { - onmatch: function(vnode, resolve) { + onmatch: function(resolve) { // using Webpack async code splitting require(['./Home.js'], resolve) }, From 879b9d6883bb787a118500d3dfe665c2d87be16a Mon Sep 17 00:00:00 2001 From: Pierre-Yves Gerardy Date: Fri, 2 Sep 2016 23:01:40 +0200 Subject: [PATCH 07/10] Adapt tests for debouncedAsync. --- api/tests/test-router.js | 277 ++++++++++++------------------ router/tests/test-defineRoutes.js | 12 +- 2 files changed, 114 insertions(+), 175 deletions(-) diff --git a/api/tests/test-router.js b/api/tests/test-router.js index 43db3493..a21efe87 100644 --- a/api/tests/test-router.js +++ b/api/tests/test-router.js @@ -28,7 +28,7 @@ o.spec("route", function() { route.prefix(prefix) }) - o("renders into `root`", function(done) { + o("renders into `root`", function() { $window.location.href = prefix + "/" route(root, "/", { "/" : { @@ -38,11 +38,21 @@ o.spec("route", function() { } }) - callAsync(function() { - o(root.firstChild.nodeName).equals("DIV") + o(root.firstChild.nodeName).equals("DIV") + }) + + o("routed mount points can redraw synchronoulsy (#1275)", function() { + var view = o.spy() + + $window.location.href = prefix + "/" + route(root, "/", {"/":{view:view}}) + + o(view.callCount).equals(1) + + redraw.publish(true) + + o(view.callCount).equals(2) - done() - }) }) o("default route doesn't break back button", function(done) { @@ -84,7 +94,7 @@ o.spec("route", function() { } }) - o("redraws when render function is executed", function(done) { + o("redraws when render function is executed", function() { var onupdate = o.spy() var oninit = o.spy() @@ -100,18 +110,11 @@ o.spec("route", function() { } }) - callAsync(function() { - o(oninit.callCount).equals(1) + o(oninit.callCount).equals(1) - redraw.publish() + redraw.publish(true) - // Wrapped to give time for the rate-limited redraw to fire - setTimeout(function() { - o(onupdate.callCount).equals(1) - - done() - }, FRAME_BUDGET) - }) + o(onupdate.callCount).equals(1) }) o("redraws on events", function(done) { @@ -135,23 +138,21 @@ o.spec("route", function() { } }) - callAsync(function() { - root.firstChild.dispatchEvent(e) + root.firstChild.dispatchEvent(e) - o(oninit.callCount).equals(1) + o(oninit.callCount).equals(1) - o(onclick.callCount).equals(1) - o(onclick.this).equals(root.firstChild) - o(onclick.args[0].type).equals("click") - o(onclick.args[0].target).equals(root.firstChild) + o(onclick.callCount).equals(1) + o(onclick.this).equals(root.firstChild) + o(onclick.args[0].type).equals("click") + o(onclick.args[0].target).equals(root.firstChild) - // Wrapped to give time for the rate-limited redraw to fire - setTimeout(function() { - o(onupdate.callCount).equals(1) + // Wrapped to give time for the rate-limited redraw to fire + setTimeout(function() { + o(onupdate.callCount).equals(1) - done() - }, FRAME_BUDGET) - }) + done() + }, FRAME_BUDGET * 2) }) o("event handlers can skip redraw", function(done) { @@ -177,21 +178,19 @@ o.spec("route", function() { } }) - callAsync(function() { - root.firstChild.dispatchEvent(e) + root.firstChild.dispatchEvent(e) - o(oninit.callCount).equals(1) + o(oninit.callCount).equals(1) - // Wrapped to ensure no redraw fired - setTimeout(function() { - o(onupdate.callCount).equals(0) + // Wrapped to ensure no redraw fired + setTimeout(function() { + o(onupdate.callCount).equals(0) - done() - }, FRAME_BUDGET) - }) + done() + }, FRAME_BUDGET) }) - o("changes location on route.link", function(done) { + o("changes location on route.link", function() { var e = $window.document.createEvent("MouseEvents") e.initEvent("click", true, true) @@ -213,20 +212,16 @@ o.spec("route", function() { } }) - callAsync(function() { - var slash = prefix[0] === "/" ? "" : "/" + var slash = prefix[0] === "/" ? "" : "/" - o($window.location.href).equals(env.protocol + "//" + (env.hostname === "/" ? "" : env.hostname) + slash + (prefix ? prefix + "/" : "")) + o($window.location.href).equals(env.protocol + "//" + (env.hostname === "/" ? "" : env.hostname) + slash + (prefix ? prefix + "/" : "")) - root.firstChild.dispatchEvent(e) + root.firstChild.dispatchEvent(e) - o($window.location.href).equals(env.protocol + "//" + (env.hostname === "/" ? "" : env.hostname) + slash + (prefix ? prefix + "/" : "") + "test") - - done() - }) + o($window.location.href).equals(env.protocol + "//" + (env.hostname === "/" ? "" : env.hostname) + slash + (prefix ? prefix + "/" : "") + "test") }) - o("accepts RouteResolver", function(done) { + o("accepts RouteResolver", function() { var matchCount = 0 var renderCount = 0 var Component = { @@ -235,7 +230,7 @@ o.spec("route", function() { } } - $window.location.href = prefix + "/" + $window.location.href = prefix + "/abc" route(root, "/abc", { "/:id" : { onmatch: function(resolve, args, requestedPath) { @@ -256,16 +251,12 @@ o.spec("route", function() { }, }) - setTimeout(function() { - o(matchCount).equals(1) - o(renderCount).equals(1) - o(root.firstChild.nodeName).equals("DIV") - - done() - }, FRAME_BUDGET) + o(matchCount).equals(1) + o(renderCount).equals(1) + o(root.firstChild.nodeName).equals("DIV") }) - o("accepts RouteResolver without `render` method as payload", function(done) { + o("accepts RouteResolver without `render` method as payload", function() { var matchCount = 0 var Component = { view: function() { @@ -273,7 +264,7 @@ o.spec("route", function() { } } - $window.location.href = prefix + "/" + $window.location.href = prefix + "/abc" route(root, "/abc", { "/:id" : { onmatch: function(resolve, args, requestedPath) { @@ -287,16 +278,12 @@ o.spec("route", function() { }, }) - setTimeout(function() { - o(matchCount).equals(1) + o(matchCount).equals(1) - o(root.firstChild.nodeName).equals("DIV") - - done() - }, FRAME_BUDGET) + o(root.firstChild.nodeName).equals("DIV") }) - o("accepts RouteResolver without `onmatch` method as payload", function(done) { + o("accepts RouteResolver without `onmatch` method as payload", function() { var renderCount = 0 var Component = { view: function() { @@ -304,7 +291,7 @@ o.spec("route", function() { } } - $window.location.href = prefix + "/" + $window.location.href = prefix + "/abc" route(root, "/abc", { "/:id" : { render: function(vnode) { @@ -317,16 +304,10 @@ o.spec("route", function() { }, }) - setTimeout(function() { - o(root.firstChild.nodeName).equals("DIV") - - done() - }, FRAME_BUDGET) + o(root.firstChild.nodeName).equals("DIV") }) - o("RouteResolver `render` does not have component semantics", function(done, timeout) { - timeout(60) - + o("RouteResolver `render` does not have component semantics", function(done) { var renderCount = 0 var A = { view: function() { @@ -334,7 +315,7 @@ o.spec("route", function() { } } - $window.location.href = prefix + "/" + $window.location.href = prefix + "/a" route(root, "/a", { "/a" : { render: function(vnode) { @@ -348,21 +329,19 @@ o.spec("route", function() { }, }) + var dom = root.firstChild + o(root.firstChild.nodeName).equals("DIV") + + route.set("/b") + setTimeout(function() { - var dom = root.firstChild - o(root.firstChild.nodeName).equals("DIV") + o(root.firstChild).equals(dom) - route.set("/b") - - setTimeout(function() { - o(root.firstChild).equals(dom) - - done() - }, FRAME_BUDGET) + done() }, FRAME_BUDGET) }) - o("calls onmatch and view correct number of times", function(done) { + o("calls onmatch and view correct number of times", function() { var matchCount = 0 var renderCount = 0 var Component = { @@ -385,25 +364,19 @@ o.spec("route", function() { }, }) - callAsync(function() { - o(matchCount).equals(1) - o(renderCount).equals(1) + o(matchCount).equals(1) + o(renderCount).equals(1) - redraw.publish() + redraw.publish(true) - setTimeout(function() { - o(matchCount).equals(1) - o(renderCount).equals(2) - - done() - }, FRAME_BUDGET) - }) + o(matchCount).equals(1) + o(renderCount).equals(2) }) o("onmatch can redirect to another route", function(done) { var redirected = false - $window.location.href = prefix + "/" + $window.location.href = prefix + "/a" route(root, "/a", { "/a" : { onmatch: function() { @@ -427,7 +400,7 @@ o.spec("route", function() { o("onmatch can redirect to another route that has RouteResolver", function(done) { var redirected = false - $window.location.href = prefix + "/" + $window.location.href = prefix + "/a" route(root, "/a", { "/a" : { onmatch: function() { @@ -477,59 +450,35 @@ o.spec("route", function() { }, FRAME_BUDGET) }) - o("the previous view redraws while onmatch resolution is pending (#1268)", function(done, timeout) { - timeout(FRAME_BUDGET * 5) + o("the previous view redraws while onmatch resolution is pending (#1268)", function(done) { var view = o.spy() var onmatch = o.spy() - $window.location.href = prefix + "/" + $window.location.href = prefix + "/a" route(root, "/", { - "/": {view: view}, - "/2": {onmatch: onmatch} + "/a": {view: view}, + "/b": {onmatch: onmatch} }) - setTimeout(function() { - o(view.callCount).equals(1) - o(onmatch.callCount).equals(0) - - route.set("/2") - - setTimeout(function(){ - o(view.callCount).equals(1) - o(onmatch.callCount).equals(1) - - redraw.publish() - - setTimeout(function() { - o(view.callCount).equals(2) - o(onmatch.callCount).equals(1) - - done() - }, FRAME_BUDGET) - }, FRAME_BUDGET) - }, FRAME_BUDGET) - }) - - o("routed mount points can redraw synchronoulsy (#1275)", function(done) { - var view = o.spy() - - $window.location.href = prefix + "/" - route(root, "/", {"/":{view:view}}) - - setTimeout(function() { + o(view.callCount).equals(1) + o(onmatch.callCount).equals(0) + + route.set("/b") + + setTimeout(function(){ o(view.callCount).equals(1) + o(onmatch.callCount).equals(1) redraw.publish(true) o(view.callCount).equals(2) + o(onmatch.callCount).equals(1) done() }, FRAME_BUDGET) }) - o("m.route.set(m.route.get()) re-runs the resolution logic (#1180)", function(done, timeout){ - timeout(FRAME_BUDGET * 3) - + o("m.route.set(m.route.get()) re-runs the resolution logic (#1180)", function(done){ var onmatch = o.spy(function(resolve){resolve()}) $window.location.href = prefix + "/" @@ -540,16 +489,14 @@ o.spec("route", function() { } }) + o(onmatch.callCount).equals(1) + + route.set(route.get()) + setTimeout(function() { - o(onmatch.callCount).equals(1) + o(onmatch.callCount).equals(2) - route.set(route.get()) - - setTimeout(function() { - o(onmatch.callCount).equals(2) - - done() - }, FRAME_BUDGET) + done() }, FRAME_BUDGET) }) @@ -562,19 +509,18 @@ o.spec("route", function() { }) - setTimeout(function() { - o(route.get()).equals("/") - - route.set("/2") - - o(route.get()).equals("/") + o(route.get()).equals("/") + + route.set("/2") - done() + setTimeout(function(){ + o(route.get()).equals("/") + done() }, FRAME_BUDGET) }) o("routing with RouteResolver works more than once (#1286)", function(done, timeout){ - timeout(FRAME_BUDGET * 4) + timeout(FRAME_BUDGET * 3) $window.location.href = prefix + "/a" route(root, '/a', { @@ -590,26 +536,24 @@ o.spec("route", function() { } }) + route.set('/b') + setTimeout(function(){ - route.set('/b') + route.set('/a') setTimeout(function(){ - route.set('/a') + o(root.firstChild.nodeName).equals("A") - setTimeout(function(){ - o(root.firstChild.nodeName).equals("A") - - done() - }, FRAME_BUDGET) + done() }, FRAME_BUDGET) }, FRAME_BUDGET) }) o("calling route.set invalidates pending onmatch resolution", function(done, timeout) { - timeout(100) + timeout(50) var resolved - $window.location.href = prefix + "/" + $window.location.href = prefix + "/a" route(root, "/a", { "/a": { onmatch: function(resolve) { @@ -621,15 +565,14 @@ o.spec("route", function() { view: function() {resolved = "b"} } }) + + route.set("/b") + setTimeout(function() { - route.set("/b") + o(resolved).equals("b") - setTimeout(function() { - o(resolved).equals("b") - - done() - }, 30) - }, FRAME_BUDGET) + done() + }, 30) }) }) }) diff --git a/router/tests/test-defineRoutes.js b/router/tests/test-defineRoutes.js index 9ad33d67..641616d1 100644 --- a/router/tests/test-defineRoutes.js +++ b/router/tests/test-defineRoutes.js @@ -285,18 +285,14 @@ o.spec("Router.defineRoutes", function() { }) }) - o("replays", function(done) { + o("replays", function() { $window.location.href = prefix + "/test" var replay = router.defineRoutes({"/test": {data: 1}}, onRouteChange, onFail) replay() - callAsync(function() { - o(onRouteChange.callCount).equals(2) - o(onRouteChange.args).deepEquals([{data: 1}, {}, "/test", "/test"]) - o(onFail.callCount).equals(0) - - done() - }) + o(onRouteChange.callCount).equals(2) + o(onRouteChange.args).deepEquals([{data: 1}, {}, "/test", "/test"]) + o(onFail.callCount).equals(0) }) }) }) From 0539fc651dbe47eae7f6015fee51959860c5fb14 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Gerardy Date: Fri, 2 Sep 2016 23:08:28 +0200 Subject: [PATCH 08/10] core router: add `debounceAsync()`, normalize onhashchange and onpopstate timing. Fix ##1269 --- router/router.js | 44 +++++++++++++++++++++++++++----------------- 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/router/router.js b/router/router.js index 61ded804..1785c480 100644 --- a/router/router.js +++ b/router/router.js @@ -16,6 +16,18 @@ module.exports = function($window) { return data } + var asyncId + function debounceAsync(f) { + return function(){ + if (asyncId != null) return + asyncId = callAsync(function(){ + asyncId = null + f() + }) + + } + } + function parsePath(path, queryData, hashData) { var queryIndex = path.indexOf("?") var hashIndex = path.indexOf("#") @@ -67,7 +79,7 @@ module.exports = function($window) { } function defineRoutes(routes, resolve, reject) { - if (supportsPushState) $window.onpopstate = resolveRoute + if (supportsPushState) $window.onpopstate = debounceAsync(resolveRoute) else if (prefix.charAt(0) === "#") $window.onhashchange = resolveRoute resolveRoute() @@ -76,25 +88,23 @@ module.exports = function($window) { var params = {} var pathname = parsePath(path, params, params) - callAsync(function() { - for (var route in routes) { - var matcher = new RegExp("^" + route.replace(/:[^\/]+?\.{3}/g, "(.*?)").replace(/:[^\/]+/g, "([^\\/]+)") + "\/?$") + for (var route in routes) { + var matcher = new RegExp("^" + route.replace(/:[^\/]+?\.{3}/g, "(.*?)").replace(/:[^\/]+/g, "([^\\/]+)") + "\/?$") - if (matcher.test(pathname)) { - pathname.replace(matcher, function() { - var keys = route.match(/:[^\/]+/g) || [] - var values = [].slice.call(arguments, 1, -2) - for (var i = 0; i < keys.length; i++) { - params[keys[i].replace(/:|\./g, "")] = decodeURIComponent(values[i]) - } - resolve(routes[route], params, path, route) - }) - return - } + if (matcher.test(pathname)) { + pathname.replace(matcher, function() { + var keys = route.match(/:[^\/]+/g) || [] + var values = [].slice.call(arguments, 1, -2) + for (var i = 0; i < keys.length; i++) { + params[keys[i].replace(/:|\./g, "")] = decodeURIComponent(values[i]) + } + resolve(routes[route], params, path, route) + }) + return } + } - reject(path, params) - }) + reject(path, params) } return resolveRoute } From a1131eb4f23d6e05981b727145cce2abf752339a Mon Sep 17 00:00:00 2001 From: Pierre-Yves Gerardy Date: Sat, 3 Sep 2016 00:32:12 +0200 Subject: [PATCH 09/10] Lint the router files --- api/router.js | 8 ++++---- router/router.js | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/api/router.js b/api/router.js index d8496c30..5f1e3365 100644 --- a/api/router.js +++ b/api/router.js @@ -10,7 +10,7 @@ module.exports = function($window, mount) { var RouteComponent = {view: function() { return currentRender(Vnode(currentComponent, null, currentArgs, undefined, undefined, undefined)) }} - function defaultRender(vnode){ + function defaultRender(vnode) { return vnode } var route = function(root, defaultRoute, routes) { @@ -20,7 +20,7 @@ module.exports = function($window, mount) { mount(root, RouteComponent) - router.defineRoutes(routes, function(payload, args, path, route) { + router.defineRoutes(routes, function(payload, args, path) { var resolutionIdentifier = globalId = {} var isResolver = typeof payload.view !== "function" var render = defaultRender @@ -29,14 +29,14 @@ module.exports = function($window, mount) { if (resolutionIdentifier !== globalId) return globalId = null - currentComponent = component != null ? component: isResolver ? "div" : payload + currentComponent = component != null ? component : isResolver ? "div" : payload currentRender = render currentArgs = args currentPath = path root.redraw(true) } - function onmatch() { + var onmatch = function() { resolve() } if (isResolver) { diff --git a/router/router.js b/router/router.js index 1785c480..3ce7fe8d 100644 --- a/router/router.js +++ b/router/router.js @@ -18,9 +18,9 @@ module.exports = function($window) { var asyncId function debounceAsync(f) { - return function(){ + return function() { if (asyncId != null) return - asyncId = callAsync(function(){ + asyncId = callAsync(function() { asyncId = null f() }) From 057f3a9d2f1511ebe2802f7a4d9804bb9dc74861 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Gerardy Date: Sat, 3 Sep 2016 23:28:17 +0200 Subject: [PATCH 10/10] Fix the bundle and bundle tests --- index.js | 2 +- tests/test-api.js | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/index.js b/index.js index 6155ba71..6eb607f3 100644 --- a/index.js +++ b/index.js @@ -10,8 +10,8 @@ var Stream = require("./stream") requestService.setCompletionCallback(redrawService.publish) -m.route = require("./route") m.mount = require("./mount") +m.route = require("./route") m.withAttr = require("./util/withAttr") m.prop = Stream m.render = renderService.render diff --git a/tests/test-api.js b/tests/test-api.js index 4d57d5cc..3c91606e 100644 --- a/tests/test-api.js +++ b/tests/test-api.js @@ -164,9 +164,11 @@ o.spec("api", function() { setTimeout(function() { m.route.set("/b") - o(m.route.get()).equals("/b") + setTimeout(function() { + o(m.route.get()).equals("/b") - done() + done() + }, FRAME_BUDGET) }, FRAME_BUDGET) }) })